| Message ID | 20220516182708.GA29437@asgard.redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fix 32-bit arch and compat support for the kprobe_multi attach type | expand |
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; > size = cnt * sizeof(*addrs); > addrs = kvmalloc(size, GFP_KERNEL); > if (!addrs) > -- > 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.
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)
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(+)