Re: [Xen-devel] [PATCH 3/3] xen/x86: Use non-canonical pointers for ERR_PTR() errors

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.11.16 at 11:46, <andrew.cooper3@xxxxxxxxxx> wrote:
> The top of the virutal address space is owned by 64bit PV kernels.  Code which
> fails to correctly check an ERR_PTR() value might follow the pointer into
> guest space.
> 
> Mitigate this risk by sliding the ERR_PTR() error range into the non-canonical
> region.
> 
> As this comes with a small overhead, and isn't necessary if 64bit PV guests
> aren't used, provide a Kconfig opt-out for power users.

And it's this overhead which I dislike. Not properly handling the values
here is just like not properly checking for NULL, and I don't think you
mean to propose to give NULL a value other than numeric zero?

> --- a/xen/include/asm-x86/config.h
> +++ b/xen/include/asm-x86/config.h
> @@ -87,6 +87,14 @@
>  #define LIST_POISON1  ((void *)0x0100100100100100UL)
>  #define LIST_POISON2  ((void *)0x0200200200200200UL)
>  
> +#if !defined(NDEBUG) || !defined(CONFIG_UNSAFE_ERRPTR)

With that the config option should depend on !DEBUG I would say,
or default to DEBUG and have its prompt hidden when DEBUG
(simplifying the expression above).

> +/*
> + * Always use safe pointers in debug builds.  Use safe pointers in release
> + * builds unless the user explicitly opts out.
> + */
> +#define ARCH_ERR_PTR_SLIDE (-(unsigned long)0xbad0eee100000000ull)

What good does casting the constant to unsigned long? Did you
perhaps mean to use just an ul suffix and cast to signed long?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel