Re: [PATCH v2 4/5] x86/shadow: fix UB pointer arithmetic in sh_mfn_is_a_page_table()

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Mar 18, 2025 at 03:36:45PM +0100, Jan Beulich wrote:
> On 18.03.2025 13:53, Andrew Cooper wrote:
> > On 18/03/2025 9:19 am, Roger Pau Monne wrote:
> >> UBSAN complains with:
> >>
> >> UBSAN: Undefined behaviour in arch/x86/mm/shadow/private.h:515:30
> >> pointer operation overflowed ffff82e000000000 to ffff82dfffffffe0
> >> [...]
> >> Xen call trace:
> >>    [<ffff82d040303882>] R common/ubsan/ubsan.c#ubsan_epilogue+0xa/0xc0
> >>    [<ffff82d040304cc3>] F 
> >> lib/xxhash64.c#__ubsan_handle_pointer_overflow+0xcb/0x100
> >>    [<ffff82d040471c5d>] F 
> >> arch/x86/mm/shadow/guest_2.c#sh_page_fault__guest_2+0x1e350
> >>    [<ffff82d0403b216b>] F lib/xxhash64.c#svm_vmexit_handler+0xdf3/0x2450
> >>    [<ffff82d0402049c0>] F lib/xxhash64.c#svm_stgi_label+0x5/0x15
> > 
> > Something is definitely wonky in this backtrace.
> > 
> >>
> >> Fix by moving the call to mfn_to_page() after the check of whether the
> >> passed gmfn is valid.  This avoid the call to mfn_to_page() with an
> >> INVALID_MFN parameter.
> >>
> >> While there make the page local variable const, it's not modified by the
> >> function.
> >>
> >> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > 
> > Whatever is wonky in the backtrace isn't related to this patch, so
> > Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, but the backtrace
> > does want fixing.
> 
> Right, but the fix may need to be in the tool chain. I'd be curious what
> the symbol table looks like that this was created from. Roger, was this
> linked with GNU ld or LLVM? Are the filename anomalies also visible in
> the corresponding xen-syms.map?

It's with LLVM LD, it's this issue:

https://lore.kernel.org/xen-devel/20220505142137.51306-1-roger.pau@xxxxxxxxxx/

I need to refresh that patch and resend.

Sorry, I got so used to those wonky filenames in the backtraces that I
no longer notice.

Thanks, Roger.