Commit 752ba56f authored by Mickaël Salaün's avatar Mickaël Salaün Committed by David S. Miller

bpf: Extend check_uarg_tail_zero() checks

The function check_uarg_tail_zero() was created from bpf(2) for
BPF_OBJ_GET_INFO_BY_FD without taking the access_ok() nor the PAGE_SIZE
checks. Make this checks more generally available while unlikely to be
triggered, extend the memory range check and add an explanation
including why the ToCToU should not be a security concern.
Signed-off-by: default avatarMickaël Salaün <mic@digikod.net>
Acked-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Kees Cook <keescook@chromium.org>
Cc: Martin KaFai Lau <kafai@fb.com>
Link: https://lkml.kernel.org/r/CAGXu5j+vRGFvJZmjtAcT8Hi8B+Wz0e1b6VKYZHfQP_=DXzC4CQ@mail.gmail.comSigned-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 58291a74
...@@ -48,6 +48,15 @@ static const struct bpf_map_ops * const bpf_map_types[] = { ...@@ -48,6 +48,15 @@ static const struct bpf_map_ops * const bpf_map_types[] = {
#undef BPF_MAP_TYPE #undef BPF_MAP_TYPE
}; };
/*
* If we're handed a bigger struct than we know of, ensure all the unknown bits
* are 0 - i.e. new user-space does not rely on any kernel feature extensions
* we don't know about yet.
*
* There is a ToCToU between this function call and the following
* copy_from_user() call. However, this is not a concern since this function is
* meant to be a future-proofing of bits.
*/
static int check_uarg_tail_zero(void __user *uaddr, static int check_uarg_tail_zero(void __user *uaddr,
size_t expected_size, size_t expected_size,
size_t actual_size) size_t actual_size)
...@@ -57,6 +66,12 @@ static int check_uarg_tail_zero(void __user *uaddr, ...@@ -57,6 +66,12 @@ static int check_uarg_tail_zero(void __user *uaddr,
unsigned char val; unsigned char val;
int err; int err;
if (unlikely(actual_size > PAGE_SIZE)) /* silly large */
return -E2BIG;
if (unlikely(!access_ok(VERIFY_READ, uaddr, actual_size)))
return -EFAULT;
if (actual_size <= expected_size) if (actual_size <= expected_size)
return 0; return 0;
...@@ -1393,17 +1408,6 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz ...@@ -1393,17 +1408,6 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
if (!capable(CAP_SYS_ADMIN) && sysctl_unprivileged_bpf_disabled) if (!capable(CAP_SYS_ADMIN) && sysctl_unprivileged_bpf_disabled)
return -EPERM; return -EPERM;
if (!access_ok(VERIFY_READ, uattr, 1))
return -EFAULT;
if (size > PAGE_SIZE) /* silly large */
return -E2BIG;
/* If we're handed a bigger struct than we know of,
* ensure all the unknown bits are 0 - i.e. new
* user-space does not rely on any kernel feature
* extensions we dont know about yet.
*/
err = check_uarg_tail_zero(uattr, sizeof(attr), size); err = check_uarg_tail_zero(uattr, sizeof(attr), size);
if (err) if (err)
return err; return err;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment