Re: [PATCH v3 02/22] compat: provide compat_ptr() on all architectures

From: Michael Ellerman
Date: Mon Jan 06 2020 - 21:05:34 EST


Arnd Bergmann <arnd@xxxxxxxx> writes:
> In order to avoid needless #ifdef CONFIG_COMPAT checks,
> move the compat_ptr() definition to linux/compat.h
> where it can be seen by any file regardless of the
> architecture.
>
> Only s390 needs a special definition, this can use the
> self-#define trick we have elsewhere.
>
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> ---
> arch/arm64/include/asm/compat.h | 17 -----------------
> arch/mips/include/asm/compat.h | 18 ------------------
> arch/parisc/include/asm/compat.h | 17 -----------------
> arch/powerpc/include/asm/compat.h | 17 -----------------
> arch/powerpc/oprofile/backtrace.c | 2 +-

LGTM.

Acked-by: Michael Ellerman <mpe@xxxxxxxxxxxxxx> (powerpc)

One minor comment:

> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index 68f79d855c3d..11083d84eb23 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -958,4 +958,22 @@ static inline bool in_compat_syscall(void) { return false; }
>
> #endif /* CONFIG_COMPAT */
>
> +/*
> + * A pointer passed in from user mode. This should not
> + * be used for syscall parameters, just declare them
> + * as pointers because the syscall entry code will have
> + * appropriately converted them already.
> + */
> +#ifndef compat_ptr
> +static inline void __user *compat_ptr(compat_uptr_t uptr)
> +{
> + return (void __user *)(unsigned long)uptr;
> +}
> +#endif
> +
> +static inline compat_uptr_t ptr_to_compat(void __user *uptr)
> +{
> + return (u32)(unsigned long)uptr;
> +}

Is there a reason we cast to u32 directly instead of using compat_uptr_t?

cheers