On Tue, Mar 18, 2025 at 04:50:58PM +0100, Jan Beulich wrote:
> On 18.03.2025 16:35, Roger Pau Monné wrote:
> > On Tue, Mar 18, 2025 at 03:33:03PM +0100, Jan Beulich wrote:
> >> On 18.03.2025 10:19, Roger Pau Monne wrote:
> >>> --- a/xen/arch/x86/include/asm/x86_64/uaccess.h
> >>> +++ b/xen/arch/x86/include/asm/x86_64/uaccess.h
> >>> @@ -9,9 +9,9 @@
> >>> * a secondary mapping installed, which needs to be used for such
> >>> accesses in
> >>> * the PV case, and will also be used for HVM to avoid extra
> >>> conditionals.
> >>> */
> >>> -#define COMPAT_ARG_XLAT_VIRT_BASE ((void *)ARG_XLAT_START(current) + \
> >>> - (PERDOMAIN_ALT_VIRT_START - \
> >>> - PERDOMAIN_VIRT_START))
> >>> +#define COMPAT_ARG_XLAT_VIRT_BASE ((void *)ARG_XLAT_START(current) - \
> >>> + (PERDOMAIN_VIRT_START - \
> >>> + PERDOMAIN_ALT_VIRT_START))
> >>
> >> Aren't we then (still) dependent on ordering between PERDOMAIN_VIRT_START
> >> and PERDOMAIN_ALT_VIRT_START? Would
> >>
> >> #define COMPAT_ARG_XLAT_VIRT_BASE ((void *)ARG_XLAT_START(current) - \
> >> PERDOMAIN_VIRT_START + \
> >> PERDOMAIN_ALT_VIRT_START)
> >>
> >> perhaps be less fragile?
> >
> > PERDOMAIN_{ALT_,}VIRT_START are unsigned long, so this might work.
> >
> > Note however that even with your suggestion we are still dependant on
> > ARG_XLAT_START(v) > PERDOMAIN_ALT_VIRT_START, or else the '-' won't
> > work. I think I prefer my proposed version, because it's clear that
> > PERDOMAIN_VIRT_START, ARG_XLAT_START(current) >
> > PERDOMAIN_ALT_VIRT_START.
>
> What makes that clear? Can't we move PERDOMAIN_ALT_VIRT_START pretty
> much at will?
We would need to adjust the calculations here again, if
PERDOMAIN_ALT_VIRT_START > PERDOMAIN_VIRT_START the subtraction would
lead to an underflow, and would also be UB pointer arithmetic?
Thanks, Roger.
