On 15/04/2025 1:49 pm, Teddy Astie wrote: > UBSAN complains when trying memcpy with a NULL pointer even if it's going to > copy zero bytes (which are the only cases where a NULL pointer is used). > Fix this by only doing the memcpy if the pointer is non-NULL.
Which compiler are you using? (Just so there's a record. These reports are version-sensitive.) > > (XEN) > ================================================================================ > (XEN) UBSAN: Undefined behaviour in arch/x86/mm/shadow/hvm.c:168:5 > (XEN) null pointer passed as argument 1, declared with nonnull attribute Interestingly, this isn't a Xen annotation. It must be coming from the builtin. And what we really want is nonnull_if_nonzero, but I bet that's not available. > (XEN) ----[ Xen-4.21-unstable x86_64 debug=y ubsan=y Not tainted ]---- > (XEN) CPU: 0 > (XEN) RIP: e008:[<ffff82d0402f789c>] > common/ubsan/ubsan.c#ubsan_epilogue+0xa/0xd2 > (XEN) RFLAGS: 0000000000010046 CONTEXT: hypervisor (d1v0) > (XEN) rax: ffff82d040a82eb0 rbx: ffff83021b6e7808 rcx: 000000000000c458 > (XEN) rdx: ffff83021b6e7fd0 rsi: 000000000000000a rdi: ffff83021b6e7808 > (XEN) rbp: ffff83021b6e77f8 rsp: ffff83021b6e77e8 r8: 00000000ffffffff > (XEN) r9: 00000000ffffffff r10: 0000000000000000 r11: 0000000000000000 > (XEN) r12: 000000000000004d r13: 0000000000000000 r14: ffff82d040a82eb0 > (XEN) r15: 00000000002ffddc cr0: 0000000080050033 cr4: 00000000001526e0 > (XEN) cr3: 000000021b7f4000 cr2: 0000000000000000 > (XEN) fsb: 0000000000000000 gsb: 0000000000000000 gss: 0000000000000000 > (XEN) ds: 0000 es: 0000 fs: 0000 gs: 0000 ss: 0000 cs: e008 > (XEN) Xen code around <ffff82d0402f789c> > (common/ubsan/ubsan.c#ubsan_epilogue+0xa/0xd2): > (XEN) 89 e5 41 54 53 48 89 fb <0f> 0b 48 8d 3d 1b cf 36 00 e8 b4 94 00 00 48 > 85 > (XEN) Xen stack trace from rsp=ffff83021b6e77e8: > (XEN) ffff82d040a82ea0 000000000000004d ffff83021b6e7820 ffff82d0402f8435 > (XEN) 0000000000000202 ffff83021b6e7d25 0000000000000000 ffff83021b6e7858 > (XEN) ffff82d040455cb6 00000000002ffddc ffff83021b6e7ef8 ffff83021fbf1010 > (XEN) 0000000000000000 0000000000000000 ffff83021b6e7bd8 ffff82d0405b562b > (XEN) ffffffff00200033 ffffffff0020874b 00007cfde4918743 ffff83021b6e7af0 > (XEN) 0000000000000003 000000000000000a 0000000000000000 0000000440661f40 > (XEN) ffffffff00000000 0000000000000000 00007cfd000000e8 ffff83021b6e79a8 > (XEN) ffff83021b6e7980 ffff82d040d3fa90 00000000a00000e8 ffff82d0406904a0 > (XEN) ffff83021b6e7cd8 ffff8302159963f0 ffff83021b6e7998 ffff82d04052f592 > (XEN) fffffffa0000000a ffff83021b6e7b21 393082d040661f40 0000001000000033 > (XEN) ffffffff00307b39 ffffffffe491868b ffffffff00200d00 00007cfde491867b > (XEN) ffff83021b6e7bb8 0000000000000003 0000000000000001 0000000000000000 > (XEN) 0000000000000000 0000000000000067 ffff8302159963f0 aaaaaaaaaaaaaaaa > (XEN) aaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaa > (XEN) aaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaa ffff83021b7fc008 > (XEN) 00000000000002ff ffff8302159963f0 0000000000000000 ffff830215994000 > (XEN) 0000000715994000 0000000000000000 0000000000000003 0000000000000000 > (XEN) 0000000000000000 8086000000008086 0000000000000000 0000000000000000 > (XEN) 0000000400000002 00000000002ffddc 0000000000000000 8086000000008086 > (XEN) 0000000000000000 0000000000000000 ffffffffffffffff 000000000000001f > (XEN) Xen call trace: > (XEN) [<ffff82d0402f789c>] R common/ubsan/ubsan.c#ubsan_epilogue+0xa/0xd2 > (XEN) [<ffff82d0402f8435>] F __ubsan_handle_nonnull_arg+0x7c/0xb3 > (XEN) [<ffff82d040455cb6>] F > arch/x86/mm/shadow/hvm.c#hvm_emulate_insn_fetch+0xfb/0x100 > (XEN) [<ffff82d0405b562b>] F x86_emulate+0x17f6b/0x3b8e3 > (XEN) [<ffff82d0405dce4f>] F x86_emulate_wrapper+0x87/0x216 > (XEN) [<ffff82d040489847>] F > arch/x86/mm/shadow/guest_4.c#sh_page_fault__guest_4+0x908/0x3b34 > (XEN) [<ffff82d0403ca3ac>] F vmx_vmexit_handler+0x1691/0x3439 > (XEN) [<ffff82d040204683>] F vmx_asm_vmexit_handler+0x103/0x220 > (XEN) > (XEN) > ================================================================================ For this, we normally abbreviate it to just the relevant information, so in this case: (XEN) UBSAN: Undefined behaviour in arch/x86/mm/shadow/hvm.c:168:5 (XEN) null pointer passed as argument 1, declared with nonnull attribute (XEN) ----[ Xen-4.21-unstable x86_64 debug=y ubsan=y Not tainted ]---- (XEN) CPU: 0 (XEN) RIP: e008:[<ffff82d0402f789c>] common/ubsan/ubsan.c#ubsan_epilogue+0xa/0xd2 ... (XEN) Xen call trace: (XEN) [<ffff82d0402f789c>] R common/ubsan/ubsan.c#ubsan_epilogue+0xa/0xd2 (XEN) [<ffff82d0402f8435>] F __ubsan_handle_nonnull_arg+0x7c/0xb3 (XEN) [<ffff82d040455cb6>] F arch/x86/mm/shadow/hvm.c#hvm_emulate_insn_fetch+0xfb/0x100 (XEN) [<ffff82d0405b562b>] F x86_emulate+0x17f6b/0x3b8e3 (XEN) [<ffff82d0405dce4f>] F x86_emulate_wrapper+0x87/0x216 (XEN) [<ffff82d040489847>] F arch/x86/mm/shadow/guest_4.c#sh_page_fault__guest_4+0x908/0x3b34 (XEN) [<ffff82d0403ca3ac>] F vmx_vmexit_handler+0x1691/0x3439 (XEN) [<ffff82d040204683>] F vmx_asm_vmexit_handler+0x103/0x220 which is rather less voluminous. > > Signed-off-by: Teddy Astie <teddy.as...@vates.tech> > --- > xen/arch/x86/mm/shadow/hvm.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/mm/shadow/hvm.c b/xen/arch/x86/mm/shadow/hvm.c > index 114957a3e1..298dd0f229 100644 > --- a/xen/arch/x86/mm/shadow/hvm.c > +++ b/xen/arch/x86/mm/shadow/hvm.c > @@ -165,7 +165,8 @@ hvm_emulate_insn_fetch(unsigned long offset, > hvm_access_insn_fetch, sh_ctxt); > > /* Hit the cache. Simple memcpy. */ > - memcpy(p_data, &sh_ctxt->insn_buf[insn_off], bytes); > + if ( p_data ) > + memcpy(p_data, &sh_ctxt->insn_buf[insn_off], bytes); Do you know precisely which condition is being hit? ~Andrew
