On Wed, Apr 8, 2026 at 11:10 PM Werner Kasselman <[email protected]> wrote: > > sock_ops_convert_ctx_access() emits guarded reads for tcp_sock-backed > bpf_sock_ops fields such as snd_cwnd, srtt_us, snd_ssthresh, rcv_nxt, > snd_nxt, snd_una, mss_cache, ecn_flags, rate_delivered, and > rate_interval_us. Those accesses go through SOCK_OPS_GET_TCP_SOCK_FIELD(), > which checks is_locked_tcp_sock before dereferencing sock_ops.sk. > > The rtt_min case is different. Because it reads a subfield of > struct minmax, it uses a custom open-coded load sequence instead of the > usual helper macro, and that sequence currently dereferences sock_ops.sk > without checking is_locked_tcp_sock first. > > This is unsafe when sock_ops.sk points to a request_sock-backed object > instead of a locked full tcp_sock. That is reachable not only from the > SYNACK header option callbacks, but also from other request_sock-backed > sock_ops callbacks such as BPF_SOCK_OPS_TIMEOUT_INIT, > BPF_SOCK_OPS_RWND_INIT, and BPF_SOCK_OPS_NEEDS_ECN. In those cases, > reading ctx->rtt_min makes the generated code treat a request_sock as a > tcp_sock and read beyond the end of the request_sock allocation. > > Fix the rtt_min conversion by adding the same is_locked_tcp_sock guard > used for the other tcp_sock field reads. Also make the accessed subfield > explicit by using offsetof(struct minmax_sample, v). > > Add a selftest that verifies request_sock-backed sock_ops callbacks see > ctx->rtt_min as zero after the fix. > > Found via AST-based call-graph analysis using sqry. > > Fixes: 44f0e43037d3 ("bpf: Add support for reading sk_state and more") > Cc: [email protected] > Signed-off-by: Werner Kasselman <[email protected]> > --- > net/core/filter.c | 53 +++++++++++++++---- > .../selftests/bpf/prog_tests/tcpbpf_user.c | 9 ++++ > .../selftests/bpf/progs/test_tcpbpf_kern.c | 21 ++++++++ > tools/testing/selftests/bpf/test_tcpbpf.h | 6 +++ > 4 files changed, 79 insertions(+), 10 deletions(-) > > diff --git a/net/core/filter.c b/net/core/filter.c > index 78b548158..5040bf7e4 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -10827,16 +10827,49 @@ static u32 sock_ops_convert_ctx_access(enum > bpf_access_type type, > case offsetof(struct bpf_sock_ops, rtt_min): > BUILD_BUG_ON(sizeof_field(struct tcp_sock, rtt_min) != > 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)); > + BUILD_BUG_ON(sizeof_field(struct bpf_sock_ops, rtt_min) != > + sizeof_field(struct minmax_sample, v)); > + off = offsetof(struct tcp_sock, rtt_min) + > + offsetof(struct minmax_sample, v); > + > + { > + int fullsock_reg = si->dst_reg, reg = BPF_REG_9, jmp > = 2; > +
please de-claude your patches before posting. pw-bot: cr
