Re: [RFC PATCH 2/4] uaccess: Add non-pagefault user-space read functions

From: Masami Hiramatsu
Date: Mon Feb 25 2019 - 22:02:27 EST


Hi Peter,

On Mon, 25 Feb 2019 16:06:03 +0100
Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:

> On Mon, Feb 25, 2019 at 11:05:41PM +0900, Masami Hiramatsu wrote:
>
> > +static __always_inline long __strncpy_from_unsafe_user(char *dst,
> > + const char __user *unsafe_addr, long count)
> > +{
> > + if (!access_ok(unsafe_addr, count))
> > + return -EFAULT;
> > +
> > + return strncpy_from_unsafe_common(dst, unsafe_addr, count);
> > +}
>
> Would something like so work for people?
>
> ---
> arch/x86/include/asm/uaccess.h | 8 +++++++-
> include/linux/uaccess.h | 18 ++++++++++++++++++
> 2 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> index 780f2b42c8ef..3125d129d3b6 100644
> --- a/arch/x86/include/asm/uaccess.h
> +++ b/arch/x86/include/asm/uaccess.h
> @@ -92,12 +92,18 @@ static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size, un
> * checks that the pointer is in the user space range - after calling
> * this function, memory access functions may still return -EFAULT.
> */
> -#define access_ok(addr, size) \
> +#define access_ok(addr, size) \
> ({ \
> WARN_ON_IN_IRQ(); \
> likely(!__range_not_ok(addr, size, user_addr_max())); \
> })
>
> +#define user_access_ok(addr, size) \
> +({ \
> + WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS)); \
> + likely(!__range_not_ok(addr, size, user_addr_max())); \
> +})
> +
> /*
> * These are the main single-value transfer routines. They automatically
> * use the right size if we just have the right pointer type.
> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> index 37b226e8df13..088f2ae09e14 100644
> --- a/include/linux/uaccess.h
> +++ b/include/linux/uaccess.h
> @@ -10,6 +10,24 @@
>
> #include <asm/uaccess.h>
>
> +/**
> + * user_access_ok: Checks if a user space pointer is valid
> + * @addr: User space pointer to start of block to check
> + * @size: Size of block to check
> + *
> + * Context: User context or explicit set_fs(USER_DS).
> + *
> + * This function is very much like access_ok(), except it (may) have different
> + * context validation. In general we must be very careful when using this.
> + */
> +#ifndef user_access_ok
> +#define user_access_ok(addr, size) \
> +({ \
> + WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS)); \
> + access_ok(addr, size); \
> +})
> +#endif
> +
> /*
> * Architectures should provide two primitives (raw_copy_{to,from}_user())
> * and get rid of their private instances of copy_{to,from}_user() and

Yeah, looks good to me.

Thank you,


--
Masami Hiramatsu <mhiramat@xxxxxxxxxx>