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 | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index d8553f4..e90c4ce7 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -2358,6 +2358,8 @@ kprobe_multi_resolve_syms(const void __user *usyms, u32 cnt, unsigned int i; char *func;
+ if (check_mul_overflow(cnt, sizeof(*syms), &size)) + return -EOVERFLOW; size = cnt * sizeof(*syms); syms = kvzalloc(size, GFP_KERNEL); if (!syms) @@ -2429,6 +2431,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; size = cnt * sizeof(*addrs); addrs = kvmalloc(size, GFP_KERNEL); if (!addrs)
On Mon, May 16, 2022 at 08:27:08PM +0200, 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 | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index d8553f4..e90c4ce7 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -2358,6 +2358,8 @@ kprobe_multi_resolve_syms(const void __user *usyms, u32 cnt, unsigned int i; char *func;
- if (check_mul_overflow(cnt, sizeof(*syms), &size))
return -EOVERFLOW;
there was an update already:
0236fec57a15 bpf: Resolve symbols with ftrace_lookup_symbols for kprobe multi link
so this won't apply anymore, could you please rebase on top of the latest bpf-next/master?
thanks, jirka
size = cnt * sizeof(*syms); syms = kvzalloc(size, GFP_KERNEL); if (!syms) @@ -2429,6 +2431,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;-- 2.1.4
On Mon, May 16, 2022 at 11:34:45PM +0200, Jiri Olsa wrote:
On Mon, May 16, 2022 at 08:27:08PM +0200, Eugene Syromiatnikov wrote:
- if (check_mul_overflow(cnt, sizeof(*syms), &size))
return -EOVERFLOW;there was an update already:
0236fec57a15 bpf: Resolve symbols with ftrace_lookup_symbols for kprobe multi link
so this won't apply anymore, could you please rebase on top of the latest bpf-next/master?
The issue that this specific check has to go in 4.18, as it covers possible out-of-bounds write, I'm not sure how to handle it, have a branch where it is merged manually?
On Tue, May 17, 2022 at 12:49:34AM +0200, Eugene Syromiatnikov wrote:
On Mon, May 16, 2022 at 11:34:45PM +0200, Jiri Olsa wrote:
On Mon, May 16, 2022 at 08:27:08PM +0200, Eugene Syromiatnikov wrote:
- if (check_mul_overflow(cnt, sizeof(*syms), &size))
return -EOVERFLOW;there was an update already:
0236fec57a15 bpf: Resolve symbols with ftrace_lookup_symbols for kprobe multi link
so this won't apply anymore, could you please rebase on top of the latest bpf-next/master?
The issue that this specific check has to go in 4.18, as it covers possible out-of-bounds write, I'm not sure how to handle it, have a branch where it is merged manually?
As Jiri said, please use bpf-next.
linux-kselftest-mirror@lists.linaro.org
-
Alexei Starovoitov -
Eugene Syromiatnikov -
Jiri Olsa