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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.08.13 at 19:00, Tomasz Wroblewski <tomasz.wroblewski@xxxxxxxxxx> 
>>> 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.

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)) )



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