[PATCH bpf v3 0/2] bpf: fix sock_ops rtt_min OOB read

[PATCH bpf v3 0/2] bpf: fix sock_ops rtt_min OOB read

[Werner Kasselman]

Patch 1 from v2 is upstream as 10f86a2a5c91 ("bpf: Fix same-register
dst/src OOB read and pointer leak in sock_ops"), so this reroll keeps
only the remaining work for ctx->rtt_min.

Patch 1 extracts the guarded field-load sequence out of
SOCK_OPS_GET_FIELD() so the rtt_min sub-field access can reuse the same
is_locked_tcp_sock guard and dst_reg == src_reg handling without
open-coding it again.

Patch 2 uses that helper for rtt_min and extends the landed
sock_ops_get_sk selftest with an rtt_min subtest covering the
request_sock-backed !fullsock path.

Changes since v2:
- drop the overlapping dst_reg == src_reg fix, now upstream as
  10f86a2a5c91
- keep only the helper extraction plus the rtt_min fix
- add an rtt_min subtest on top of the landed sock_ops_get_sk selftest

Werner Kasselman (2):
  bpf: extract SOCK_OPS_LOAD_TCP_SOCK_FIELD from SOCK_OPS_GET_FIELD
  bpf: guard sock_ops rtt_min against non-locked tcp_sock

 net/core/filter.c                             | 31 ++++++++++---------
 .../bpf/prog_tests/sock_ops_get_sk.c          |  9 ++++++
 .../selftests/bpf/progs/sock_ops_get_sk.c     | 31 +++++++++++++++++++
 3 files changed, 57 insertions(+), 14 deletions(-)

-- 
2.43.0

[PATCH 1/2] bpf: extract SOCK_OPS_LOAD_TCP_SOCK_FIELD from SOCK_OPS_GET_FIELD

[Werner Kasselman]

Extract the guarded sock_ops field load sequence out of
SOCK_OPS_GET_FIELD() into SOCK_OPS_LOAD_TCP_SOCK_FIELD(FIELD_SIZE,
FIELD_OFFSET) so callers that need a custom field offset can reuse the
same is_locked_tcp_sock guard and dst_reg == src_reg handling.

This is needed for rtt_min, which reads a minmax sub-field rather than a
direct struct member.

No functional change.

Signed-off-by: Werner Kasselman <werner@verivus.com>
---
 net/core/filter.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index fcfcb72663ca..e8ad062f63bc 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -10535,12 +10535,10 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 	struct bpf_insn *insn = insn_buf;
 	int off;
 
-/* Helper macro for adding read access to tcp_sock or sock fields. */
-#define SOCK_OPS_GET_FIELD(BPF_FIELD, OBJ_FIELD, OBJ)			      \
+/* Helper macro for adding guarded read access to tcp_sock fields. */
+#define SOCK_OPS_LOAD_TCP_SOCK_FIELD(FIELD_SIZE, FIELD_OFFSET)		      \
 	do {								      \
 		int fullsock_reg = si->dst_reg, reg = BPF_REG_9, jmp = 2;     \
-		BUILD_BUG_ON(sizeof_field(OBJ, OBJ_FIELD) >		      \
-			     sizeof_field(struct bpf_sock_ops, BPF_FIELD));   \
 		if (si->dst_reg == reg || si->src_reg == reg)		      \
 			reg--;						      \
 		if (si->dst_reg == reg || si->src_reg == reg)		      \
@@ -10567,10 +10565,9 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 						struct bpf_sock_ops_kern, sk),\
 				      si->dst_reg, si->src_reg,		      \
 				      offsetof(struct bpf_sock_ops_kern, sk));\
-		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(OBJ,		      \
-						       OBJ_FIELD),	      \
+		*insn++ = BPF_LDX_MEM(FIELD_SIZE,			      \
 				      si->dst_reg, si->dst_reg,		      \
-				      offsetof(OBJ, OBJ_FIELD));	      \
+				      FIELD_OFFSET);			      \
 		if (si->dst_reg == si->src_reg)	{			      \
 			*insn++ = BPF_JMP_A(2);				      \
 			*insn++ = BPF_LDX_MEM(BPF_DW, reg, si->src_reg,	      \
@@ -10580,6 +10577,14 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 		}							      \
 	} while (0)
 
+#define SOCK_OPS_GET_FIELD(BPF_FIELD, OBJ_FIELD, OBJ)			      \
+	do {								      \
+		BUILD_BUG_ON(sizeof_field(OBJ, OBJ_FIELD) >		      \
+			     sizeof_field(struct bpf_sock_ops, BPF_FIELD));   \
+		SOCK_OPS_LOAD_TCP_SOCK_FIELD(BPF_FIELD_SIZEOF(OBJ, OBJ_FIELD),\
+					     offsetof(OBJ, OBJ_FIELD));       \
+	} while (0)
+
 #define SOCK_OPS_GET_SK()							      \
 	do {								      \
 		int fullsock_reg = si->dst_reg, reg = BPF_REG_9, jmp = 1;     \
-- 
2.43.0

[PATCH 2/2] bpf: guard sock_ops rtt_min against non-locked tcp_sock

[Werner Kasselman]

sock_ops_convert_ctx_access() reads rtt_min without the
is_locked_tcp_sock guard used for every other tcp_sock field. On
request_sock-backed sock_ops callbacks, sk points at a
tcp_request_sock and the converted load reads past the end of the
allocation.

Reuse SOCK_OPS_LOAD_TCP_SOCK_FIELD() for the rtt_min access and compute
the offset with offsetof(struct minmax_sample, v). This leaves the byte
addressed unchanged from the old sizeof_field(struct minmax_sample, t)
expression, while making rtt_min consistent with every other tcp_sock
field.

This also picks up the same dst_reg == src_reg handling used by the
other guarded field loads. Extend the sock_ops_get_sk selftest with an
rtt_min subtest that checks request_sock-backed !fullsock callbacks read
zero instead of leaking request_sock-adjacent memory.

Found via AST-based call-graph analysis using sqry.

Fixes: 44f0e43037d3 ("bpf: Add support for reading sk_state and more")
Cc: stable@vger.kernel.org
Signed-off-by: Werner Kasselman <werner@verivus.com>
---
 net/core/filter.c                             | 12 +++----
 .../bpf/prog_tests/sock_ops_get_sk.c          |  9 ++++++
 .../selftests/bpf/progs/sock_ops_get_sk.c     | 31 +++++++++++++++++++
 3 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index e8ad062f63bc..9c43193a5c39 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -10827,14 +10827,12 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 			     sizeof(struct minmax));
 		BUILD_BUG_ON(sizeof(struct minmax) <
 			     sizeof(struct minmax_sample));
+		BUILD_BUG_ON(offsetof(struct tcp_sock, rtt_min) +
+			     offsetof(struct minmax_sample, v) > S16_MAX);
 
-		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(
-						struct bpf_sock_ops_kern, sk),
-				      si->dst_reg, si->src_reg,
-				      offsetof(struct bpf_sock_ops_kern, sk));
-		*insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->dst_reg,
-				      offsetof(struct tcp_sock, rtt_min) +
-				      sizeof_field(struct minmax_sample, t));
+		off = offsetof(struct tcp_sock, rtt_min) +
+		      offsetof(struct minmax_sample, v);
+		SOCK_OPS_LOAD_TCP_SOCK_FIELD(BPF_W, off);
 		break;
 
 	case offsetof(struct bpf_sock_ops, bpf_sock_ops_cb_flags):
diff --git a/tools/testing/selftests/bpf/prog_tests/sock_ops_get_sk.c b/tools/testing/selftests/bpf/prog_tests/sock_ops_get_sk.c
index 343d92c4df30..1aea4c97d5d3 100644
--- a/tools/testing/selftests/bpf/prog_tests/sock_ops_get_sk.c
+++ b/tools/testing/selftests/bpf/prog_tests/sock_ops_get_sk.c
@@ -70,6 +70,15 @@ void test_ns_sock_ops_get_sk(void)
 		ASSERT_EQ(skel->bss->diff_reg_bug_detected, 0, "diff_reg_bug_not_detected");
 	}
 
+	/* Test sock_ops rtt_min access in !fullsock callbacks */
+	if (test__start_subtest("get_rtt_min")) {
+		run_sock_ops_test(cgroup_fd,
+				  bpf_program__fd(skel->progs.sock_ops_get_rtt_min));
+		ASSERT_EQ(skel->bss->rtt_min_null_seen, 1, "rtt_min_null_seen");
+		ASSERT_EQ(skel->bss->rtt_min_bug_detected, 0,
+			  "rtt_min_bug_not_detected");
+	}
+
 	sock_ops_get_sk__destroy(skel);
 close_cgroup:
 	close(cgroup_fd);
diff --git a/tools/testing/selftests/bpf/progs/sock_ops_get_sk.c b/tools/testing/selftests/bpf/progs/sock_ops_get_sk.c
index 3a0689f8ce7c..dee07da8901e 100644
--- a/tools/testing/selftests/bpf/progs/sock_ops_get_sk.c
+++ b/tools/testing/selftests/bpf/progs/sock_ops_get_sk.c
@@ -114,4 +114,35 @@ __naked void sock_ops_get_sk_diff_reg(void)
 		: __clobber_all);
 }
 
+/* sock_ops rtt_min access: different-register, is_locked_tcp_sock == 0 path (TCP_NEW_SYN_RECV). */
+int rtt_min_bug_detected;
+int rtt_min_null_seen;
+
+SEC("sockops")
+__naked void sock_ops_get_rtt_min(void)
+{
+	asm volatile (
+		"r7 = *(u32 *)(r1 + %[is_fullsock_off]);"
+		"r2 = *(u32 *)(r1 + %[rtt_min_off]);"
+		"if r7 != 0 goto 2f;"
+		"if r2 == 0 goto 1f;"
+		"r1 = %[rtt_min_bug_detected] ll;"
+		"r3 = 1;"
+		"*(u32 *)(r1 + 0) = r3;"
+		"goto 2f;"
+	"1:"
+		"r1 = %[rtt_min_null_seen] ll;"
+		"r3 = 1;"
+		"*(u32 *)(r1 + 0) = r3;"
+	"2:"
+		"r0 = 1;"
+		"exit;"
+		:
+		: __imm_const(is_fullsock_off, offsetof(struct bpf_sock_ops, is_fullsock)),
+		  __imm_const(rtt_min_off, offsetof(struct bpf_sock_ops, rtt_min)),
+		  __imm_addr(rtt_min_bug_detected),
+		  __imm_addr(rtt_min_null_seen)
+		: __clobber_all);
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.43.0

Re: [PATCH 2/2] bpf: guard sock_ops rtt_min against non-locked tcp_sock

[Martin KaFai Lau]

On Fri, Apr 17, 2026 at 02:31:26AM +0000, Werner Kasselman wrote:
> diff --git a/net/core/filter.c b/net/core/filter.c
> index e8ad062f63bc..9c43193a5c39 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -10827,14 +10827,12 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
>  			     sizeof(struct minmax));
>  		BUILD_BUG_ON(sizeof(struct minmax) <
>  			     sizeof(struct minmax_sample));
> +		BUILD_BUG_ON(offsetof(struct tcp_sock, rtt_min) +
> +			     offsetof(struct minmax_sample, v) > S16_MAX);

This doesn't look like a test that is added by human.
Will sizeof(tcp_sock) ever reach S16_MAX? It is unnecessarily defensive and
inconsistent with other tcp_sock field loads.

> diff --git a/tools/testing/selftests/bpf/prog_tests/sock_ops_get_sk.c b/tools/testing/selftests/bpf/prog_tests/sock_ops_get_sk.c
> index 343d92c4df30..1aea4c97d5d3 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sock_ops_get_sk.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sock_ops_get_sk.c

Separate the test in its own patch.

Also tag and add revision to subject, "[PATCH v3 bpf...]".
Take a look at how other patches are posted in the bpf mailing list.

pw-bot: cr

[PATCH bpf v4 1/2] bpf: guard sock_ops rtt_min against non-locked tcp_sock

[Werner Kasselman]

sock_ops_convert_ctx_access() reads rtt_min without the
is_locked_tcp_sock guard used for every other tcp_sock field. On
request_sock-backed sock_ops callbacks, sk points at a
tcp_request_sock and the converted load reads past the end of the
allocation.

Extract the guarded tcp_sock field load sequence into
SOCK_OPS_LOAD_TCP_SOCK_FIELD() and use it for the rtt_min access after
computing the sub-field offset with offsetof(struct minmax_sample, v).
Reusing the shared helper keeps rtt_min aligned with the other guarded
tcp_sock field loads and preserves the dst_reg == src_reg failure path
that zeros the destination register when the guard fails.

Found via AST-based call-graph analysis using sqry.

Fixes: 44f0e43037d3 ("bpf: Add support for reading sk_state and more")
Cc: stable@vger.kernel.org
Signed-off-by: Werner Kasselman <werner@verivus.com>
---
net/core/filter.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 78b548158fb0..b60f279c004a 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -10544,12 +10544,10 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 	struct bpf_insn *insn = insn_buf;
 	int off;
 
-/* Helper macro for adding read access to tcp_sock or sock fields. */
-#define SOCK_OPS_GET_FIELD(BPF_FIELD, OBJ_FIELD, OBJ)			      \
+/* Helper macro for adding guarded read access to tcp_sock fields. */
+#define SOCK_OPS_LOAD_TCP_SOCK_FIELD(FIELD_SIZE, FIELD_OFFSET)		      \
 	do {								      \
 		int fullsock_reg = si->dst_reg, reg = BPF_REG_9, jmp = 2;     \
-		BUILD_BUG_ON(sizeof_field(OBJ, OBJ_FIELD) >		      \
-			     sizeof_field(struct bpf_sock_ops, BPF_FIELD));   \
 		if (si->dst_reg == reg || si->src_reg == reg)		      \
 			reg--;						      \
 		if (si->dst_reg == reg || si->src_reg == reg)		      \
@@ -10557,7 +10555,7 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 		if (si->dst_reg == si->src_reg) {			      \
 			*insn++ = BPF_STX_MEM(BPF_DW, si->src_reg, reg,	      \
 					  offsetof(struct bpf_sock_ops_kern,  \
-					  temp));			      \
+						   temp));		      \
 			fullsock_reg = reg;				      \
 			jmp += 2;					      \
 		}							      \
@@ -10571,23 +10569,31 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 		if (si->dst_reg == si->src_reg)				      \
 			*insn++ = BPF_LDX_MEM(BPF_DW, reg, si->src_reg,	      \
 				      offsetof(struct bpf_sock_ops_kern,      \
-				      temp));				      \
+					       temp));			      \
 		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(			      \
 						struct bpf_sock_ops_kern, sk),\
 				      si->dst_reg, si->src_reg,		      \
 				      offsetof(struct bpf_sock_ops_kern, sk));\
-		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(OBJ,		      \
-						       OBJ_FIELD),	      \
+		*insn++ = BPF_LDX_MEM(FIELD_SIZE,			      \
 				      si->dst_reg, si->dst_reg,		      \
-				      offsetof(OBJ, OBJ_FIELD));	      \
+				      FIELD_OFFSET);			      \
 		if (si->dst_reg == si->src_reg)	{			      \
-			*insn++ = BPF_JMP_A(1);				      \
+			*insn++ = BPF_JMP_A(2);				      \
 			*insn++ = BPF_LDX_MEM(BPF_DW, reg, si->src_reg,	      \
 				      offsetof(struct bpf_sock_ops_kern,      \
-				      temp));				      \
+					       temp));			      \
+			*insn++ = BPF_MOV64_IMM(si->dst_reg, 0);	      \
 		}							      \
 	} while (0)
 
+#define SOCK_OPS_GET_FIELD(BPF_FIELD, OBJ_FIELD, OBJ)			      \
+	do {								      \
+		BUILD_BUG_ON(sizeof_field(OBJ, OBJ_FIELD) >		      \
+			     sizeof_field(struct bpf_sock_ops, BPF_FIELD));   \
+		SOCK_OPS_LOAD_TCP_SOCK_FIELD(BPF_FIELD_SIZEOF(OBJ, OBJ_FIELD),\
+					     offsetof(OBJ, OBJ_FIELD));       \
+	} while (0)
+
 #define SOCK_OPS_GET_SK()							      \
 	do {								      \
 		int fullsock_reg = si->dst_reg, reg = BPF_REG_9, jmp = 1;     \
@@ -10829,14 +10835,9 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 			     sizeof(struct minmax));
 		BUILD_BUG_ON(sizeof(struct minmax) <
 			     sizeof(struct minmax_sample));
-
-		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(
-						struct bpf_sock_ops_kern, sk),
-				      si->dst_reg, si->src_reg,
-				      offsetof(struct bpf_sock_ops_kern, sk));
-		*insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->dst_reg,
-				      offsetof(struct tcp_sock, rtt_min) +
-				      sizeof_field(struct minmax_sample, t));
+		off = offsetof(struct tcp_sock, rtt_min) +
+		      offsetof(struct minmax_sample, v);
+		SOCK_OPS_LOAD_TCP_SOCK_FIELD(BPF_W, off);
 		break;
 
 	case offsetof(struct bpf_sock_ops, bpf_sock_ops_cb_flags):
