From: Christoph Hellwig
> Sent: 28 July 2020 17:39
>
> Make sure not just the pointer itself but the whole range lies in
> the user address space. For that pass the length and then use
> the access_ok helper to do the check.
Now that the address is never changed it is enough to check the
base address (although this would be slightly safer if sockaddr_t
were 'const').
This is especially true given some code paths ignore the length
(so the length must be checked later - which it will be).
Isn't TASK_SIZE the wrong 'constant'.
Looks pretty 'heavy' on x86-64.
You just need something between valid user and valid kernel addresses.
So (1ull << 63) is fine on x86-64 (and probably all 64bit with
non-overlapping user/kernel addresses).
For i386 you need (3ul << 30) - probably also common.
David
>
> Fixes: 6d04fe15f78a ("net: optimize the sockptr_t for unified kernel/user
> address spaces")
> Reported-by: David Laight <david.lai...@aculab.com>
> Signed-off-by: Christoph Hellwig <h...@lst.de>
> ---
> include/linux/sockptr.h | 18 ++++++------------
> net/ipv4/bpfilter/sockopt.c | 2 +-
> net/socket.c | 2 +-
> 3 files changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/include/linux/sockptr.h b/include/linux/sockptr.h
> index 9e6c81d474cba8..96840def9d69cc 100644
> --- a/include/linux/sockptr.h
> +++ b/include/linux/sockptr.h
> @@ -27,14 +27,6 @@ static inline sockptr_t KERNEL_SOCKPTR(void *p)
> {
> return (sockptr_t) { .kernel = p };
> }
> -
> -static inline int __must_check init_user_sockptr(sockptr_t *sp, void __user
> *p)
> -{
> - if ((unsigned long)p >= TASK_SIZE)
> - return -EFAULT;
> - sp->user = p;
> - return 0;
> -}
> #else /* CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE */
> typedef struct {
> union {
> @@ -53,14 +45,16 @@ static inline sockptr_t KERNEL_SOCKPTR(void *p)
> {
> return (sockptr_t) { .kernel = p, .is_kernel = true };
> }
> +#endif /* CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE */
>
> -static inline int __must_check init_user_sockptr(sockptr_t *sp, void __user
> *p)
> +static inline int __must_check init_user_sockptr(sockptr_t *sp, void __user
> *p,
> + size_t size)
> {
> - sp->user = p;
> - sp->is_kernel = false;
> + if (!access_ok(p, size))
> + return -EFAULT;
> + *sp = (sockptr_t) { .user = p };
> return 0;
> }
> -#endif /* CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE */
>
> static inline bool sockptr_is_null(sockptr_t sockptr)
> {
> diff --git a/net/ipv4/bpfilter/sockopt.c b/net/ipv4/bpfilter/sockopt.c
> index 94f18d2352d007..545b2640f0194d 100644
> --- a/net/ipv4/bpfilter/sockopt.c
> +++ b/net/ipv4/bpfilter/sockopt.c
> @@ -65,7 +65,7 @@ int bpfilter_ip_get_sockopt(struct sock *sk, int optname,
>
> if (get_user(len, optlen))
> return -EFAULT;
> - err = init_user_sockptr(&optval, user_optval);
> + err = init_user_sockptr(&optval, user_optval, len);
> if (err)
> return err;
> return bpfilter_mbox_request(sk, optname, optval, len, false);
> diff --git a/net/socket.c b/net/socket.c
> index 94ca4547cd7c53..aff52e81653ce3 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -2105,7 +2105,7 @@ int __sys_setsockopt(int fd, int level, int optname,
> char __user *user_optval,
> if (optlen < 0)
> return -EINVAL;
>
> - err = init_user_sockptr(&optval, user_optval);
> + err = init_user_sockptr(&optval, user_optval, optlen);
> if (err)
> return err;
>
> --
> 2.27.0
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
