Re: [PATCH] bpf: explicitly memset the bpf_attr structure
From: Yonghong Song <hidden>
Date: 2020-03-20 15:24:22
Also in:
bpf, lkml
On 3/20/20 2:48 AM, Greg Kroah-Hartman wrote:
For the bpf syscall, we are relying on the compiler to properly zero out the bpf_attr union that we copy userspace data into. Unfortunately that doesn't always work properly, padding and other oddities might not be correctly zeroed, and in some tests odd things have been found when the stack is pre-initialized to other values.
Maybe add more contexts about the failure itself so it could be clear why we need this patch. As far as I know from the link below, the failure happens in CHECK_ATTR() which checks any unused *area* for a particular subcommand must be 0, and this patch tries to provide this guarantee beyond area beyond min(uattr_size, sizeof(attr)).
Fix this by explicitly memsetting the structure to 0 before using it. Reported-by: Maciej Żenczykowski <redacted> Reported-by: John Stultz <redacted> Reported-by: Alexander Potapenko <glider@google.com> Reported-by: Alistair Delva <redacted> Cc: stable <redacted> Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__android-2Dreview.googlesource.com_c_kernel_common_-2B_1235490&d=DwIDaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=LW31qE_U9SQxf_FnwMUaEXeM9h54NJ1fOf44hk_QDWk&s=HJ-aQi8Ho6V6ZegmWlPYJqnY7e3KRKfFjFj6C2yEN04&e= Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Yonghong Song <redacted>
quoted hunk ↗ jump to hunk
--- kernel/bpf/syscall.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index a91ad518c050..a4b1de8ea409 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c@@ -3354,7 +3354,7 @@ static int bpf_map_do_batch(const union bpf_attr *attr, SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size) { - union bpf_attr attr = {}; + union bpf_attr attr; int err; if (sysctl_unprivileged_bpf_disabled && !capable(CAP_SYS_ADMIN))@@ -3366,6 +3366,7 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz size = min_t(u32, size, sizeof(attr)); /* copy attributes from user space, may be less than sizeof(bpf_attr) */ + memset(&attr, 0, sizeof(attr)); if (copy_from_user(&attr, uattr, size) != 0) return -EFAULT;base-commit: 6c90b86a745a446717fdf408c4a8a4631a5e8ee3