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é <[email protected]>
Reviewed-by: Andrew Cooper <[email protected]>, 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.
> 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.
~Andrew