-- 
2.43.0

[PATCH bpf v4 2/2] selftests/bpf: cover same-reg sock_ops rtt_min request_sock access

[Werner Kasselman]

Add a tcpbpf sock_ops selftest that forces a same-register
ctx->rtt_min read on request_sock-backed callbacks and verifies the
observed value is zero.

This covers the dst_reg == src_reg path that the previous
ctx_rewrite-only test did not exercise.

Signed-off-by: Werner Kasselman <werner@verivus.com>
---
 .../testing/selftests/bpf/prog_tests/tcpbpf_user.c |  4 ++++
 .../testing/selftests/bpf/progs/test_tcpbpf_kern.c | 14 ++++++++++++++
 tools/testing/selftests/bpf/test_tcpbpf.h          |  2 ++
 3 files changed, 20 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c b/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c
index 7e8fe1bad03f..1b08e49327d0 100644
--- a/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c
+++ b/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c
@@ -42,6 +42,10 @@ static void verify_result(struct tcpbpf_globals *result)
 	/* check getsockopt for window_clamp */
 	ASSERT_EQ(result->window_clamp_client, 9216, "window_clamp_client");
 	ASSERT_EQ(result->window_clamp_server, 9216, "window_clamp_server");
+
+	/* check same-reg rtt_min read on request_sock-backed callbacks */
+	ASSERT_NEQ(result->rtt_min_req_seen, 0, "rtt_min_req_seen");
+	ASSERT_EQ(result->rtt_min_req_nonzero, 0, "rtt_min_req_nonzero");
 }
 
 static void run_test(struct tcpbpf_globals *result)
diff --git a/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c b/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c
index 6935f32eeb8f..a488b282b5dd 100644
--- a/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c
+++ b/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c
@@ -33,6 +33,7 @@ int bpf_testcb(struct bpf_sock_ops *skops)
 {
 	char header[sizeof(struct ipv6hdr) + sizeof(struct tcphdr)];
 	struct bpf_sock_ops *reuse = skops;
+	long rtt_min = (long)skops;
 	struct tcphdr *thdr;
 	int window_clamp = 9216;
 	int save_syn = 1;
@@ -84,6 +85,19 @@ int bpf_testcb(struct bpf_sock_ops *skops)
 
 	global.event_map |= (1 << op);
 
+	if (!skops->is_fullsock &&
+	    (op == BPF_SOCK_OPS_RWND_INIT || op == BPF_SOCK_OPS_NEEDS_ECN)) {
+		asm volatile (
+			"%[rtt_min] = *(u32 *)(%[rtt_min] + %[rtt_min_off]);\n"
+			: [rtt_min] "+r"(rtt_min)
+			: [rtt_min_off] "i"(offsetof(struct bpf_sock_ops, rtt_min))
+			:);
+
+		global.rtt_min_req_seen = 1;
+		if (rtt_min)
+			global.rtt_min_req_nonzero = 1;
+	}
+
 	switch (op) {
 	case BPF_SOCK_OPS_TCP_CONNECT_CB:
 		rv = bpf_setsockopt(skops, SOL_TCP, TCP_WINDOW_CLAMP,
diff --git a/tools/testing/selftests/bpf/test_tcpbpf.h b/tools/testing/selftests/bpf/test_tcpbpf.h
index 9dd9b5590f9d..e9806215cbc0 100644
--- a/tools/testing/selftests/bpf/test_tcpbpf.h
+++ b/tools/testing/selftests/bpf/test_tcpbpf.h
@@ -18,5 +18,7 @@ struct tcpbpf_globals {
 	__u32 tcp_saved_syn;
 	__u32 window_clamp_client;
 	__u32 window_clamp_server;
+	__u32 rtt_min_req_seen;
+	__u32 rtt_min_req_nonzero;
 };
 #endif
-- 
2.43.0

[PATCH bpf v5 1/2] bpf: guard sock_ops rtt_min against non-locked tcp_sock

[Werner Kasselman]

sock_ops_convert_ctx_access() reads rtt_min without the is_locked_tcp_sock guard used for every other tcp_sock field. On request_sock-backed sock_ops callbacks, sk points at a tcp_request_sock and the converted load reads past the end of the allocation.

Extract the guarded tcp_sock field load sequence into SOCK_OPS_LOAD_TCP_SOCK_FIELD() and use it for the rtt_min access after computing the sub-field offset with offsetof(struct minmax_sample, v). Reusing the shared helper keeps rtt_min aligned with the other guarded tcp_sock field loads and preserves the dst_reg == src_reg failure path that zeros the destination register when the guard fails.

Found via AST-based call-graph analysis using sqry.

Fixes: 44f0e43037d3 ("bpf: Add support for reading sk_state and more")
Cc: stable@vger.kernel.org
Signed-off-by: Werner Kasselman <werner@verivus.com>
---
 net/core/filter.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index fcfcb72663ca..2e7c33d00749 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -10535,12 +10535,10 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 	struct bpf_insn *insn = insn_buf;
 	int off;
 
-/* Helper macro for adding read access to tcp_sock or sock fields. */
-#define SOCK_OPS_GET_FIELD(BPF_FIELD, OBJ_FIELD, OBJ)			      \
+/* Helper macro for adding guarded read access to tcp_sock fields. */
+#define SOCK_OPS_LOAD_TCP_SOCK_FIELD(FIELD_SIZE, FIELD_OFFSET)		      \
 	do {								      \
 		int fullsock_reg = si->dst_reg, reg = BPF_REG_9, jmp = 2;     \
-		BUILD_BUG_ON(sizeof_field(OBJ, OBJ_FIELD) >		      \
-			     sizeof_field(struct bpf_sock_ops, BPF_FIELD));   \
 		if (si->dst_reg == reg || si->src_reg == reg)		      \
 			reg--;						      \
 		if (si->dst_reg == reg || si->src_reg == reg)		      \
@@ -10548,7 +10546,7 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 		if (si->dst_reg == si->src_reg) {			      \
 			*insn++ = BPF_STX_MEM(BPF_DW, si->src_reg, reg,	      \
 					  offsetof(struct bpf_sock_ops_kern,  \
-					  temp));			      \
+						   temp));		      \
 			fullsock_reg = reg;				      \
 			jmp += 2;					      \
 		}							      \
@@ -10562,24 +10560,31 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 		if (si->dst_reg == si->src_reg)				      \
 			*insn++ = BPF_LDX_MEM(BPF_DW, reg, si->src_reg,	      \
 				      offsetof(struct bpf_sock_ops_kern,      \
-				      temp));				      \
+					       temp));			      \
 		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(			      \
 						struct bpf_sock_ops_kern, sk),\
 				      si->dst_reg, si->src_reg,		      \
 				      offsetof(struct bpf_sock_ops_kern, sk));\
