Re: Xen-unstable, stubdom causes hypervisor crash

["Roger Pau Monné" <roger.pau@citrix.com>]

El 20/05/15 a les 17.59, Jan Beulich ha escrit:
>>>> On 20.05.15 at 17:45, <andrew.cooper3@citrix.com> wrote:
>> On 20/05/15 16:43, Andrew Cooper wrote:
>>> On 20/05/15 16:39, Wei Liu wrote:
>>>> I discovered this when running qemu-trad stubdom + shadow page table.
>>>>
>>>> (XEN) Assertion 'pages' failed at vmap.c:275
>>>> (XEN) ----[ Xen-4.6-unstable  x86_64  debug=y  Tainted:    C ]----
>>>> (XEN) CPU:    1
>>>> (XEN) RIP:    e008:[<ffff82d08013d226>] vfree+0x1e/0x128
>>>> (XEN) RFLAGS: 0000000000010246   CONTEXT: hypervisor (d2v0)
>>>> (XEN) rax: 0000000000000000   rbx: 0000000000000000   rcx: ffff82c0001fff66
>>>> (XEN) rdx: 0000000000000000   rsi: 0000000000009bd1   rdi: 0000000000000000
>>>> (XEN) rbp: ffff830224857cc8   rsp: ffff830224857c88   r8:  ffff830224857ca4
>>>> (XEN) r9:  0000000000000000   r10: ffff82d080261e40   r11: 0000000000000202
>>>> (XEN) r12: 0000000000000000   r13: ffff830215672000   r14: 0000000000000000
>>>> (XEN) r15: 0000000000000000   cr0: 000000008005003b   cr4: 00000000000026f4
>>>> (XEN) cr3: 00000001cb060000   cr2: ffff880012dbd6c8
>>>> (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
>>>> (XEN) Xen stack trace from rsp=ffff830224857c88:
>>>> (XEN)    0000000000000000 ffff830224857ca8 ffff82d08012f5c6 0000000000000000
>>>> (XEN)    0000000000000000 ffff830215672000 0000000000000000 0000000000000000
>>>> (XEN)    ffff830224857d78 ffff82d08021c4ad 0000000000000200 0000000000000005
>>>> (XEN)    ffff830224857d58 ffff82d0801620ca ffff830224886020 0000000000000000
>>>> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>>> (XEN)    ffff830215672ac0 0000000000000000 0000000000000006 000000200200b004
>>>> (XEN)    ffffffffffffffea ffffffffffffffff 0000000000000006 000000200200b004
>>>> (XEN)    ffffffffffffffea 0000000000000000 ffff830224857e58 ffff82d0801d4ae0
>>>> (XEN)    0000000000000000 0000000000000000 0000000000000001 0000000000000000
>>>> (XEN)    ffff830224857db8 ffff830224857dc8 0000000000000202 ffff830224857dd8
>>>> (XEN)    ffff830224857dd8 ffff82d08019e6eb ffff830224857e28 ffff82d08019ed8a
>>>> (XEN)    ffff83020180a0c8 00000000000ee6c7 ffff830224857e28 ffff830215672000
>>>> (XEN)    0000000000000001 0000000000000000 0000000000000000 0000000000000000
>>>> (XEN)    ffff830224857f08 ffff8300cf0fc1f8 ffff8300cf0fc000 00000000005ef640
>>>> (XEN)    ffff830224850000 0000000000000000 ffff830224857ef8 ffff82d08011bb5f
>>>> (XEN)    ffff8300cf0fc200 ffff8300cf0fc208 0000000100000000 ffff8300cf0fc1f8
>>>> (XEN)    ffff830224857ea8 ffff82d000a0fb00 0000000000000000 ffffffffffffffff
>>>> (XEN)    ffff830224857ec8 ffff82d000000031 ffff82d080320000 ffff82d08031ff80
>>>> (XEN)    ffff830224857ef8 ffff8300cf0fc000 00000000005ef640 000000200202e1f0
>>>> (XEN)    0000000000000001 000000200201ba18 00007cfddb7a80c7 ffff82d080247bdb
>>>> (XEN) Xen call trace:
>>>> (XEN)    [<ffff82d08013d226>] vfree+0x1e/0x128
>>>> (XEN)    [<ffff82d08021c4ad>] shadow_track_dirty_vram+0x7ca/0x8aa
>>>> (XEN)    [<ffff82d0801d4ae0>] do_hvm_op+0x1aec/0x273b
>>>> (XEN)    [<ffff82d08011bb5f>] do_multicall+0x257/0x3dc
>>>> (XEN)    [<ffff82d080247bdb>] syscall_enter+0xeb/0x145
>>>> (XEN)
>>>> (XEN)
>>>> (XEN) ****************************************
>>>> (XEN) Panic on CPU 1:
>>>> (XEN) Assertion 'pages' failed at vmap.c:275
>>>> (XEN) ****************************************
>>>> (XEN)
>>>>
>>>> Any idea what might go wrong?
>>> I have an idea - patch incoming
>>
>> Try this:  It appears that vfree(NULL) isn't safe.
> 
> And intentionally so (I think this was even mentioned while discussing
> the patch), matching vunmap().

Yes, but previous versions of vfree where able to cope with NULL. The 
following fixes the callers:

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index cea7990..0316b59 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -174,7 +174,8 @@ int hap_track_dirty_vram(struct domain *d,
                                   p2m_ram_logdirty, p2m_ram_rw);
     }
 out:
-    vfree(dirty_bitmap);
+    if ( dirty_bitmap )
+        vfree(dirty_bitmap);
 
     return rc;
 }
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index 9e9d19f..88e0f7e 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -3707,7 +3707,8 @@ out:
         paging_unlock(d);
         rc = -EFAULT;
     }
-    vfree(dirty_bitmap);
+    if ( dirty_bitmap )
+        vfree(dirty_bitmap);
     p2m_unlock(p2m_get_hostp2m(d));
     return rc;
 }