On 1/12/25 3:37 AM, Jason Xing wrote:
In the next round, we will support the UDP proto for SO_TIMESTAMPING
bpf extension, so we need to ensure there is no safety problem.
Signed-off-by: Jason Xing <kerneljasonxing@xxxxxxxxx>
---
net/core/filter.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index 0e915268db5f..517f09aabc92 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5571,7 +5571,7 @@ static int __bpf_getsockopt(struct sock *sk, int level, int optname,
static int _bpf_getsockopt(struct sock *sk, int level, int optname,
char *optval, int optlen)
{
- if (sk_fullsock(sk))
+ if (sk_fullsock(sk) && optname != SK_BPF_CB_FLAGS)
sock_owned_by_me(sk);
return __bpf_getsockopt(sk, level, optname, optval, optlen);
}
@@ -5776,6 +5776,7 @@ BPF_CALL_5(bpf_sock_ops_getsockopt, struct bpf_sock_ops_kern *, bpf_sock,
int, level, int, optname, char *, optval, int, optlen)
{
if (IS_ENABLED(CONFIG_INET) && level == SOL_TCP &&
+ bpf_sock->sk->sk_protocol == IPPROTO_TCP &&
optname >= TCP_BPF_SYN && optname <= TCP_BPF_SYN_MAC) {
int ret, copy_len = 0;
const u8 *start;
@@ -5817,7 +5818,8 @@ BPF_CALL_2(bpf_sock_ops_cb_flags_set, struct bpf_sock_ops_kern *, bpf_sock,
struct sock *sk = bpf_sock->sk;
int val = argval & BPF_SOCK_OPS_ALL_CB_FLAGS;
- if (!IS_ENABLED(CONFIG_INET) || !sk_fullsock(sk))
+ if (!IS_ENABLED(CONFIG_INET) || !sk_fullsock(sk) ||
+ sk->sk_protocol != IPPROTO_TCP)
return -EINVAL;
tcp_sk(sk)->bpf_sock_ops_cb_flags = val;
@@ -7626,6 +7628,9 @@ BPF_CALL_4(bpf_sock_ops_load_hdr_opt, struct bpf_sock_ops_kern *, bpf_sock,
u8 search_kind, search_len, copy_len, magic_len;
int ret;
+ if (bpf_sock->op != SK_BPF_CB_FLAGS)
SK_BPF_CB_FLAGS is not an op enum, so the check is incorrect. It does break the
existing test.
./test_progs -t tcp_hdr_options
WARNING! Selftests relying on bpf_testmod.ko will be skipped.
#402/1 tcp_hdr_options/simple_estab:FAIL
#402/2 tcp_hdr_options/no_exprm_estab:FAIL
#402/3 tcp_hdr_options/syncookie_estab:FAIL
#402/4 tcp_hdr_options/fastopen_estab:FAIL
#402/5 tcp_hdr_options/fin:FAIL
#402/6 tcp_hdr_options/misc:FAIL
#402 tcp_hdr_options:FAIL
#402/1 tcp_hdr_options/simple_estab:FAIL
#402/2 tcp_hdr_options/no_exprm_estab:FAIL
#402/3 tcp_hdr_options/syncookie_estab:FAIL
#402/4 tcp_hdr_options/fastopen_estab:FAIL
#402/5 tcp_hdr_options/fin:FAIL
#402/6 tcp_hdr_options/misc:FAIL
#402 tcp_hdr_options:FAIL
Many changes of this set is in bpf and the newly added selftest is also a bpf
prog, all bpf selftests should be run before posting.
(Documentation/bpf/bpf_devel_QA.rst)
The bpf CI can automatically pick it up and get an auto email on breakage like
this if the set is tagged to bpf-next. We can figure out where to land the set
later (bpf-next/net or net-next/main) when it is ready.
All these changes also need a test in selftests/bpf. For example, I expect there
is a test to ensure calling these bpf helpers from the new tstamp callback will
get a negative errno value.
For patch 4 and patch 5, I would suggest keeping it simple to only check for
bpf_sock->op for the helpers that make tcp_sock and/or locked sk assumption.
Something like this on top of your patch. Untested:
diff --git i/net/core/filter.c w/net/core/filter.c
index 517f09aabc92..ccb13b61c528 100644
--- i/net/core/filter.c
+++ w/net/core/filter.c
@@ -7620,6 +7620,11 @@ static const u8 *bpf_search_tcp_opt(const u8 *op, const
u8 *opend,
return ERR_PTR(-ENOMSG);
}
+static bool is_locked_tcp_sock_ops(struct bpf_sock_ops_kern *bpf_sock)
+{
+ return bpf_sock->op <= BPF_SOCK_OPS_WRITE_HDR_OPT_CB;
+}
+
BPF_CALL_4(bpf_sock_ops_load_hdr_opt, struct bpf_sock_ops_kern *, bpf_sock,
void *, search_res, u32, len, u64, flags)
{
@@ -7628,8 +7633,8 @@ BPF_CALL_4(bpf_sock_ops_load_hdr_opt, struct
bpf_sock_ops_kern *, bpf_sock,
u8 search_kind, search_len, copy_len, magic_len;
int ret;
- if (bpf_sock->op != SK_BPF_CB_FLAGS)
- return -EINVAL;
+ if (!is_locked_tcp_sock_ops(bpf_sock))
+ return -EOPNOTSUPP;
/* 2 byte is the minimal option len except TCPOPT_NOP and
* TCPOPT_EOL which are useless for the bpf prog to learn
+ return -EINVAL;
+
/* 2 byte is the minimal option len except TCPOPT_NOP and
* TCPOPT_EOL which are useless for the bpf prog to learn
* and this helper disallow loading them also.