-		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(OBJ,		      \
-						       OBJ_FIELD),	      \
+		*insn++ = BPF_LDX_MEM(FIELD_SIZE,			      \
 				      si->dst_reg, si->dst_reg,		      \
-				      offsetof(OBJ, OBJ_FIELD));	      \
+				      FIELD_OFFSET);			      \
 		if (si->dst_reg == si->src_reg)	{			      \
 			*insn++ = BPF_JMP_A(2);				      \
 			*insn++ = BPF_LDX_MEM(BPF_DW, reg, si->src_reg,	      \
 				      offsetof(struct bpf_sock_ops_kern,      \
-				      temp));				      \
+					       temp));			      \
 			*insn++ = BPF_MOV64_IMM(si->dst_reg, 0);	      \
 		}							      \
 	} while (0)
 
+#define SOCK_OPS_GET_FIELD(BPF_FIELD, OBJ_FIELD, OBJ)			      \
+	do {								      \
+		BUILD_BUG_ON(sizeof_field(OBJ, OBJ_FIELD) >		      \
+			     sizeof_field(struct bpf_sock_ops, BPF_FIELD));   \
+		SOCK_OPS_LOAD_TCP_SOCK_FIELD(BPF_FIELD_SIZEOF(OBJ, OBJ_FIELD),\
+					     offsetof(OBJ, OBJ_FIELD));       \
+	} while (0)
+
 #define SOCK_OPS_GET_SK()							      \
 	do {								      \
 		int fullsock_reg = si->dst_reg, reg = BPF_REG_9, jmp = 1;     \
@@ -10822,14 +10827,9 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 			     sizeof(struct minmax));
 		BUILD_BUG_ON(sizeof(struct minmax) <
 			     sizeof(struct minmax_sample));
