[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 5/7] x86/ioremap: prevent additions against the NULL pointer
On Thu, Mar 13, 2025 at 05:21:13PM +0000, Andrew Cooper wrote: > On 13/03/2025 3:30 pm, Roger Pau Monne wrote: > > This was reported by clang UBSAN as: > > > > UBSAN: Undefined behaviour in arch/x86/mm.c:6297:40 > > applying zero offset to null pointer > > [...] > > Xen call trace: > > [<ffff82d040303662>] R common/ubsan/ubsan.c#ubsan_epilogue+0xa/0xc0 > > [<ffff82d040304aa3>] F __ubsan_handle_pointer_overflow+0xcb/0x100 > > [<ffff82d0406ebbc0>] F ioremap_wc+0xc8/0xe0 > > [<ffff82d0406c3728>] F video_init+0xd0/0x180 > > [<ffff82d0406ab6f5>] F console_init_preirq+0x3d/0x220 > > [<ffff82d0406f1876>] F __start_xen+0x68e/0x5530 > > [<ffff82d04020482e>] F __high_start+0x8e/0x90 > > > > Fix bt_ioremap() and ioremap{,_wc}() to not add the offset if the returned > > pointer from __vmap() is NULL. > > > > Fixes: d0d4635d034f ('implement vmap()') > > Fixes: f390941a92f1 ('x86/DMI: fix table mapping when one lives above 1Mb') > > Fixes: 81d195c6c0e2 ('x86: introduce ioremap_wc()') > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> > > Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, with one style fix. > > It's unfortunate, because C23 makes this one case (add 0 to NULL > pointer) explicitly well defined to avoid corner cases like this. Oh well. Interesting, so they added a new type (nullptr_t) that has a single possible value (nullptr), and hence arithmetic operations against it always result in nullptr. That's helpful to prevent this kind of bugs. > > diff --git a/xen/arch/x86/dmi_scan.c b/xen/arch/x86/dmi_scan.c > > index 2fcc485295eb..a05492037519 100644 > > --- a/xen/arch/x86/dmi_scan.c > > +++ b/xen/arch/x86/dmi_scan.c > > @@ -119,8 +120,10 @@ static const void *__init bt_ioremap(paddr_t addr, > > unsigned int len) > > if ( system_state < SYS_STATE_boot ) > > return __acpi_map_table(addr, len); > > > > - return __vmap(&mfn, PFN_UP(offs + len), 1, 1, PAGE_HYPERVISOR_RO, > > - VMAP_DEFAULT) + offs; > > + va = __vmap(&mfn, PFN_UP(offs + len), 1, 1, PAGE_HYPERVISOR_RO, > > + VMAP_DEFAULT); > > You've got mixed tabs/spaces here. Thanks, vim autodetection is a bit confused with this file because it uses both hard and soft tabs, fixed now. Roger.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |