[PATCH v2 0/3] bpf: fix sock_ops rtt_min OOB read and related guard issues

[PATCH v2 0/3] bpf: fix sock_ops rtt_min OOB read and related guard issues

[Werner Kasselman]

Patch 3 fixes an out-of-bounds read in sock_ops_convert_ctx_access()
for the rtt_min context field. It is the only tcp_sock-backed field
that bypasses the is_locked_tcp_sock guard, so on request_sock-backed
sock_ops callbacks the converted BPF load reads past the end of a
tcp_request_sock.

Patches 1 and 2 are groundwork. Patch 1 fixes a pre-existing info
leak in SOCK_OPS_GET_FIELD() and SOCK_OPS_GET_SK() where dst_reg is
left holding the context pointer on the guard-failure branch when
dst_reg == src_reg, instead of being zeroed. Patch 2 extracts
SOCK_OPS_LOAD_TCP_SOCK_FIELD() from SOCK_OPS_GET_FIELD() so the
rtt_min sub-field access in patch 3 can reuse it.

Patches 1 and 3 carry Fixes: tags and Cc: stable. Patch 2 is a pure
refactor.

v1: https://lore.kernel.org/bpf/ (earlier single-patch posting)
  - Inlined the guarded load sequence by hand.
  - Feedback: please factor it through the existing helper instead
    of open-coding 30 lines.

v2:
  - Patch 1 (new): fix latent dst == src info leak in both macros.
  - Patch 2 (new): refactor SOCK_OPS_GET_FIELD().
  - Patch 3: use SOCK_OPS_LOAD_TCP_SOCK_FIELD() for rtt_min and use
    offsetof(struct minmax_sample, v) for the sub-field offset.

Werner Kasselman (3):
  bpf: zero dst_reg on sock_ops field guard failure when dst == src
  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 | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

-- 
2.43.0

[PATCH v2 1/3] bpf: zero dst_reg on sock_ops field guard failure when dst == src

[Werner Kasselman]

When a BPF_PROG_TYPE_SOCK_OPS program reads a tcp_sock-backed context
field (e.g. ctx->snd_ssthresh) or ctx->sk using the same register for
source and destination, SOCK_OPS_GET_FIELD() and SOCK_OPS_GET_SK()
load is_locked_tcp_sock/is_fullsock into a scratch register rather
than into dst_reg. On the guard-failure branch the macro only
restores the scratch register before falling through, leaving
dst_reg holding the unchanged context pointer.

Callers expect dst_reg to read as a scalar 0 when the guard fails.
Instead the BPF program sees a kernel heap address, which the
verifier has already typed as a scalar, giving a narrow kernel
pointer leak. Clang does not emit the dst == src pattern for normal
C ctx field reads, but it is reachable via inline asm and
hand-written BPF.

Add an explicit BPF_MOV64_IMM(dst_reg, 0) on the failure path in
both macros and bump the success-path BPF_JMP_A() to skip over it.

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

Fixes: fd09af010788 ("bpf: sock_ops ctx access may stomp registers in corner case")
Fixes: 84f44df664e9 ("bpf: sock_ops sk access may stomp registers when dst_reg = src_reg")
Cc: stable@vger.kernel.org
Signed-off-by: Werner Kasselman <werner@verivus.com>
---
 net/core/filter.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 78b548158fb0..53ce06ed4a88 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -10581,10 +10581,11 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 				      si->dst_reg, si->dst_reg,		      \
 				      offsetof(OBJ, OBJ_FIELD));	      \
 		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));				      \
+			*insn++ = BPF_MOV64_IMM(si->dst_reg, 0);	      \
 		}							      \
 	} while (0)
 
@@ -10618,10 +10619,11 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 				      si->dst_reg, si->src_reg,		      \
 				      offsetof(struct bpf_sock_ops_kern, sk));\
 		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));				      \
+			*insn++ = BPF_MOV64_IMM(si->dst_reg, 0);	      \
 		}							      \
 	} while (0)
 
-- 
2.43.0

[PATCH v2 2/3] bpf: extract SOCK_OPS_LOAD_TCP_SOCK_FIELD from SOCK_OPS_GET_FIELD

[Werner Kasselman]

Split the tcp_sock field load sequence out of SOCK_OPS_GET_FIELD()
into SOCK_OPS_LOAD_TCP_SOCK_FIELD(FIELD_SIZE, FIELD_OFFSET) so it can
be reused for fields that are not direct struct members.

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 53ce06ed4a88..385fc3e9eb4a 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)		      \
@@ -10576,10 +10574,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,	      \
@@ -10589,6 +10586,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 v2 3/3] 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.

Use SOCK_OPS_LOAD_TCP_SOCK_FIELD() so the load is guarded, and
compute the offset via offsetof(struct minmax_sample, v).

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 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 385fc3e9eb4a..88fa290caeaa 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -10836,14 +10836,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):
-- 
2.43.0

Re: [PATCH v2 0/3] bpf: fix sock_ops rtt_min OOB read and related guard issues

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sun, 12 Apr 2026 03:03:08 +0000 you wrote:
> Patch 3 fixes an out-of-bounds read in sock_ops_convert_ctx_access()
> for the rtt_min context field. It is the only tcp_sock-backed field
> that bypasses the is_locked_tcp_sock guard, so on request_sock-backed
> sock_ops callbacks the converted BPF load reads past the end of a
> tcp_request_sock.
> 
> Patches 1 and 2 are groundwork. Patch 1 fixes a pre-existing info
> leak in SOCK_OPS_GET_FIELD() and SOCK_OPS_GET_SK() where dst_reg is
> left holding the context pointer on the guard-failure branch when
> dst_reg == src_reg, instead of being zeroed. Patch 2 extracts
> SOCK_OPS_LOAD_TCP_SOCK_FIELD() from SOCK_OPS_GET_FIELD() so the
> rtt_min sub-field access in patch 3 can reuse it.
> 
> [...]

Here is the summary with links:
  - [v2,1/3] bpf: zero dst_reg on sock_ops field guard failure when dst == src
    https://git.kernel.org/netdev/net/c/10f86a2a5c91
  - [v2,2/3] bpf: extract SOCK_OPS_LOAD_TCP_SOCK_FIELD from SOCK_OPS_GET_FIELD
    (no matching commit)
  - [v2,3/3] bpf: guard sock_ops rtt_min against non-locked tcp_sock
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html