-
-		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(
-						struct bpf_sock_ops_kern, sk),
-				      si->dst_reg, si->src_reg,
-				      offsetof(struct bpf_sock_ops_kern, sk));
-		*insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->dst_reg,
-				      offsetof(struct tcp_sock, rtt_min) +
-				      sizeof_field(struct minmax_sample, t));
+		off = offsetof(struct tcp_sock, rtt_min) +
+		      offsetof(struct minmax_sample, v);
+		SOCK_OPS_LOAD_TCP_SOCK_FIELD(BPF_W, off);
 		break;
 
 	case offsetof(struct bpf_sock_ops, bpf_sock_ops_cb_flags):
-- 
2.43.0

[PATCH bpf v5 2/2] selftests/bpf: cover same-reg sock_ops rtt_min request_sock access

[Werner Kasselman]

Add a tcpbpf sock_ops selftest that forces a same-register ctx->rtt_min read on request_sock-backed callbacks and verifies the observed value is zero.

This covers the dst_reg == src_reg path that the previous ctx_rewrite-only test did not exercise.

Signed-off-by: Werner Kasselman <werner@verivus.com>
---
 .../testing/selftests/bpf/prog_tests/tcpbpf_user.c |  4 ++++
 .../testing/selftests/bpf/progs/test_tcpbpf_kern.c | 14 ++++++++++++++
 tools/testing/selftests/bpf/test_tcpbpf.h          |  2 ++
 3 files changed, 20 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c b/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c
index 7e8fe1bad03f..1b08e49327d0 100644
--- a/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c
+++ b/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c
@@ -42,6 +42,10 @@ static void verify_result(struct tcpbpf_globals *result)
 	/* check getsockopt for window_clamp */
 	ASSERT_EQ(result->window_clamp_client, 9216, "window_clamp_client");
 	ASSERT_EQ(result->window_clamp_server, 9216, "window_clamp_server");
+
+	/* check same-reg rtt_min read on request_sock-backed callbacks */
+	ASSERT_NEQ(result->rtt_min_req_seen, 0, "rtt_min_req_seen");
+	ASSERT_EQ(result->rtt_min_req_nonzero, 0, "rtt_min_req_nonzero");
 }
 
 static void run_test(struct tcpbpf_globals *result)
diff --git a/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c b/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c
index 6935f32eeb8f..a488b282b5dd 100644
--- a/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c
+++ b/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c
@@ -33,6 +33,7 @@ int bpf_testcb(struct bpf_sock_ops *skops)
 {
 	char header[sizeof(struct ipv6hdr) + sizeof(struct tcphdr)];
 	struct bpf_sock_ops *reuse = skops;
+	long rtt_min = (long)skops;
 	struct tcphdr *thdr;
 	int window_clamp = 9216;
 	int save_syn = 1;
@@ -84,6 +85,19 @@ int bpf_testcb(struct bpf_sock_ops *skops)
 
 	global.event_map |= (1 << op);
 
+	if (!skops->is_fullsock &&
+	    (op == BPF_SOCK_OPS_RWND_INIT || op == BPF_SOCK_OPS_NEEDS_ECN)) {
+		asm volatile (
+			"%[rtt_min] = *(u32 *)(%[rtt_min] + %[rtt_min_off]);\n"
+			: [rtt_min] "+r"(rtt_min)
+			: [rtt_min_off] "i"(offsetof(struct bpf_sock_ops, rtt_min))
+			:);
+
+		global.rtt_min_req_seen = 1;
+		if (rtt_min)
+			global.rtt_min_req_nonzero = 1;
+	}
+
 	switch (op) {
 	case BPF_SOCK_OPS_TCP_CONNECT_CB:
 		rv = bpf_setsockopt(skops, SOL_TCP, TCP_WINDOW_CLAMP,
diff --git a/tools/testing/selftests/bpf/test_tcpbpf.h b/tools/testing/selftests/bpf/test_tcpbpf.h
index 9dd9b5590f9d..e9806215cbc0 100644
--- a/tools/testing/selftests/bpf/test_tcpbpf.h
+++ b/tools/testing/selftests/bpf/test_tcpbpf.h
@@ -18,5 +18,7 @@ struct tcpbpf_globals {
 	__u32 tcp_saved_syn;
 	__u32 window_clamp_client;
 	__u32 window_clamp_server;
+	__u32 rtt_min_req_seen;
+	__u32 rtt_min_req_nonzero;
 };
 #endif
-- 
2.43.0