Re: [Xen-devel] [RFC] x86/hpet: Correct -ENOMEM actions in hpet_fsb_cap_lookup()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.08.13 at 18:55, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> These changes are entirely from inspection, discovered while investigating
> another problem.
> 
> * Don't leak the previously allocated cpumasks
> * Don't leave num_hpets_used > 0.  It would fool hpet_broadcast_init() into
>   believing that broadcast mode had been set up, despite having freed the
>   underlying datastructure (and subsequenly result in a NULL pointer fault).
> * Unconditionally decallocate hpet_events.  hpet_broadcast_init() will then
>   try to allocate a single hpet_event_channel instead.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: Keir Fraser <keir@xxxxxxx>
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> 
> ---
> 
> This patch is RFC as I didn't actually encounter the problem, nor can think of
> an easy way of actually testing the correctness of the codepath.  Chances are
> that if -ENOMEM occurs here, Xen is not actually going to complete booting.

And you're turning a success case (just using fewer than the
available channels) into an error one - it was intentionally coded
this way, and only if there's a problem with that logic I'd consider
a patch valid.

Jan

> --- a/xen/arch/x86/hpet.c
> +++ b/xen/arch/x86/hpet.c
> @@ -415,11 +415,12 @@ static void __init hpet_fsb_cap_lookup(void)
>  
>          if ( !zalloc_cpumask_var(&ch->cpumask) )
>          {
> -            if ( !num_hpets_used )
> -            {
> -                xfree(hpet_events);
> -                hpet_events = NULL;
> -            }
> +            /* Out of mem.  Clean up and bail. */
> +            for ( i = 0; i < num_hpets_used; ++i )
> +                free_cpumask_var(hpet_events[i].cpumask);
> +            xfree(hpet_events);
> +            hpet_events = NULL;
> +            num_hpets_used = 0;
>              break;
>          }
>  
> -- 
> 1.7.10.4




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel