Re: [Xen-devel] [PATCH v2] xen/arm: vcpu: Correctly release resource when the VCPU failed to initialized

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Ian,

On 05/02/2014 01:25 PM, Ian Campbell wrote:
> On Wed, 2014-04-30 at 20:15 +0100, Julien Grall wrote:
>> While I was adding new failing code at the end of the function, I've noticed
>> that the vtimers are not freed which mess all the timers and will crash Xen
>> quickly when the page will be reused.
>>
>> Currently neither vcpu_vgic_init nor vcpu_vtimer_init fail, so we
>> are safe for now. With the new GICv3 code, the former function will be able
>> to fail. This will result to a memory leak.
>>
>> Call vcpu_destroy if the initialization has failed. We also need to add a
>> boolean to know if the vtimers are correctly setup as the timer common code
>> doesn't have safe guard against removing non-initialized timer.
>>
>> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> 
> I was about to acked + apply but it failed to build on arm64 with:
> 
>         domain.c: In function 'alloc_vcpu_struct':
>         /local/scratch/ianc/devel/committer.git/xen/include/xen/lib.h:19:31: 
> error: static assertion failed: "!(sizeof(*v) > PAGE_SIZE)"
>          #define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond 
> ")"); })
>                                        ^
>         domain.c:415:5: note: in expansion of macro 'BUILD_BUG_ON'
>              BUILD_BUG_ON(sizeof(*v) > PAGE_SIZE);
>              ^
> struct arch_vcpu is apparently now too large.
> 
> I had also reworded your commit message somewhat:
>     xen/arm: vcpu: Correctly release resources when a VCPU fails to initialize
>     
>     While I was adding new failing code at the end of the function, I noticed
>     that the vtimers are not freed which messes up all the timers and will 
> crash
>     Xen quickly when the page s reused.
>     
>     Currently neither vcpu_vgic_init nor vcpu_vtimer_init fails, so we
>     are safe for now. With the new GICv3 code, the former function will be 
> able
>     to fail. This will result in a memory leak.
>     
>     Call vcpu_destroy if the initialization has failed. We also need to add a
>     boolean to know if the vtimers are correctly setup as the timer common 
> code
>     doesn't have any safeguard against removing a non-initialized timer.

The commit 6fedf29 "xen/arm: Drop event_mask in arch_vcpu" which makes
this patch compiles on both arm32 and arm64.

I think you can safely push this patch now :).

Thanks,

-- 
Julien Grall

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