Re: [PATCH] Fix boot crash on xsm/flask enabled builds when no policy module is present

[Tomasz Wroblewski <tomasz.wroblewski@citrix.com>]

On 08/27/2013 09:13 AM, Jan Beulich wrote:
>>>> On 26.08.13 at 19:00, Tomasz Wroblewski<tomasz.wroblewski@citrix.com>  wrote:
>> I've verified that replacing security_load_policy call
>> completely with the following allocation/deallocaiton is enough to cause
>> this crash:
>>
>>       //ret = security_load_policy(policy_buffer, policy_size);
>>       {
>>               void ** p = xmalloc_array(void*, 768);
>>               xfree(p);
>>       }
>>
>> Note that this allocation succeeds, and also if you would not call xfree
>> (which is not called if say a policy was succesfully loaded), there is
>> no crash. So yeah my original patch accidentaly fixes it by just
>> avoiding the alloc/free completely.
> But I then understand, together with the below, that the crash isn't
> down the xfree() path, ...
>
>> The shaky manually constructed call graph for the assertion failure:
>>
>> setup.c: init_idle_domain
>> schedule.c: scheduler_init
>> domain.c: domain_create
>> domain.c: alloc_domain_struct
>> domain.c: alloc_xenheap_pages
>> ..
>> page_alloc.c: alloc_heap_pages
>> flushtlb.h: flush_tlb_mask
>> flushtlb.h: flush_mask
>> smp.c: flush_area_mask - hits ASSERT because interrupts are disabled here
> ... but instead is on a _subsequent_ allocation. Hence the prior
> free gets a heap page into a state that makes in non-suitable for
> re-use. But you certainly noticed that free_heap_pages() sets a
> page's u.free.need_tlbflush only if the page had an owner,
> which shouldn't be the case for Xen-internal allocations.
>
> With that, I think I can see where the bug really is: The owner
> field (v.inuse._domain) is in a union with the order field re-used
> by the xmalloc() implementation for whole page allocations. The
> fix therefore ought to be as simple as the patch below.
Just tested that fix, works! Thanks
> Jan
>
> --- a/xen/common/xmalloc_tlsf.c
> +++ a/xen/common/xmalloc_tlsf.c
> @@ -629,6 +629,7 @@ void xfree(void *p)
>           unsigned int i, order = get_order_from_pages(size);
>
>           BUG_ON((unsigned long)p&  ((PAGE_SIZE<<  order) - 1));
> +        PFN_ORDER(virt_to_page(p)) = 0;
>           for ( i = 0; ; ++i )
>           {
>               if ( !(size&  (1<<  i)) )
>
>