From: Daniel Borkmann <daniel@iogearbox.net>
To: bpf@vger.kernel.org
Cc: shung-hsi.yu@suse.com, andrii@kernel.org, ast@kernel.org,
kongln9170@gmail.com, Daniel Borkmann <daniel@iogearbox.net>
Subject: [PATCH bpf-next v4 5/8] bpf: Zero former ARG_PTR_TO_{LONG,INT} args in case of error
Date: Fri, 6 Sep 2024 15:56:05 +0200 [thread overview]
Message-ID: <20240906135608.26477-5-daniel@iogearbox.net> (raw)
In-Reply-To: <20240906135608.26477-1-daniel@iogearbox.net>
For all non-tracing helpers which formerly had ARG_PTR_TO_{LONG,INT} as 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 hence bpf_get_func_arg() and bpf_get_func_ret() is skipped
in here.
Also, rearrange the MTU checker helpers a bit to among other nit fixes
consolidate flag checks such that we only need to zero in one location with
regards to malformed flag inputs.
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>
---
v1 -> v2:
- only set *mtu_len in error path (Alexei)
kernel/bpf/helpers.c | 2 ++
kernel/bpf/syscall.c | 1 +
net/core/filter.c | 35 +++++++++++++++++------------------
3 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 0587d0c2375a..ff66a0522799 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -523,6 +523,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;
@@ -549,6 +550,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 feb276771c03..513b4301a0af 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -5934,6 +5934,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 4be175f84eb9..c219385e7bb4 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -6264,18 +6264,19 @@ BPF_CALL_5(bpf_skb_check_mtu, struct sk_buff *, skb,
int skb_len, dev_len;
int mtu;
- if (unlikely(flags & ~(BPF_MTU_CHK_SEGS)))
- return -EINVAL;
-
- if (unlikely(flags & BPF_MTU_CHK_SEGS && (len_diff || *mtu_len)))
+ if (unlikely((flags & ~(BPF_MTU_CHK_SEGS)) ||
+ (flags & BPF_MTU_CHK_SEGS && (len_diff || *mtu_len)))) {
+ *mtu_len = 0;
return -EINVAL;
+ }
dev = __dev_via_ifindex(dev, ifindex);
- if (unlikely(!dev))
+ if (unlikely(!dev)) {
+ *mtu_len = 0;
return -ENODEV;
+ }
mtu = READ_ONCE(dev->mtu);
-
dev_len = mtu + dev->hard_header_len;
/* If set use *mtu_len as input, L3 as iph->tot_len (like fib_lookup) */
@@ -6286,10 +6287,10 @@ BPF_CALL_5(bpf_skb_check_mtu, struct sk_buff *, skb,
ret = BPF_MTU_CHK_RET_SUCCESS;
goto out;
}
- /* At this point, skb->len exceed MTU, but as it include length of all
- * segments, it can still be below MTU. The SKB can possibly get
- * re-segmented in transmit path (see validate_xmit_skb). Thus, user
- * must choose if segs are to be MTU checked.
+ /* At this point, skb->len exceeds MTU, but as it includes the length
+ * of all segments, it can still be below MTU. The skb can possibly
+ * get re-segmented in transmit path (see validate_xmit_skb). Thus,
+ * the user must choose if segments are to be MTU checked.
*/
if (skb_is_gso(skb)) {
ret = BPF_MTU_CHK_RET_SUCCESS;
@@ -6299,9 +6300,7 @@ BPF_CALL_5(bpf_skb_check_mtu, struct sk_buff *, skb,
ret = BPF_MTU_CHK_RET_SEGS_TOOBIG;
}
out:
- /* BPF verifier guarantees valid pointer */
*mtu_len = mtu;
-
return ret;
}
@@ -6314,16 +6313,18 @@ BPF_CALL_5(bpf_xdp_check_mtu, struct xdp_buff *, xdp,
int mtu, dev_len;
/* XDP variant doesn't support multi-buffer segment check (yet) */
- if (unlikely(flags))
+ if (unlikely(flags)) {
+ *mtu_len = 0;
return -EINVAL;
+ }
dev = __dev_via_ifindex(dev, ifindex);
- if (unlikely(!dev))
+ if (unlikely(!dev)) {
+ *mtu_len = 0;
return -ENODEV;
+ }
mtu = READ_ONCE(dev->mtu);
-
- /* Add L2-header as dev MTU is L3 size */
dev_len = mtu + dev->hard_header_len;
/* Use *mtu_len as input, L3 as iph->tot_len (like fib_lookup) */
@@ -6334,9 +6335,7 @@ BPF_CALL_5(bpf_xdp_check_mtu, struct xdp_buff *, xdp,
if (xdp_len > dev_len)
ret = BPF_MTU_CHK_RET_FRAG_NEEDED;
- /* BPF verifier guarantees valid pointer */
*mtu_len = mtu;
-
return ret;
}
--
2.43.0
next prev parent reply other threads:[~2024-09-06 13:56 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-06 13:56 [PATCH bpf-next v4 1/8] bpf: Fix bpf_strtol and bpf_strtoul helpers for 32bit Daniel Borkmann
2024-09-06 13:56 ` [PATCH bpf-next v4 2/8] bpf: Remove truncation test in bpf_strtol and bpf_strtoul helpers Daniel Borkmann
2024-09-06 21:03 ` Andrii Nakryiko
2024-09-06 13:56 ` [PATCH bpf-next v4 3/8] bpf: Fix helper writes to read-only maps Daniel Borkmann
2024-09-06 21:03 ` Andrii Nakryiko
2024-09-06 22:36 ` Alexei Starovoitov
2024-09-09 11:51 ` Shung-Hsi Yu
2024-09-06 13:56 ` [PATCH bpf-next v4 4/8] bpf: Improve check_raw_mode_ok test for MEM_UNINIT-tagged types Daniel Borkmann
2024-09-06 21:03 ` Andrii Nakryiko
2024-09-09 12:24 ` Shung-Hsi Yu
2024-09-06 13:56 ` Daniel Borkmann [this message]
2024-09-06 21:03 ` [PATCH bpf-next v4 5/8] bpf: Zero former ARG_PTR_TO_{LONG,INT} args in case of error Andrii Nakryiko
2024-09-06 22:35 ` Alexei Starovoitov
2024-09-09 12:16 ` Daniel Borkmann
2024-09-12 9:47 ` Daniel Borkmann
2024-09-12 17:59 ` Andrii Nakryiko
2024-09-12 22:47 ` Daniel Borkmann
2024-09-06 13:56 ` [PATCH bpf-next v4 6/8] selftests/bpf: Fix ARG_PTR_TO_LONG {half-,}uninitialized test Daniel Borkmann
2024-09-06 21:03 ` Andrii Nakryiko
2024-09-06 13:56 ` [PATCH bpf-next v4 7/8] selftests/bpf: Rename ARG_PTR_TO_LONG test description Daniel Borkmann
2024-09-06 13:56 ` [PATCH bpf-next v4 8/8] selftests/bpf: Add a test case to write into .rodata Daniel Borkmann
2024-09-06 21:02 ` Andrii Nakryiko
2024-09-06 21:02 ` [PATCH bpf-next v4 1/8] bpf: Fix bpf_strtol and bpf_strtoul helpers for 32bit Andrii Nakryiko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240906135608.26477-5-daniel@iogearbox.net \
--to=daniel@iogearbox.net \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=kongln9170@gmail.com \
--cc=shung-hsi.yu@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox