Re: [PATCH bpf-next v4 11/15] bpf: tcp: Support selected sock_ops callbacks as struct_ops
From: bot+bpf-ci@kernel.org
Date: 2026-09-17 21:34:16
Also in:
bpf
quoted hunk ↗ jump to hunk
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index f34b410f903ec..3abde9a2a375e 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h@@ -2634,6 +2634,7 @@ struct bpf_trace_run_ctx { struct bpf_tramp_run_ctx { struct bpf_run_ctx run_ctx; u64 bpf_cookie; + int retval; struct bpf_run_ctx *saved_run_ctx; };
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/net/ipv4/bpf_tcp_ops.c b/net/ipv4/bpf_tcp_ops.c new file mode 100644 index 0000000000000..3febbc8dd1a06 --- /dev/null +++ b/net/ipv4/bpf_tcp_ops.c@@ -0,0 +1,188 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */ + +#include <linux/bpf.h> +#include <linux/btf_ids.h> +#include <linux/bpf_verifier.h> +#include <net/bpf_sk_storage.h> +#include <net/tcp.h> + +static int timeout_init_stub(struct sock *sk, struct request_sock *req__nullable) +{ + struct bpf_tramp_run_ctx *ctx = + container_of(current->bpf_ctx, struct bpf_tramp_run_ctx, run_ctx); + + return ctx->retval; +} + +static int rwnd_init_stub(struct sock *sk, struct request_sock *req__nullable) +{ + struct bpf_tramp_run_ctx *ctx = + container_of(current->bpf_ctx, struct bpf_tramp_run_ctx, run_ctx); + + return ctx->retval; +}
[ ... ]
+BPF_CALL_0(bpf_tcp_ops_get_retval)
+{
+ struct bpf_tramp_run_ctx *ctx =
+ container_of(current->bpf_ctx, struct bpf_tramp_run_ctx, run_ctx);
+
+ /* bpf_get_retval() is only exposed to timeout_init/rwnd_init, which
+ * always run via bpf_tcp_ops_call_int(). Its run_ctx carries the int
+ * return value chained across the bpf_tcp_ops attached to the cgroup
+ * and is this program's saved_run_ctx.
+ */
+ if (WARN_ON_ONCE(!ctx->saved_run_ctx))
+ return 0;
+
+ return container_of(ctx->saved_run_ctx, struct bpf_tramp_run_ctx,
+ run_ctx)->retval;
+}
+
+const struct bpf_func_proto bpf_tcp_ops_get_retval_proto = {
+ .func = bpf_tcp_ops_get_retval,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+};
+
+static const struct bpf_func_proto *
+get_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+ u32 moff = prog->aux->attach_st_ops_member_off;
+
+ switch (func_id) {
+ case BPF_FUNC_sk_storage_get:
+ return &bpf_sk_storage_get_proto;
+ case BPF_FUNC_sk_storage_delete:
+ return &bpf_sk_storage_delete_proto;
+ case BPF_FUNC_setsockopt:
+ /* The listener is not locked. */
+ if (moff == offsetof(struct bpf_tcp_ops, rwnd_init) ||
+ moff == offsetof(struct bpf_tcp_ops, timeout_init))
+ return NULL;
+ return &bpf_sk_setsockopt_proto;
+ case BPF_FUNC_getsockopt:
+ if (moff == offsetof(struct bpf_tcp_ops, rwnd_init) ||
+ moff == offsetof(struct bpf_tcp_ops, timeout_init))
+ return NULL;
+ return &bpf_sk_getsockopt_proto;
+ case BPF_FUNC_get_retval:
+ if (moff == offsetof(struct bpf_tcp_ops, timeout_init) ||
+ moff == offsetof(struct bpf_tcp_ops, rwnd_init))
+ return &bpf_tcp_ops_get_retval_proto;
+ return NULL;
Does this expose BPF_FUNC_get_retval with incorrect verifier bounds?
Looking at kernel/bpf/verifier.c's do_refine_retval_range(), it hardcodes
the assumption that bpf_get_retval() returns an errno. For BPF_FUNC_get_retval
it only exempts BPF_PROG_TYPE_CGROUP_SOCKOPT/BPF_CGROUP_GETSOCKOPT and
BPF_PROG_TYPE_LSM/BPF_LSM_CGROUP. Every other prog type falls into the else
branch that sets range = [-MAX_ERRNO, 0] and then calls
reg_set_srange64()/reg_set_srange32() on R0, which *assign* the range rather
than intersect it.
The value bpf_tcp_ops_get_retval() actually returns is the chained int from
bpf_tramp_run_ctx.retval, which for these two members is positive by design:
tcp_timeout_init() seeds it with tcp_call_bpf(BPF_SOCK_OPS_TIMEOUT_INIT) (a
timeout in jiffies) and tcp_rwnd_init_bpf() seeds it with the receive window
in packets. Each earlier bpf_tcp_ops program in the cgroup chain overwrites
it with its own positive return value.
Two consequences:
1. Functional: a program that inspects the chained value with a positive test,
e.g. 'v = bpf_get_retval(); if (v > 0) return v; return my_default;', has
the 'v > 0' branch proven dead and hard-wired away by
opt_hard_wire_dead_code_branches(). The documented chaining semantics
silently stop working at runtime instead of failing to load.
2. Verifier soundness: because the bound is asserted not enforced, a program
can launder a large positive value into a register the verifier believes
is negative. For example:
long v = bpf_get_retval();
if (v < -100)
return 0;
arr[-v] = 1; /* out-of-bounds access */
This passes bounds checking with smin/smax in [-100, 0] while -v is a
large negative/positive value at runtime, giving an out-of-bounds map or
stack access.
Could this be addressed by either introducing a new helper with its own
func_id (so do_refine_retval_range() does not apply), or exempting
BPF_PROG_TYPE_STRUCT_OPS for BPF_FUNC_get_retval in do_refine_retval_range()?
+ default: + return bpf_base_func_proto(func_id, prog); + } +}
[ ... ] --- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md CI run summary: https://github.com/kernel-patches/bpf/actions/runs/35271288839