| Message ID | 20240823222033.31006-2-daniel@iogearbox.net (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf,1/4] bpf: Fix helper writes to read-only maps | expand |
On Sat, Aug 24, 2024 at 12:20:31AM GMT, Daniel Borkmann wrote: > For all non-tracing helpers which have ARG_PTR_TO_{LONG,INT} | MEM_UNINIT > input arguments, zero the value for the case of an error as otherwise it > could leak memory. For tracing, it is not needed given CAP_PERFMON can > already read all kernel memory anyway. > > Fixes: 8a67f2de9b1d ("bpf: expose bpf_strtol and bpf_strtoul to all program types") > Fixes: d7a4cb9b6705 ("bpf: Introduce bpf_strtol and bpf_strtoul helpers") > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> [...] Acked-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
On Fri, Aug 23, 2024 at 3:20 PM Daniel Borkmann <daniel@iogearbox.net> wrote: > diff --git a/net/core/filter.c b/net/core/filter.c > index 2ff210cb068c..a25c32da3d6c 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -6264,6 +6264,8 @@ BPF_CALL_5(bpf_skb_check_mtu, struct sk_buff *, skb, > int skb_len, dev_len; > int mtu; > > + *mtu_len = 0; > + > if (unlikely(flags & ~(BPF_MTU_CHK_SEGS))) > return -EINVAL; > > @@ -6313,6 +6315,8 @@ BPF_CALL_5(bpf_xdp_check_mtu, struct xdp_buff *, xdp, > int ret = BPF_MTU_CHK_RET_SUCCESS; > int mtu, dev_len; > > + *mtu_len = 0; > + > /* XDP variant doesn't support multi-buffer segment check (yet) */ > if (unlikely(flags)) > return -EINVAL; This looks wrong. If selftests are not failing because of that they should.
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 356a58aeb79b..20f6a2b7e708 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -522,6 +522,7 @@ BPF_CALL_4(bpf_strtol, const char *, buf, size_t, buf_len, u64, flags, long long _res; int err; + *res = 0; err = __bpf_strtoll(buf, buf_len, flags, &_res); if (err < 0) return err; @@ -548,6 +549,7 @@ BPF_CALL_4(bpf_strtoul, const char *, buf, size_t, buf_len, u64, flags, bool is_negative; int err; + *res = 0; err = __bpf_strtoull(buf, buf_len, flags, &_res, &is_negative); if (err < 0) return err; diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 6d5942a6f41f..f799179fd6c7 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -5932,6 +5932,7 @@ static const struct bpf_func_proto bpf_sys_close_proto = { BPF_CALL_4(bpf_kallsyms_lookup_name, const char *, name, int, name_sz, int, flags, u64 *, res) { + *res = 0; if (flags) return -EINVAL; diff --git a/net/core/filter.c b/net/core/filter.c index 2ff210cb068c..a25c32da3d6c 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -6264,6 +6264,8 @@ BPF_CALL_5(bpf_skb_check_mtu, struct sk_buff *, skb, int skb_len, dev_len; int mtu; + *mtu_len = 0; + if (unlikely(flags & ~(BPF_MTU_CHK_SEGS))) return -EINVAL; @@ -6313,6 +6315,8 @@ BPF_CALL_5(bpf_xdp_check_mtu, struct xdp_buff *, xdp, int ret = BPF_MTU_CHK_RET_SUCCESS; int mtu, dev_len; + *mtu_len = 0; + /* XDP variant doesn't support multi-buffer segment check (yet) */ if (unlikely(flags)) return -EINVAL;
For all non-tracing helpers which have ARG_PTR_TO_{LONG,INT} | MEM_UNINIT input arguments, zero the value for the case of an error as otherwise it could leak memory. For tracing, it is not needed given CAP_PERFMON can already read all kernel memory anyway. Fixes: 8a67f2de9b1d ("bpf: expose bpf_strtol and bpf_strtoul to all program types") Fixes: d7a4cb9b6705 ("bpf: Introduce bpf_strtol and bpf_strtoul helpers") Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> --- kernel/bpf/helpers.c | 2 ++ kernel/bpf/syscall.c | 1 + net/core/filter.c | 4 ++++ 3 files changed, 7 insertions(+)