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 12:53:30PM +0000, 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.

Oh, yes, it's a TODO I have pending when using LLVM LD.  I sent a fix
very long time ago, but it was quite ugly.

> >
> > 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.

I can get the proper backtrace using clang + GNU LD.

Thanks, Roger.