On 5/16/22 4:04 PM, Eugene Syromiatnikov wrote:
Check that size would not overflow before calculation (and return -EOVERFLOW if it will), to prevent potential out-of-bounds write with the following copy_from_user. Add the same check to kprobe_multi_resolve_syms in case it will be called from elsewhere in the future.
Fixes: 0dcac272540613d4 ("bpf: Add multi kprobe link") Signed-off-by: Eugene Syromiatnikov esyr@redhat.com
kernel/trace/bpf_trace.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index d8553f4..f1d4e68 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -2352,12 +2352,15 @@ static int kprobe_multi_resolve_syms(const void __user *usyms, u32 cnt, unsigned long *addrs) {
- unsigned long addr, size;
- unsigned long addr, sym_size;
- u32 size; const char __user **syms; int err = -ENOMEM; unsigned int i; char *func;
- if (check_mul_overflow(cnt, (u32)sizeof(*syms), &size))
return -EOVERFLOW;
The statement 'size = cnt * sizeof(*syms)' can be removed right? Here, the 'size' will be computed properly with check_mul_overflow() if 0 is returned.
syms = kvzalloc(size, GFP_KERNEL); if (!syms) @@ -2382,9 +2385,9 @@ kprobe_multi_resolve_syms(const void __user *usyms, u32 cnt, addr = kallsyms_lookup_name(func); if (!addr) goto error;
if (!kallsyms_lookup_size_offset(addr, &size, NULL))
if (!kallsyms_lookup_size_offset(addr, &sym_size, NULL)) goto error;
addr = ftrace_location_range(addr, addr + size - 1);
addr = ftrace_location_range(addr, addr + sym_size - 1);@@ -2429,6 +2432,8 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr if (!cnt) return -EINVAL;
- if (check_mul_overflow(cnt, (u32)sizeof(*addrs), &size))
return -EOVERFLOW;
Same here, 'size = ...' is not needed.
addrs = kvmalloc(size, GFP_KERNEL); if (!addrs)