* [PATCH bpf-next v6 0/7] Support associating BPF programs with struct_ops
@ 2025-11-14 22:17 Amery Hung
2025-11-14 22:17 ` [PATCH bpf-next v6 1/6] bpf: Allow verifier to fixup kernel module kfuncs Amery Hung
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: Amery Hung @ 2025-11-14 22:17 UTC (permalink / raw)
To: bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, tj, martin.lau,
ameryhung, kernel-team
Hi,
This patchset adds a new BPF command BPF_PROG_ASSOC_STRUCT_OPS to
the bpf() syscall to allow associating a BPF program with a struct_ops.
The command is introduced to address a emerging need from struct_ops
users. As the number of subsystems adopting struct_ops grows, more
users are building their struct_ops-based solution with some help from
other BPF programs. For example, scx_layer uses a syscall program as
a user space trigger to refresh layers [0]. It also uses tracing program
to infer whether a task is using GPU and needs to be prioritized [1]. In
these use cases, when there are multiple struct_ops instances, the
struct_ops kfuncs called from different BPF programs, whether struct_ops
or not needs to be able to refer to a specific one, which currently is
not possible.
The new BPF command will allow users to explicitly associate a BPF
program with a struct_ops map. The libbpf wrapper can be called after
loading programs and before attaching programs and struct_ops.
Internally, it will set prog->aux->st_ops_assoc to the struct_ops
map. struct_ops kfuncs can then get the associated struct_ops struct
by calling bpf_prog_get_assoc_struct_ops() with prog->aux, which can
be acquired from a "__prog" argument. The value of the special
argument will be fixed up by the verifier during verification.
The command conceptually associates the implementation of BPF programs
with struct_ops map, not the attachment. A program associated with the
map will take a refcount of it so that st_ops_assoc always points to a
valid struct_ops struct. struct_ops implementers can use the helper,
bpf_prog_get_assoc_struct_ops to get the pointer. The returned
struct_ops if not NULL is guaranteed to be valid and initialized.
However, it is not guaranteed that the struct_ops is attached. The
struct_ops implementer still need to take steps to track and check the
state of the struct_ops in kdata, if the use case demand the struct_ops
to be attached.
We can also consider support associating struct_ops link with BPF
programs, which on one hand make struct_ops implementer's job easier,
but might complicate libbpf workflow and does not apply to legacy
struct_ops attachment.
[0] https://github.com/sched-ext/scx/blob/main/scheds/rust/scx_layered/src/bpf/main.bpf.c#L557
[1] https://github.com/sched-ext/scx/blob/main/scheds/rust/scx_layered/src/bpf/main.bpf.c#L754
---
v5 -> v6
- Drop refcnt bumping for async callbacks and add RCU annotation (Martin)
- Fix libbpf bug and update comments (Andrii)
- Fix refcount bug in bpf_prog_assoc_struct_ops() (AI)
Link: https://lore.kernel.org/bpf/20251104172652.1746988-1-ameryhung@gmail.com/
v4 -> v5
- Simplify the API for getting associated struct_ops and dont't
expose struct_ops map lifecycle management (Andrii, Alexei)
Link: https://lore.kernel.org/bpf/20251024212914.1474337-1-ameryhung@gmail.com/
v3 -> v4
- Fix potential dangling pointer in timer callback. Protect
st_ops_assoc with RCU. The get helper now needs to be paired with
bpf_struct_ops_put()
- The command should only increase refcount once for a program
(Andrii)
- Test a struct_ops program reused in two struct_ops maps
- Test getting associated struct_ops in timer callback
Link: https://lore.kernel.org/bpf/20251017215627.722338-1-ameryhung@gmail.com/
v2 -> v3
- Change the type of st_ops_assoc from void* (i.e., kdata) to bpf_map
(Andrii)
- Fix a bug that clears BPF_PTR_POISON when a struct_ops map is freed
(Andrii)
- Return NULL if the map is not fully initialized (Martin)
- Move struct_ops map refcount inc/dec into internal helpers (Martin)
- Add libbpf API, bpf_program__assoc_struct_ops (Andrii)
Link: https://lore.kernel.org/bpf/20251016204503.3203690-1-ameryhung@gmail.com/
v1 -> v2
- Poison st_ops_assoc when reusing the program in more than one
struct_ops maps and add a helper to access the pointer (Andrii)
- Minor style and naming changes (Andrii)
Link: https://lore.kernel.org/bpf/20251010174953.2884682-1-ameryhung@gmail.com/
---
Amery Hung (6):
bpf: Allow verifier to fixup kernel module kfuncs
bpf: Support associating BPF program with struct_ops
libbpf: Add support for associating BPF program with struct_ops
selftests/bpf: Test BPF_PROG_ASSOC_STRUCT_OPS command
selftests/bpf: Test ambiguous associated struct_ops
selftests/bpf: Test getting associated struct_ops in timer callback
include/linux/bpf.h | 16 ++
include/uapi/linux/bpf.h | 17 ++
kernel/bpf/bpf_struct_ops.c | 92 +++++++++
kernel/bpf/core.c | 3 +
kernel/bpf/syscall.c | 46 +++++
kernel/bpf/verifier.c | 3 +-
tools/include/uapi/linux/bpf.h | 17 ++
tools/lib/bpf/bpf.c | 19 ++
tools/lib/bpf/bpf.h | 21 ++
tools/lib/bpf/libbpf.c | 31 +++
tools/lib/bpf/libbpf.h | 16 ++
tools/lib/bpf/libbpf.map | 2 +
.../bpf/prog_tests/test_struct_ops_assoc.c | 191 ++++++++++++++++++
.../selftests/bpf/progs/struct_ops_assoc.c | 105 ++++++++++
.../bpf/progs/struct_ops_assoc_in_timer.c | 77 +++++++
.../bpf/progs/struct_ops_assoc_reuse.c | 75 +++++++
.../selftests/bpf/test_kmods/bpf_testmod.c | 17 ++
.../bpf/test_kmods/bpf_testmod_kfunc.h | 1 +
18 files changed, 747 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c
create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_assoc.c
create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_assoc_in_timer.c
create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_assoc_reuse.c
--
2.47.3
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH bpf-next v6 1/6] bpf: Allow verifier to fixup kernel module kfuncs
2025-11-14 22:17 [PATCH bpf-next v6 0/7] Support associating BPF programs with struct_ops Amery Hung
@ 2025-11-14 22:17 ` Amery Hung
2025-11-14 23:06 ` bot+bpf-ci
2025-11-14 22:17 ` [PATCH bpf-next v6 2/6] bpf: Support associating BPF program with struct_ops Amery Hung
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Amery Hung @ 2025-11-14 22:17 UTC (permalink / raw)
To: bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, tj, martin.lau,
ameryhung, kernel-team
Allow verifier to fixup kfuncs in kernel module to support kfuncs with
__prog arguments. Currently, special kfuncs and kfuncs with __prog
arguments are kernel kfuncs. Allowing kernel module kfuncs should not
affect existing kfunc fixup as kernel module kfuncs have BTF IDs greater
than kernel kfuncs' BTF IDs.
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
kernel/bpf/verifier.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 542e23fb19c7..8f4410eee3b6 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -21973,8 +21973,7 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
if (!bpf_jit_supports_far_kfunc_call())
insn->imm = BPF_CALL_IMM(desc->addr);
- if (insn->off)
- return 0;
+
if (desc->func_id == special_kfunc_list[KF_bpf_obj_new_impl] ||
desc->func_id == special_kfunc_list[KF_bpf_percpu_obj_new_impl]) {
struct btf_struct_meta *kptr_struct_meta = env->insn_aux_data[insn_idx].kptr_struct_meta;
--
2.47.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next v6 2/6] bpf: Support associating BPF program with struct_ops
2025-11-14 22:17 [PATCH bpf-next v6 0/7] Support associating BPF programs with struct_ops Amery Hung
2025-11-14 22:17 ` [PATCH bpf-next v6 1/6] bpf: Allow verifier to fixup kernel module kfuncs Amery Hung
@ 2025-11-14 22:17 ` Amery Hung
2025-11-14 22:51 ` bot+bpf-ci
2025-11-14 22:17 ` [PATCH bpf-next v6 3/6] libbpf: Add support for " Amery Hung
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Amery Hung @ 2025-11-14 22:17 UTC (permalink / raw)
To: bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, tj, martin.lau,
ameryhung, kernel-team
Add a new BPF command BPF_PROG_ASSOC_STRUCT_OPS to allow associating
a BPF program with a struct_ops map. This command takes a file
descriptor of a struct_ops map and a BPF program and set
prog->aux->st_ops_assoc to the kdata of the struct_ops map.
The command does not accept a struct_ops program nor a non-struct_ops
map. Programs of a struct_ops map is automatically associated with the
map during map update. If a program is shared between two struct_ops
maps, prog->aux->st_ops_assoc will be poisoned to indicate that the
associated struct_ops is ambiguous. The pointer, once poisoned, cannot
be reset since we have lost track of associated struct_ops. For other
program types, the associated struct_ops map, once set, cannot be
changed later. This restriction may be lifted in the future if there is
a use case.
A kernel helper bpf_prog_get_assoc_struct_ops() can be used to retrieve
the associated struct_ops pointer. The returned pointer, if not NULL, is
guaranteed to be valid and point to a fully updated struct_ops struct.
For struct_ops program reused in multiple struct_ops map, the return
will be NULL.
prog->aux->st_ops_assoc is protected by bumping the refcount for
non-struct_ops programs and RCU for struct_ops programs. Since it would
be inefficient to track programs associated with a struct_ops map, every
non-struct_ops program will bump the refcount of the map to make sure
st_ops_assoc stays valid. For a struct_ops program, it is protected by
RCU as map_free will wait for an RCU grace period before disassociating
the program with the map. The helper must be called in BPF program
context or RCU read-side critical section.
struct_ops implementers should note that the struct_ops returned may or
may not be attached. The struct_ops implementer will be responsible for
tracking and checking the state of the associated struct_ops map if the
use case requires an attached struct_ops.
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
include/linux/bpf.h | 16 ++++++
include/uapi/linux/bpf.h | 17 +++++++
kernel/bpf/bpf_struct_ops.c | 92 ++++++++++++++++++++++++++++++++++
kernel/bpf/core.c | 3 ++
kernel/bpf/syscall.c | 46 +++++++++++++++++
tools/include/uapi/linux/bpf.h | 17 +++++++
6 files changed, 191 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index a47d67db3be5..6c1c64400a86 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1726,6 +1726,8 @@ struct bpf_prog_aux {
struct rcu_head rcu;
};
struct bpf_stream stream[2];
+ struct mutex st_ops_assoc_mutex;
+ struct bpf_map __rcu *st_ops_assoc;
};
struct bpf_prog {
@@ -2026,6 +2028,9 @@ static inline void bpf_module_put(const void *data, struct module *owner)
module_put(owner);
}
int bpf_struct_ops_link_create(union bpf_attr *attr);
+int bpf_prog_assoc_struct_ops(struct bpf_prog *prog, struct bpf_map *map);
+void bpf_prog_disassoc_struct_ops(struct bpf_prog *prog);
+void *bpf_prog_get_assoc_struct_ops(const struct bpf_prog_aux *aux);
u32 bpf_struct_ops_id(const void *kdata);
#ifdef CONFIG_NET
@@ -2073,6 +2078,17 @@ static inline int bpf_struct_ops_link_create(union bpf_attr *attr)
{
return -EOPNOTSUPP;
}
+static inline int bpf_prog_assoc_struct_ops(struct bpf_prog *prog, struct bpf_map *map)
+{
+ return -EOPNOTSUPP;
+}
+static inline void bpf_prog_disassoc_struct_ops(struct bpf_prog *prog)
+{
+}
+static inline void *bpf_prog_get_assoc_struct_ops(const struct bpf_prog_aux *aux)
+{
+ return NULL;
+}
static inline void bpf_map_struct_ops_info_fill(struct bpf_map_info *info, struct bpf_map *map)
{
}
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 1d73f165394d..a2d8cfac8555 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -918,6 +918,16 @@ union bpf_iter_link_info {
* Number of bytes read from the stream on success, or -1 if an
* error occurred (in which case, *errno* is set appropriately).
*
+ * BPF_PROG_ASSOC_STRUCT_OPS
+ * Description
+ * Associate a BPF program with a struct_ops map. The struct_ops
+ * map is identified by *map_fd* and the BPF program is
+ * identified by *prog_fd*.
+ *
+ * Return
+ * 0 on success or -1 if an error occurred (in which case,
+ * *errno* is set appropriately).
+ *
* NOTES
* eBPF objects (maps and programs) can be shared between processes.
*
@@ -974,6 +984,7 @@ enum bpf_cmd {
BPF_PROG_BIND_MAP,
BPF_TOKEN_CREATE,
BPF_PROG_STREAM_READ_BY_FD,
+ BPF_PROG_ASSOC_STRUCT_OPS,
__MAX_BPF_CMD,
};
@@ -1893,6 +1904,12 @@ union bpf_attr {
__u32 prog_fd;
} prog_stream_read;
+ struct {
+ __u32 map_fd;
+ __u32 prog_fd;
+ __u32 flags;
+ } prog_assoc_struct_ops;
+
} __attribute__((aligned(8)));
/* The description below is an attempt at providing documentation to eBPF
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index a41e6730edcf..626aa91979b2 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -533,6 +533,17 @@ static void bpf_struct_ops_map_put_progs(struct bpf_struct_ops_map *st_map)
}
}
+static void bpf_struct_ops_map_dissoc_progs(struct bpf_struct_ops_map *st_map)
+{
+ u32 i;
+
+ for (i = 0; i < st_map->funcs_cnt; i++) {
+ if (!st_map->links[i])
+ break;
+ bpf_prog_disassoc_struct_ops(st_map->links[i]->prog);
+ }
+}
+
static void bpf_struct_ops_map_free_image(struct bpf_struct_ops_map *st_map)
{
int i;
@@ -811,6 +822,12 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
&bpf_struct_ops_link_lops, prog, prog->expected_attach_type);
*plink++ = &link->link;
+ err = bpf_prog_assoc_struct_ops(prog, &st_map->map);
+ if (err) {
+ bpf_prog_put(prog);
+ goto reset_unlock;
+ }
+
ksym = kzalloc(sizeof(*ksym), GFP_USER);
if (!ksym) {
err = -ENOMEM;
@@ -980,6 +997,8 @@ static void bpf_struct_ops_map_free(struct bpf_map *map)
if (btf_is_module(st_map->btf))
module_put(st_map->st_ops_desc->st_ops->owner);
+ bpf_struct_ops_map_dissoc_progs(st_map);
+
bpf_struct_ops_map_del_ksyms(st_map);
/* The struct_ops's function may switch to another struct_ops.
@@ -1394,6 +1413,79 @@ int bpf_struct_ops_link_create(union bpf_attr *attr)
return err;
}
+int bpf_prog_assoc_struct_ops(struct bpf_prog *prog, struct bpf_map *map)
+{
+ struct bpf_map *st_ops_assoc;
+
+ guard(mutex)(&prog->aux->st_ops_assoc_mutex);
+
+ st_ops_assoc = rcu_dereference_protected(prog->aux->st_ops_assoc,
+ lockdep_is_held(&prog->aux->st_ops_assoc_mutex));
+ if (st_ops_assoc && st_ops_assoc == map)
+ return 0;
+
+ if (st_ops_assoc) {
+ if (prog->type != BPF_PROG_TYPE_STRUCT_OPS)
+ return -EBUSY;
+
+ rcu_assign_pointer(prog->aux->st_ops_assoc, BPF_PTR_POISON);
+ } else {
+ /*
+ * struct_ops map does not track associated non-struct_ops programs.
+ * Bump the refcount to make sure st_ops_assoc is always valid.
+ */
+ if (prog->type != BPF_PROG_TYPE_STRUCT_OPS)
+ bpf_map_inc(map);
+
+ rcu_assign_pointer(prog->aux->st_ops_assoc, map);
+ }
+
+ return 0;
+}
+
+void bpf_prog_disassoc_struct_ops(struct bpf_prog *prog)
+{
+ struct bpf_map *st_ops_assoc;
+
+ guard(mutex)(&prog->aux->st_ops_assoc_mutex);
+
+ st_ops_assoc = rcu_dereference_protected(prog->aux->st_ops_assoc,
+ lockdep_is_held(&prog->aux->st_ops_assoc_mutex));
+ if (!st_ops_assoc || st_ops_assoc == BPF_PTR_POISON)
+ return;
+
+ if (prog->type != BPF_PROG_TYPE_STRUCT_OPS)
+ bpf_map_put(st_ops_assoc);
+
+ RCU_INIT_POINTER(prog->aux->st_ops_assoc, NULL);
+}
+
+/*
+ * Get a reference to the struct_ops struct (i.e., kdata) associated with a
+ * program. Should only be called in BPF program context (e.g., in a kfunc).
+ *
+ * If the returned pointer is not NULL, it must points to a valid and
+ * initialized struct_ops. The struct_ops may or may not be attached.
+ * Kernel struct_ops implementers are responsible for tracking and checking
+ * the state of the struct_ops if the use case requires an attached struct_ops.
+ */
+void *bpf_prog_get_assoc_struct_ops(const struct bpf_prog_aux *aux)
+{
+ struct bpf_struct_ops_map *st_map;
+ struct bpf_map *st_ops_assoc;
+
+ st_ops_assoc = rcu_dereference_check(aux->st_ops_assoc, bpf_rcu_lock_held());
+ if (!st_ops_assoc || st_ops_assoc == BPF_PTR_POISON)
+ return NULL;
+
+ st_map = (struct bpf_struct_ops_map *)st_ops_assoc;
+ if (smp_load_acquire(&st_map->kvalue.common.state) == BPF_STRUCT_OPS_STATE_INIT)
+ return NULL;
+
+ return &st_map->kvalue.data;
+}
+EXPORT_SYMBOL_GPL(bpf_prog_get_assoc_struct_ops);
+
void bpf_map_struct_ops_info_fill(struct bpf_map_info *info, struct bpf_map *map)
{
struct bpf_struct_ops_map *st_map = (struct bpf_struct_ops_map *)map;
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index d595fe512498..441bfeece377 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -136,6 +136,7 @@ struct bpf_prog *bpf_prog_alloc_no_stats(unsigned int size, gfp_t gfp_extra_flag
mutex_init(&fp->aux->used_maps_mutex);
mutex_init(&fp->aux->ext_mutex);
mutex_init(&fp->aux->dst_mutex);
+ mutex_init(&fp->aux->st_ops_assoc_mutex);
#ifdef CONFIG_BPF_SYSCALL
bpf_prog_stream_init(fp);
@@ -286,6 +287,7 @@ void __bpf_prog_free(struct bpf_prog *fp)
if (fp->aux) {
mutex_destroy(&fp->aux->used_maps_mutex);
mutex_destroy(&fp->aux->dst_mutex);
+ mutex_destroy(&fp->aux->st_ops_assoc_mutex);
kfree(fp->aux->poke_tab);
kfree(fp->aux);
}
@@ -2875,6 +2877,7 @@ static void bpf_prog_free_deferred(struct work_struct *work)
#endif
bpf_free_used_maps(aux);
bpf_free_used_btfs(aux);
+ bpf_prog_disassoc_struct_ops(aux->prog);
if (bpf_prog_is_dev_bound(aux))
bpf_prog_dev_bound_destroy(aux->prog);
#ifdef CONFIG_PERF_EVENTS
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 8a129746bd6c..dddbe89e9718 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -6107,6 +6107,49 @@ static int prog_stream_read(union bpf_attr *attr)
return ret;
}
+#define BPF_PROG_ASSOC_STRUCT_OPS_LAST_FIELD prog_assoc_struct_ops.prog_fd
+
+static int prog_assoc_struct_ops(union bpf_attr *attr)
+{
+ struct bpf_prog *prog;
+ struct bpf_map *map;
+ int ret;
+
+ if (CHECK_ATTR(BPF_PROG_ASSOC_STRUCT_OPS))
+ return -EINVAL;
+
+ if (attr->prog_assoc_struct_ops.flags)
+ return -EINVAL;
+
+ prog = bpf_prog_get(attr->prog_assoc_struct_ops.prog_fd);
+ if (IS_ERR(prog))
+ return PTR_ERR(prog);
+
+ if (prog->type == BPF_PROG_TYPE_STRUCT_OPS) {
+ ret = -EINVAL;
+ goto put_prog;
+ }
+
+ map = bpf_map_get(attr->prog_assoc_struct_ops.map_fd);
+ if (IS_ERR(map)) {
+ ret = PTR_ERR(map);
+ goto put_prog;
+ }
+
+ if (map->map_type != BPF_MAP_TYPE_STRUCT_OPS) {
+ ret = -EINVAL;
+ goto put_map;
+ }
+
+ ret = bpf_prog_assoc_struct_ops(prog, map);
+
+put_map:
+ bpf_map_put(map);
+put_prog:
+ bpf_prog_put(prog);
+ return ret;
+}
+
static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size)
{
union bpf_attr attr;
@@ -6246,6 +6289,9 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size)
case BPF_PROG_STREAM_READ_BY_FD:
err = prog_stream_read(&attr);
break;
+ case BPF_PROG_ASSOC_STRUCT_OPS:
+ err = prog_assoc_struct_ops(&attr);
+ break;
default:
err = -EINVAL;
break;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 1d73f165394d..a2d8cfac8555 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -918,6 +918,16 @@ union bpf_iter_link_info {
* Number of bytes read from the stream on success, or -1 if an
* error occurred (in which case, *errno* is set appropriately).
*
+ * BPF_PROG_ASSOC_STRUCT_OPS
+ * Description
+ * Associate a BPF program with a struct_ops map. The struct_ops
+ * map is identified by *map_fd* and the BPF program is
+ * identified by *prog_fd*.
+ *
+ * Return
+ * 0 on success or -1 if an error occurred (in which case,
+ * *errno* is set appropriately).
+ *
* NOTES
* eBPF objects (maps and programs) can be shared between processes.
*
@@ -974,6 +984,7 @@ enum bpf_cmd {
BPF_PROG_BIND_MAP,
BPF_TOKEN_CREATE,
BPF_PROG_STREAM_READ_BY_FD,
+ BPF_PROG_ASSOC_STRUCT_OPS,
__MAX_BPF_CMD,
};
@@ -1893,6 +1904,12 @@ union bpf_attr {
__u32 prog_fd;
} prog_stream_read;
+ struct {
+ __u32 map_fd;
+ __u32 prog_fd;
+ __u32 flags;
+ } prog_assoc_struct_ops;
+
} __attribute__((aligned(8)));
/* The description below is an attempt at providing documentation to eBPF
--
2.47.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next v6 3/6] libbpf: Add support for associating BPF program with struct_ops
2025-11-14 22:17 [PATCH bpf-next v6 0/7] Support associating BPF programs with struct_ops Amery Hung
2025-11-14 22:17 ` [PATCH bpf-next v6 1/6] bpf: Allow verifier to fixup kernel module kfuncs Amery Hung
2025-11-14 22:17 ` [PATCH bpf-next v6 2/6] bpf: Support associating BPF program with struct_ops Amery Hung
@ 2025-11-14 22:17 ` Amery Hung
2025-11-14 22:58 ` bot+bpf-ci
2025-11-14 22:17 ` [PATCH bpf-next v6 4/6] selftests/bpf: Test BPF_PROG_ASSOC_STRUCT_OPS command Amery Hung
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Amery Hung @ 2025-11-14 22:17 UTC (permalink / raw)
To: bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, tj, martin.lau,
ameryhung, kernel-team
Add low-level wrapper and libbpf API for BPF_PROG_ASSOC_STRUCT_OPS
command in the bpf() syscall.
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
tools/lib/bpf/bpf.c | 19 +++++++++++++++++++
tools/lib/bpf/bpf.h | 21 +++++++++++++++++++++
tools/lib/bpf/libbpf.c | 31 +++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 16 ++++++++++++++++
tools/lib/bpf/libbpf.map | 2 ++
5 files changed, 89 insertions(+)
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index b66f5fbfbbb2..21b57a629916 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -1397,3 +1397,22 @@ int bpf_prog_stream_read(int prog_fd, __u32 stream_id, void *buf, __u32 buf_len,
err = sys_bpf(BPF_PROG_STREAM_READ_BY_FD, &attr, attr_sz);
return libbpf_err_errno(err);
}
+
+int bpf_prog_assoc_struct_ops(int prog_fd, int map_fd,
+ struct bpf_prog_assoc_struct_ops_opts *opts)
+{
+ const size_t attr_sz = offsetofend(union bpf_attr, prog_assoc_struct_ops);
+ union bpf_attr attr;
+ int err;
+
+ if (!OPTS_VALID(opts, bpf_prog_assoc_struct_ops_opts))
+ return libbpf_err(-EINVAL);
+
+ memset(&attr, 0, attr_sz);
+ attr.prog_assoc_struct_ops.map_fd = map_fd;
+ attr.prog_assoc_struct_ops.prog_fd = prog_fd;
+ attr.prog_assoc_struct_ops.flags = OPTS_GET(opts, flags, 0);
+
+ err = sys_bpf(BPF_PROG_ASSOC_STRUCT_OPS, &attr, attr_sz);
+ return libbpf_err_errno(err);
+}
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index e983a3e40d61..1f9c28d27795 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -733,6 +733,27 @@ struct bpf_prog_stream_read_opts {
LIBBPF_API int bpf_prog_stream_read(int prog_fd, __u32 stream_id, void *buf, __u32 buf_len,
struct bpf_prog_stream_read_opts *opts);
+struct bpf_prog_assoc_struct_ops_opts {
+ size_t sz;
+ __u32 flags;
+ size_t :0;
+};
+#define bpf_prog_assoc_struct_ops_opts__last_field flags
+
+/**
+ * @brief **bpf_prog_assoc_struct_ops** associates a BPF program with a
+ * struct_ops map.
+ *
+ * @param prog_fd FD for the BPF program
+ * @param map_fd FD for the struct_ops map to be associated with the BPF program
+ * @param opts optional options, can be NULL
+ *
+ * @return 0 on success; negative error code, otherwise (errno is also set to
+ * the error code)
+ */
+LIBBPF_API int bpf_prog_assoc_struct_ops(int prog_fd, int map_fd,
+ struct bpf_prog_assoc_struct_ops_opts *opts);
+
#ifdef __cplusplus
} /* extern "C" */
#endif
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index fbe74686c97d..98ec63947fc9 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -13891,6 +13891,37 @@ int bpf_program__set_attach_target(struct bpf_program *prog,
return 0;
}
+int bpf_program__assoc_struct_ops(struct bpf_program *prog, struct bpf_map *map,
+ struct bpf_prog_assoc_struct_ops_opts *opts)
+{
+ int prog_fd, map_fd;
+
+ prog_fd = bpf_program__fd(prog);
+ if (prog_fd < 0) {
+ pr_warn("prog '%s': can't associate BPF program without FD (was it loaded?)\n",
+ prog->name);
+ return -EINVAL;
+ }
+
+ if (prog->type == BPF_PROG_TYPE_STRUCT_OPS) {
+ pr_warn("prog '%s': can't associate struct_ops program\n", prog->name);
+ return -EINVAL;
+ }
+
+ map_fd = bpf_map__fd(map);
+ if (map_fd < 0) {
+ pr_warn("map '%s': can't associate BPF map without FD (was it created?)\n", map->name);
+ return -EINVAL;
+ }
+
+ if (!bpf_map__is_struct_ops(map)) {
+ pr_warn("map '%s': can't associate non-struct_ops map\n", map->name);
+ return -EINVAL;
+ }
+
+ return bpf_prog_assoc_struct_ops(prog_fd, map_fd, opts);
+}
+
int parse_cpu_mask_str(const char *s, bool **mask, int *mask_sz)
{
int err = 0, n, len, start, end = -1;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 5118d0a90e24..8866e5bf7b0c 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -1003,6 +1003,22 @@ LIBBPF_API int
bpf_program__set_attach_target(struct bpf_program *prog, int attach_prog_fd,
const char *attach_func_name);
+struct bpf_prog_assoc_struct_ops_opts; /* defined in bpf.h */
+
+/**
+ * @brief **bpf_program__assoc_struct_ops()** associates a BPF program with a
+ * struct_ops map.
+ *
+ * @param prog BPF program
+ * @param map struct_ops map to be associated with the BPF program
+ * @param opts optional options, can be NULL
+ *
+ * @return 0, on success; negative error code, otherwise
+ */
+LIBBPF_API int
+bpf_program__assoc_struct_ops(struct bpf_program *prog, struct bpf_map *map,
+ struct bpf_prog_assoc_struct_ops_opts *opts);
+
/**
* @brief **bpf_object__find_map_by_name()** returns BPF map of
* the given name, if it exists within the passed BPF object
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 8ed8749907d4..84fb90a016c9 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -451,4 +451,6 @@ LIBBPF_1.7.0 {
global:
bpf_map__set_exclusive_program;
bpf_map__exclusive_program;
+ bpf_prog_assoc_struct_ops;
+ bpf_program__assoc_struct_ops;
} LIBBPF_1.6.0;
--
2.47.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next v6 4/6] selftests/bpf: Test BPF_PROG_ASSOC_STRUCT_OPS command
2025-11-14 22:17 [PATCH bpf-next v6 0/7] Support associating BPF programs with struct_ops Amery Hung
` (2 preceding siblings ...)
2025-11-14 22:17 ` [PATCH bpf-next v6 3/6] libbpf: Add support for " Amery Hung
@ 2025-11-14 22:17 ` Amery Hung
2025-11-14 22:17 ` [PATCH bpf-next v6 5/6] selftests/bpf: Test ambiguous associated struct_ops Amery Hung
2025-11-14 22:17 ` [PATCH bpf-next v6 6/6] selftests/bpf: Test getting associated struct_ops in timer callback Amery Hung
5 siblings, 0 replies; 12+ messages in thread
From: Amery Hung @ 2025-11-14 22:17 UTC (permalink / raw)
To: bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, tj, martin.lau,
ameryhung, kernel-team
Test BPF_PROG_ASSOC_STRUCT_OPS command that associates a BPF program
with a struct_ops. The test follows the same logic in commit
ba7000f1c360 ("selftests/bpf: Test multi_st_ops and calling kfuncs from
different programs"), but instead of using map id to identify a specific
struct_ops, this test uses the new BPF command to associate a struct_ops
with a program.
The test consists of two sets of almost identical struct_ops maps and BPF
programs associated with the map. Their only difference is the unique
value returned by bpf_testmod_multi_st_ops::test_1().
The test first loads the programs and associates them with struct_ops
maps. Then, it exercises the BPF programs. They will in turn call kfunc
bpf_kfunc_multi_st_ops_test_1_prog_arg() to trigger test_1() of the
associated struct_ops map, and then check if the right unique value is
returned.
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
.../bpf/prog_tests/test_struct_ops_assoc.c | 72 ++++++++++++
.../selftests/bpf/progs/struct_ops_assoc.c | 105 ++++++++++++++++++
.../selftests/bpf/test_kmods/bpf_testmod.c | 17 +++
.../bpf/test_kmods/bpf_testmod_kfunc.h | 1 +
4 files changed, 195 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c
create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_assoc.c
diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c
new file mode 100644
index 000000000000..29e8b58a14fa
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+#include "struct_ops_assoc.skel.h"
+
+static void test_st_ops_assoc(void)
+{
+ struct struct_ops_assoc *skel = NULL;
+ int err, pid;
+
+ skel = struct_ops_assoc__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "struct_ops_assoc__open"))
+ goto out;
+
+ /* cannot explicitly associate struct_ops program */
+ err = bpf_program__assoc_struct_ops(skel->progs.test_1_a,
+ skel->maps.st_ops_map_a, NULL);
+ ASSERT_ERR(err, "bpf_program__assoc_struct_ops");
+
+ err = bpf_program__assoc_struct_ops(skel->progs.syscall_prog_a,
+ skel->maps.st_ops_map_a, NULL);
+ ASSERT_OK(err, "bpf_program__assoc_struct_ops");
+
+ err = bpf_program__assoc_struct_ops(skel->progs.sys_enter_prog_a,
+ skel->maps.st_ops_map_a, NULL);
+ ASSERT_OK(err, "bpf_program__assoc_struct_ops");
+
+ err = bpf_program__assoc_struct_ops(skel->progs.syscall_prog_b,
+ skel->maps.st_ops_map_b, NULL);
+ ASSERT_OK(err, "bpf_program__assoc_struct_ops");
+
+ err = bpf_program__assoc_struct_ops(skel->progs.sys_enter_prog_b,
+ skel->maps.st_ops_map_b, NULL);
+ ASSERT_OK(err, "bpf_program__assoc_struct_ops");
+
+ /* sys_enter_prog_a already associated with map_a */
+ err = bpf_program__assoc_struct_ops(skel->progs.sys_enter_prog_a,
+ skel->maps.st_ops_map_b, NULL);
+ ASSERT_ERR(err, "bpf_program__assoc_struct_ops");
+
+ err = struct_ops_assoc__attach(skel);
+ if (!ASSERT_OK(err, "struct_ops_assoc__attach"))
+ goto out;
+
+ /* run tracing prog that calls .test_1 and checks return */
+ pid = getpid();
+ skel->bss->test_pid = pid;
+ sys_gettid();
+ skel->bss->test_pid = 0;
+
+ ASSERT_EQ(skel->bss->test_err_a, 0, "skel->bss->test_err_a");
+ ASSERT_EQ(skel->bss->test_err_b, 0, "skel->bss->test_err_b");
+
+ /* run syscall_prog that calls .test_1 and checks return */
+ err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.syscall_prog_a), NULL);
+ ASSERT_OK(err, "bpf_prog_test_run_opts");
+
+ err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.syscall_prog_b), NULL);
+ ASSERT_OK(err, "bpf_prog_test_run_opts");
+
+ ASSERT_EQ(skel->bss->test_err_a, 0, "skel->bss->test_err_a");
+ ASSERT_EQ(skel->bss->test_err_b, 0, "skel->bss->test_err_b");
+
+out:
+ struct_ops_assoc__destroy(skel);
+}
+
+void test_struct_ops_assoc(void)
+{
+ if (test__start_subtest("st_ops_assoc"))
+ test_st_ops_assoc();
+}
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_assoc.c b/tools/testing/selftests/bpf/progs/struct_ops_assoc.c
new file mode 100644
index 000000000000..fe47287a49f0
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/struct_ops_assoc.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+#include "../test_kmods/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
+
+char _license[] SEC("license") = "GPL";
+
+int test_pid;
+
+/* Programs associated with st_ops_map_a */
+
+#define MAP_A_MAGIC 1234
+int test_err_a;
+
+SEC("struct_ops")
+int BPF_PROG(test_1_a, struct st_ops_args *args)
+{
+ return MAP_A_MAGIC;
+}
+
+SEC("tp_btf/sys_enter")
+int BPF_PROG(sys_enter_prog_a, struct pt_regs *regs, long id)
+{
+ struct st_ops_args args = {};
+ struct task_struct *task;
+ int ret;
+
+ task = bpf_get_current_task_btf();
+ if (!test_pid || task->pid != test_pid)
+ return 0;
+
+ ret = bpf_kfunc_multi_st_ops_test_1_prog_arg(&args, NULL);
+ if (ret != MAP_A_MAGIC)
+ test_err_a++;
+
+ return 0;
+}
+
+SEC("syscall")
+int syscall_prog_a(void *ctx)
+{
+ struct st_ops_args args = {};
+ int ret;
+
+ ret = bpf_kfunc_multi_st_ops_test_1_prog_arg(&args, NULL);
+ if (ret != MAP_A_MAGIC)
+ test_err_a++;
+
+ return 0;
+}
+
+SEC(".struct_ops.link")
+struct bpf_testmod_multi_st_ops st_ops_map_a = {
+ .test_1 = (void *)test_1_a,
+};
+
+/* Programs associated with st_ops_map_b */
+
+#define MAP_B_MAGIC 5678
+int test_err_b;
+
+SEC("struct_ops")
+int BPF_PROG(test_1_b, struct st_ops_args *args)
+{
+ return MAP_B_MAGIC;
+}
+
+SEC("tp_btf/sys_enter")
+int BPF_PROG(sys_enter_prog_b, struct pt_regs *regs, long id)
+{
+ struct st_ops_args args = {};
+ struct task_struct *task;
+ int ret;
+
+ task = bpf_get_current_task_btf();
+ if (!test_pid || task->pid != test_pid)
+ return 0;
+
+ ret = bpf_kfunc_multi_st_ops_test_1_prog_arg(&args, NULL);
+ if (ret != MAP_B_MAGIC)
+ test_err_b++;
+
+ return 0;
+}
+
+SEC("syscall")
+int syscall_prog_b(void *ctx)
+{
+ struct st_ops_args args = {};
+ int ret;
+
+ ret = bpf_kfunc_multi_st_ops_test_1_prog_arg(&args, NULL);
+ if (ret != MAP_B_MAGIC)
+ test_err_b++;
+
+ return 0;
+}
+
+SEC(".struct_ops.link")
+struct bpf_testmod_multi_st_ops st_ops_map_b = {
+ .test_1 = (void *)test_1_b,
+};
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
index 8074bc5f6f20..f45258202997 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
@@ -1108,6 +1108,7 @@ __bpf_kfunc int bpf_kfunc_st_ops_inc10(struct st_ops_args *args)
}
__bpf_kfunc int bpf_kfunc_multi_st_ops_test_1(struct st_ops_args *args, u32 id);
+__bpf_kfunc int bpf_kfunc_multi_st_ops_test_1_prog_arg(struct st_ops_args *args, void *aux_prog);
BTF_KFUNCS_START(bpf_testmod_check_kfunc_ids)
BTF_ID_FLAGS(func, bpf_testmod_test_mod_kfunc)
@@ -1150,6 +1151,7 @@ BTF_ID_FLAGS(func, bpf_kfunc_st_ops_test_epilogue, KF_TRUSTED_ARGS | KF_SLEEPABL
BTF_ID_FLAGS(func, bpf_kfunc_st_ops_test_pro_epilogue, KF_TRUSTED_ARGS | KF_SLEEPABLE)
BTF_ID_FLAGS(func, bpf_kfunc_st_ops_inc10, KF_TRUSTED_ARGS)
BTF_ID_FLAGS(func, bpf_kfunc_multi_st_ops_test_1, KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_kfunc_multi_st_ops_test_1_prog_arg, KF_TRUSTED_ARGS)
BTF_KFUNCS_END(bpf_testmod_check_kfunc_ids)
static int bpf_testmod_ops_init(struct btf *btf)
@@ -1611,6 +1613,7 @@ static struct bpf_testmod_multi_st_ops *multi_st_ops_find_nolock(u32 id)
return NULL;
}
+/* Call test_1() of the struct_ops map identified by the id */
int bpf_kfunc_multi_st_ops_test_1(struct st_ops_args *args, u32 id)
{
struct bpf_testmod_multi_st_ops *st_ops;
@@ -1626,6 +1629,20 @@ int bpf_kfunc_multi_st_ops_test_1(struct st_ops_args *args, u32 id)
return ret;
}
+/* Call test_1() of the associated struct_ops map */
+int bpf_kfunc_multi_st_ops_test_1_prog_arg(struct st_ops_args *args, void *aux__prog)
+{
+ struct bpf_prog_aux *prog_aux = (struct bpf_prog_aux *)aux__prog;
+ struct bpf_testmod_multi_st_ops *st_ops;
+ int ret = -1;
+
+ st_ops = (struct bpf_testmod_multi_st_ops *)bpf_prog_get_assoc_struct_ops(prog_aux);
+ if (st_ops)
+ ret = st_ops->test_1(args);
+
+ return ret;
+}
+
static int multi_st_ops_reg(void *kdata, struct bpf_link *link)
{
struct bpf_testmod_multi_st_ops *st_ops =
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
index 4df6fa6a92cb..d40f4cddbd1e 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
@@ -162,5 +162,6 @@ struct task_struct *bpf_kfunc_ret_rcu_test(void) __ksym;
int *bpf_kfunc_ret_rcu_test_nostruct(int rdonly_buf_size) __ksym;
int bpf_kfunc_multi_st_ops_test_1(struct st_ops_args *args, u32 id) __ksym;
+int bpf_kfunc_multi_st_ops_test_1_prog_arg(struct st_ops_args *args, void *aux__prog) __ksym;
#endif /* _BPF_TESTMOD_KFUNC_H */
--
2.47.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next v6 5/6] selftests/bpf: Test ambiguous associated struct_ops
2025-11-14 22:17 [PATCH bpf-next v6 0/7] Support associating BPF programs with struct_ops Amery Hung
` (3 preceding siblings ...)
2025-11-14 22:17 ` [PATCH bpf-next v6 4/6] selftests/bpf: Test BPF_PROG_ASSOC_STRUCT_OPS command Amery Hung
@ 2025-11-14 22:17 ` Amery Hung
2025-11-14 22:17 ` [PATCH bpf-next v6 6/6] selftests/bpf: Test getting associated struct_ops in timer callback Amery Hung
5 siblings, 0 replies; 12+ messages in thread
From: Amery Hung @ 2025-11-14 22:17 UTC (permalink / raw)
To: bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, tj, martin.lau,
ameryhung, kernel-team
Add a test to make sure implicit struct_ops association does not
break backward compatibility nor return incorrect struct_ops.
struct_ops programs should still be allowed to be reused in
different struct_ops map. The associated struct_ops map set implicitly
however will be poisoned. Trying to read it through the helper
bpf_prog_get_assoc_struct_ops() should result in a NULL pointer.
While recursion of test_1() cannot happen due to the associated
struct_ops being ambiguois, explicitly check for it to prevent stack
overflow if the test regresses.
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
.../bpf/prog_tests/test_struct_ops_assoc.c | 38 ++++++++++
.../bpf/progs/struct_ops_assoc_reuse.c | 75 +++++++++++++++++++
2 files changed, 113 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_assoc_reuse.c
diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c
index 29e8b58a14fa..f69306cb8974 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c
@@ -2,6 +2,7 @@
#include <test_progs.h>
#include "struct_ops_assoc.skel.h"
+#include "struct_ops_assoc_reuse.skel.h"
static void test_st_ops_assoc(void)
{
@@ -65,8 +66,45 @@ static void test_st_ops_assoc(void)
struct_ops_assoc__destroy(skel);
}
+static void test_st_ops_assoc_reuse(void)
+{
+ struct struct_ops_assoc_reuse *skel = NULL;
+ int err;
+
+ skel = struct_ops_assoc_reuse__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "struct_ops_assoc_reuse__open"))
+ goto out;
+
+ err = bpf_program__assoc_struct_ops(skel->progs.syscall_prog_a,
+ skel->maps.st_ops_map_a, NULL);
+ ASSERT_OK(err, "bpf_program__assoc_struct_ops");
+
+ err = bpf_program__assoc_struct_ops(skel->progs.syscall_prog_b,
+ skel->maps.st_ops_map_b, NULL);
+ ASSERT_OK(err, "bpf_program__assoc_struct_ops");
+
+ err = struct_ops_assoc_reuse__attach(skel);
+ if (!ASSERT_OK(err, "struct_ops_assoc__attach"))
+ goto out;
+
+ /* run syscall_prog that calls .test_1 and checks return */
+ err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.syscall_prog_a), NULL);
+ ASSERT_OK(err, "bpf_prog_test_run_opts");
+
+ err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.syscall_prog_b), NULL);
+ ASSERT_OK(err, "bpf_prog_test_run_opts");
+
+ ASSERT_EQ(skel->bss->test_err_a, 0, "skel->bss->test_err_a");
+ ASSERT_EQ(skel->bss->test_err_b, 0, "skel->bss->test_err_b");
+
+out:
+ struct_ops_assoc_reuse__destroy(skel);
+}
+
void test_struct_ops_assoc(void)
{
if (test__start_subtest("st_ops_assoc"))
test_st_ops_assoc();
+ if (test__start_subtest("st_ops_assoc_reuse"))
+ test_st_ops_assoc_reuse();
}
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_assoc_reuse.c b/tools/testing/selftests/bpf/progs/struct_ops_assoc_reuse.c
new file mode 100644
index 000000000000..caaa45bdccc2
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/struct_ops_assoc_reuse.c
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+#include "../test_kmods/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
+
+char _license[] SEC("license") = "GPL";
+
+#define MAP_A_MAGIC 1234
+int test_err_a;
+int recur;
+
+/*
+ * test_1_a is reused. The kfunc should not be able to get the associated
+ * struct_ops and call test_1 recursively as it is ambiguous.
+ */
+SEC("struct_ops")
+int BPF_PROG(test_1_a, struct st_ops_args *args)
+{
+ int ret;
+
+ if (!recur) {
+ recur++;
+ ret = bpf_kfunc_multi_st_ops_test_1_prog_arg(args, NULL);
+ if (ret != -1)
+ test_err_a++;
+ recur--;
+ }
+
+ return MAP_A_MAGIC;
+}
+
+/* Programs associated with st_ops_map_a */
+
+SEC("syscall")
+int syscall_prog_a(void *ctx)
+{
+ struct st_ops_args args = {};
+ int ret;
+
+ ret = bpf_kfunc_multi_st_ops_test_1_prog_arg(&args, NULL);
+ if (ret != MAP_A_MAGIC)
+ test_err_a++;
+
+ return 0;
+}
+
+SEC(".struct_ops.link")
+struct bpf_testmod_multi_st_ops st_ops_map_a = {
+ .test_1 = (void *)test_1_a,
+};
+
+/* Programs associated with st_ops_map_b */
+
+int test_err_b;
+
+SEC("syscall")
+int syscall_prog_b(void *ctx)
+{
+ struct st_ops_args args = {};
+ int ret;
+
+ ret = bpf_kfunc_multi_st_ops_test_1_prog_arg(&args, NULL);
+ if (ret != MAP_A_MAGIC)
+ test_err_b++;
+
+ return 0;
+}
+
+SEC(".struct_ops.link")
+struct bpf_testmod_multi_st_ops st_ops_map_b = {
+ .test_1 = (void *)test_1_a,
+};
--
2.47.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next v6 6/6] selftests/bpf: Test getting associated struct_ops in timer callback
2025-11-14 22:17 [PATCH bpf-next v6 0/7] Support associating BPF programs with struct_ops Amery Hung
` (4 preceding siblings ...)
2025-11-14 22:17 ` [PATCH bpf-next v6 5/6] selftests/bpf: Test ambiguous associated struct_ops Amery Hung
@ 2025-11-14 22:17 ` Amery Hung
5 siblings, 0 replies; 12+ messages in thread
From: Amery Hung @ 2025-11-14 22:17 UTC (permalink / raw)
To: bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, tj, martin.lau,
ameryhung, kernel-team
Make sure 1) a timer callback can also reference the associated
struct_ops, and then make sure 2) the timer callback cannot get a
dangled pointer to the struct_ops when the map is freed.
The test schedules a timer callback from a struct_ops program since
struct_ops programs do not pin the map. It is possible for the timer
callback to run after the map is freed. The timer callback calls a
kfunc that runs .test_1() of the associated struct_ops, which should
return MAP_MAGIC when the map is still alive or -1 when the map is
gone.
The first subtest added in this patch schedules the timer callback to
run immediately, while the map is still alive. The second subtest added
schedules the callback to run 500ms after syscall_prog runs and then
frees the map right after syscall_prog runs. Both subtests then wait
until the callback runs to check the return of the kfunc.
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
.../bpf/prog_tests/test_struct_ops_assoc.c | 81 +++++++++++++++++++
.../bpf/progs/struct_ops_assoc_in_timer.c | 77 ++++++++++++++++++
2 files changed, 158 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_assoc_in_timer.c
diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c
index f69306cb8974..e09436442d07 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c
@@ -3,6 +3,7 @@
#include <test_progs.h>
#include "struct_ops_assoc.skel.h"
#include "struct_ops_assoc_reuse.skel.h"
+#include "struct_ops_assoc_in_timer.skel.h"
static void test_st_ops_assoc(void)
{
@@ -101,10 +102,90 @@ static void test_st_ops_assoc_reuse(void)
struct_ops_assoc_reuse__destroy(skel);
}
+static void test_st_ops_assoc_in_timer(void)
+{
+ struct struct_ops_assoc_in_timer *skel = NULL;
+ int err;
+
+ skel = struct_ops_assoc_in_timer__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "struct_ops_assoc_reuse__open"))
+ goto out;
+
+ err = bpf_program__assoc_struct_ops(skel->progs.syscall_prog,
+ skel->maps.st_ops_map, NULL);
+ ASSERT_OK(err, "bpf_program__assoc_struct_ops");
+
+ err = struct_ops_assoc_in_timer__attach(skel);
+ if (!ASSERT_OK(err, "struct_ops_assoc__attach"))
+ goto out;
+
+ /*
+ * Run .test_1 by calling kfunc bpf_kfunc_multi_st_ops_test_1_prog_arg() and checks
+ * the return value. .test_1 will also schedule timer_cb that runs .test_1 again
+ * immediately.
+ */
+ err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.syscall_prog), NULL);
+ ASSERT_OK(err, "bpf_prog_test_run_opts");
+
+ /* Check the return of the kfunc after timer_cb runs */
+ while (!READ_ONCE(skel->bss->timer_cb_run))
+ sched_yield();
+ ASSERT_EQ(skel->bss->timer_test_1_ret, 1234, "skel->bss->timer_test_1_ret");
+ ASSERT_EQ(skel->bss->test_err, 0, "skel->bss->test_err_a");
+out:
+ struct_ops_assoc_in_timer__destroy(skel);
+}
+
+static void test_st_ops_assoc_in_timer_no_uref(void)
+{
+ struct struct_ops_assoc_in_timer *skel = NULL;
+ struct bpf_link *link;
+ int err;
+
+ skel = struct_ops_assoc_in_timer__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "struct_ops_assoc_reuse__open"))
+ goto out;
+
+ err = bpf_program__assoc_struct_ops(skel->progs.syscall_prog,
+ skel->maps.st_ops_map, NULL);
+ ASSERT_OK(err, "bpf_program__assoc_struct_ops");
+
+ link = bpf_map__attach_struct_ops(skel->maps.st_ops_map);
+ if (!ASSERT_OK_PTR(link, "bpf_map__attach_struct_ops"))
+ goto out;
+
+ /*
+ * Run .test_1 by calling kfunc bpf_kfunc_multi_st_ops_test_1_prog_arg() and checks
+ * the return value. .test_1 will also schedule timer_cb that runs .test_1 again.
+ * timer_cb will run 500ms after syscall_prog runs, when the user space no longer
+ * holds a reference to st_ops_map.
+ */
+ skel->bss->timer_ns = 500000000;
+ err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.syscall_prog), NULL);
+ ASSERT_OK(err, "bpf_prog_test_run_opts");
+
+ /* Detach and close struct_ops map to cause it to be freed */
+ bpf_link__destroy(link);
+ close(bpf_program__fd(skel->progs.syscall_prog));
+ close(bpf_map__fd(skel->maps.st_ops_map));
+
+ /* Check the return of the kfunc after timer_cb runs */
+ while (!READ_ONCE(skel->bss->timer_cb_run))
+ sched_yield();
+ ASSERT_EQ(skel->bss->timer_test_1_ret, -1, "skel->bss->timer_test_1_ret");
+ ASSERT_EQ(skel->bss->test_err, 0, "skel->bss->test_err_a");
+out:
+ struct_ops_assoc_in_timer__destroy(skel);
+}
+
void test_struct_ops_assoc(void)
{
if (test__start_subtest("st_ops_assoc"))
test_st_ops_assoc();
if (test__start_subtest("st_ops_assoc_reuse"))
test_st_ops_assoc_reuse();
+ if (test__start_subtest("st_ops_assoc_in_timer"))
+ test_st_ops_assoc_in_timer();
+ if (test__start_subtest("st_ops_assoc_in_timer_no_uref"))
+ test_st_ops_assoc_in_timer_no_uref();
}
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_assoc_in_timer.c b/tools/testing/selftests/bpf/progs/struct_ops_assoc_in_timer.c
new file mode 100644
index 000000000000..9d4e427568b2
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/struct_ops_assoc_in_timer.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+#include "../test_kmods/bpf_testmod.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
+
+char _license[] SEC("license") = "GPL";
+
+struct elem {
+ struct bpf_timer timer;
+};
+
+struct {
+ __uint(type, BPF_MAP_TYPE_ARRAY);
+ __uint(max_entries, 1);
+ __type(key, int);
+ __type(value, struct elem);
+} array_map SEC(".maps");
+
+#define MAP_MAGIC 1234
+int recur;
+int test_err;
+int timer_ns;
+int timer_test_1_ret;
+int timer_cb_run;
+
+__noinline static int timer_cb(void *map, int *key, struct bpf_timer *timer)
+{
+ struct st_ops_args args = {};
+
+ recur++;
+ timer_test_1_ret = bpf_kfunc_multi_st_ops_test_1_prog_arg(&args, NULL);
+ recur--;
+
+ timer_cb_run++;
+
+ return 0;
+}
+
+SEC("struct_ops")
+int BPF_PROG(test_1, struct st_ops_args *args)
+{
+ struct bpf_timer *timer;
+ int key = 0;
+
+ if (!recur) {
+ timer = bpf_map_lookup_elem(&array_map, &key);
+ if (!timer)
+ return 0;
+
+ bpf_timer_init(timer, &array_map, 1);
+ bpf_timer_set_callback(timer, timer_cb);
+ bpf_timer_start(timer, timer_ns, 0);
+ }
+
+ return MAP_MAGIC;
+}
+
+SEC("syscall")
+int syscall_prog(void *ctx)
+{
+ struct st_ops_args args = {};
+ int ret;
+
+ ret = bpf_kfunc_multi_st_ops_test_1_prog_arg(&args, NULL);
+ if (ret != MAP_MAGIC)
+ test_err++;
+
+ return 0;
+}
+
+SEC(".struct_ops.link")
+struct bpf_testmod_multi_st_ops st_ops_map = {
+ .test_1 = (void *)test_1,
+};
--
2.47.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next v6 2/6] bpf: Support associating BPF program with struct_ops
2025-11-14 22:17 ` [PATCH bpf-next v6 2/6] bpf: Support associating BPF program with struct_ops Amery Hung
@ 2025-11-14 22:51 ` bot+bpf-ci
2025-11-17 21:39 ` Martin KaFai Lau
0 siblings, 1 reply; 12+ messages in thread
From: bot+bpf-ci @ 2025-11-14 22:51 UTC (permalink / raw)
To: ameryhung, bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, tj, martin.lau,
ameryhung, kernel-team, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 1768 bytes --]
> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index a41e6730e..626aa9197 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
[ ... ]
> @@ -811,6 +822,12 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
> bpf_link_init(&link->link, BPF_LINK_TYPE_STRUCT_OPS,
> &bpf_struct_ops_link_lops, prog, prog->expected_attach_type);
> *plink++ = &link->link;
> +
> + err = bpf_prog_assoc_struct_ops(prog, &st_map->map);
> + if (err) {
> + bpf_prog_put(prog);
> + goto reset_unlock;
> + }
Can this double-put the prog reference? In bpf_struct_ops_map_update_elem:
prog = bpf_prog_get(prog_fd); // refcnt = 1
bpf_link_init(..., prog, ...); // link takes ownership
*plink++ = &link->link; // link stored in st_map
err = bpf_prog_assoc_struct_ops(prog, ...);
if (err) {
bpf_prog_put(prog); // refcnt = 0, prog freed
goto reset_unlock;
}
Then at reset_unlock:
bpf_struct_ops_map_put_progs(st_map); // calls bpf_link_put(link)
The bpf_link_put will eventually call bpf_link_dealloc()->bpf_prog_put(link->prog),
attempting to put the already-freed prog reference. This looks like a use-after-free
if bpf_prog_assoc_struct_ops fails (which can happen when a non-struct_ops program
is already associated and returns -EBUSY).
Should the error path skip the bpf_prog_put and let bpf_struct_ops_map_put_progs
handle the cleanup via the link?
---
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/19379369447
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next v6 3/6] libbpf: Add support for associating BPF program with struct_ops
2025-11-14 22:17 ` [PATCH bpf-next v6 3/6] libbpf: Add support for " Amery Hung
@ 2025-11-14 22:58 ` bot+bpf-ci
0 siblings, 0 replies; 12+ messages in thread
From: bot+bpf-ci @ 2025-11-14 22:58 UTC (permalink / raw)
To: ameryhung, bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, tj, martin.lau,
ameryhung, kernel-team, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 3155 bytes --]
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index b66f5fbfb..21b57a629 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
[ ... ]
> +int bpf_prog_assoc_struct_ops(int prog_fd, int map_fd,
> + struct bpf_prog_assoc_struct_ops_opts *opts)
> +{
> + const size_t attr_sz = offsetofend(union bpf_attr, prog_assoc_struct_ops);
> + union bpf_attr attr;
> + int err;
> +
> + if (!OPTS_VALID(opts, bpf_prog_assoc_struct_ops_opts))
> + return libbpf_err(-EINVAL);
> +
> + memset(&attr, 0, attr_sz);
> + attr.prog_assoc_struct_ops.map_fd = map_fd;
> + attr.prog_assoc_struct_ops.prog_fd = prog_fd;
> + attr.prog_assoc_struct_ops.flags = OPTS_GET(opts, flags, 0);
> +
> + err = sys_bpf(BPF_PROG_ASSOC_STRUCT_OPS, &attr, attr_sz);
> + return libbpf_err_errno(err);
> +}
[ ... ]
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 706e7481b..1d5424276 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
[ ... ]
> +int bpf_program__assoc_struct_ops(struct bpf_program *prog, struct bpf_map *map,
> + struct bpf_prog_assoc_struct_ops_opts *opts)
> +{
> + int prog_fd, map_fd;
> +
> + prog_fd = bpf_program__fd(prog);
> + if (prog_fd < 0) {
> + pr_warn("prog '%s': can't associate BPF program without FD (was it loaded?)\n",
> + prog->name);
^^^^^^^^^^
Can this crash if prog is NULL? bpf_program__fd() handles NULL by
returning libbpf_err(-EINVAL), so prog_fd will be negative and we
enter this error path. Then prog->name dereferences the NULL pointer.
> + return -EINVAL;
> + }
> +
> + if (prog->type == BPF_PROG_TYPE_STRUCT_OPS) {
^^^^^^^^^
Similarly, is prog guaranteed non-NULL here? We know prog_fd is valid,
but that doesn't prove prog itself was checked.
> + pr_warn("prog '%s': can't associate struct_ops program\n", prog->name);
> + return -EINVAL;
> + }
> +
> + map_fd = bpf_map__fd(map);
> + if (map_fd < 0) {
> + pr_warn("map '%s': can't associate BPF map without FD (was it created?)\n", map->name);
^^^^^^^^^^
Same issue with map - bpf_map__fd() handles NULL by returning an error,
but then we dereference map->name in the error message.
> + return -EINVAL;
> + }
> +
> + if (!bpf_map__is_struct_ops(map)) {
^^^^^^^^^^^^^^^^^^^^^^^^
bpf_map__is_struct_ops() also dereferences map->def.type without a NULL
check, which would crash here if map is NULL.
> + pr_warn("map '%s': can't associate non-struct_ops map\n", map->name);
> + return -EINVAL;
> + }
> +
> + return bpf_prog_assoc_struct_ops(prog_fd, map_fd, opts);
> +}
Note: This follows the same pattern as existing functions like
bpf_program__pin() which also dereference prog->name without checking
prog for NULL. However, since the helper functions (bpf_program__fd,
bpf_map__fd) do handle NULL, it seems inconsistent to crash in the
error message when reporting that NULL was passed.
[ ... ]
---
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/19379369447
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next v6 1/6] bpf: Allow verifier to fixup kernel module kfuncs
2025-11-14 22:17 ` [PATCH bpf-next v6 1/6] bpf: Allow verifier to fixup kernel module kfuncs Amery Hung
@ 2025-11-14 23:06 ` bot+bpf-ci
0 siblings, 0 replies; 12+ messages in thread
From: bot+bpf-ci @ 2025-11-14 23:06 UTC (permalink / raw)
To: ameryhung, bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, tj, martin.lau,
ameryhung, kernel-team, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 2154 bytes --]
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 99d62d765..d7702b9c6 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -22416,8 +22416,7 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>
> if (!bpf_jit_supports_far_kfunc_call())
> insn->imm = BPF_CALL_IMM(desc->addr);
> - if (insn->off)
> - return 0;
> +
Are the special kfunc comparisons below safe for module kfuncs?
The code after this change compares desc->func_id against
special_kfunc_list[] entries without checking desc->offset. Since
special_kfunc_list[] contains BTF IDs from kernel BTF, and module kfuncs
have BTF IDs from their own module BTF, could a module kfunc with a BTF
ID that happens to match a special kernel kfunc ID be incorrectly
processed?
In fixup_kfunc_call():
desc = find_kfunc_desc(env->prog, insn->imm, insn->off);
[...]
if (desc->func_id == special_kfunc_list[KF_bpf_obj_new_impl] || ...)
The desc->func_id contains a BTF-local ID (set at line 3373), while
special_kfunc_list[] contains kernel BTF IDs. Module BTF IDs start from
1 independently, so collision is architecturally possible.
During verification, check_special_kfunc() at line 13691 checks "if
(meta->btf != btf_vmlinux) return 0;" which prevents aux_data fields
like kptr_struct_meta from being set for module kfuncs. But at fixup
time, if a module kfunc's BTF ID collides with a special kernel kfunc
ID, won't it enter these paths and read uninitialized aux_data fields?
Would adding "desc->offset == 0 &&" or "insn->off == 0 &&" to the
comparisons at lines 22420, 22437, 22460, 22481 ensure we only match
kernel kfuncs?
> if (desc->func_id == special_kfunc_list[KF_bpf_obj_new_impl] ||
> desc->func_id == special_kfunc_list[KF_bpf_percpu_obj_new_impl]) {
> struct btf_struct_meta *kptr_struct_meta = env->insn_aux_data[insn_idx].kptr_struct_meta;
[ ... ]
---
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/19379369447
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next v6 2/6] bpf: Support associating BPF program with struct_ops
2025-11-14 22:51 ` bot+bpf-ci
@ 2025-11-17 21:39 ` Martin KaFai Lau
2025-11-17 21:53 ` Amery Hung
0 siblings, 1 reply; 12+ messages in thread
From: Martin KaFai Lau @ 2025-11-17 21:39 UTC (permalink / raw)
To: ameryhung
Cc: netdev, alexei.starovoitov, andrii, daniel, tj, martin.lau,
kernel-team, ast, eddyz87, yonghong.song, clm, ihor.solodrai, bpf,
bot+bpf-ci
On 11/14/25 2:51 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
>> index a41e6730e..626aa9197 100644
>> --- a/kernel/bpf/bpf_struct_ops.c
>> +++ b/kernel/bpf/bpf_struct_ops.c
>
> [ ... ]
>
>> @@ -811,6 +822,12 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
>> bpf_link_init(&link->link, BPF_LINK_TYPE_STRUCT_OPS,
>> &bpf_struct_ops_link_lops, prog, prog->expected_attach_type);
>> *plink++ = &link->link;
>> +
>> + err = bpf_prog_assoc_struct_ops(prog, &st_map->map);
>> + if (err) {
>> + bpf_prog_put(prog);
>> + goto reset_unlock;
>> + }
>
> Can this double-put the prog reference? In bpf_struct_ops_map_update_elem:
>
> prog = bpf_prog_get(prog_fd); // refcnt = 1
> bpf_link_init(..., prog, ...); // link takes ownership
> *plink++ = &link->link; // link stored in st_map
> err = bpf_prog_assoc_struct_ops(prog, ...);
> if (err) {
> bpf_prog_put(prog); // refcnt = 0, prog freed
> goto reset_unlock;
> }
>
> Then at reset_unlock:
>
> bpf_struct_ops_map_put_progs(st_map); // calls bpf_link_put(link)
>
> The bpf_link_put will eventually call bpf_link_dealloc()->bpf_prog_put(link->prog),
> attempting to put the already-freed prog reference. This looks like a use-after-free
> if bpf_prog_assoc_struct_ops fails (which can happen when a non-struct_ops program
> is already associated and returns -EBUSY).
The ai-review should be correct in general on the double bpf_prog_put.
>
> Should the error path skip the bpf_prog_put and let bpf_struct_ops_map_put_progs
> handle the cleanup via the link?
bpf_prog_assoc_struct_ops will never return error for
BPF_PROG_TYPE_STRUCT_OPS. If that is the case, maybe completely remove
the err check.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next v6 2/6] bpf: Support associating BPF program with struct_ops
2025-11-17 21:39 ` Martin KaFai Lau
@ 2025-11-17 21:53 ` Amery Hung
0 siblings, 0 replies; 12+ messages in thread
From: Amery Hung @ 2025-11-17 21:53 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: netdev, alexei.starovoitov, andrii, daniel, tj, martin.lau,
kernel-team, ast, eddyz87, yonghong.song, clm, ihor.solodrai, bpf,
bot+bpf-ci
On Mon, Nov 17, 2025 at 1:39 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
>
>
> On 11/14/25 2:51 PM, bot+bpf-ci@kernel.org wrote:
> >> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> >> index a41e6730e..626aa9197 100644
> >> --- a/kernel/bpf/bpf_struct_ops.c
> >> +++ b/kernel/bpf/bpf_struct_ops.c
> >
> > [ ... ]
> >
> >> @@ -811,6 +822,12 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
> >> bpf_link_init(&link->link, BPF_LINK_TYPE_STRUCT_OPS,
> >> &bpf_struct_ops_link_lops, prog, prog->expected_attach_type);
> >> *plink++ = &link->link;
> >> +
> >> + err = bpf_prog_assoc_struct_ops(prog, &st_map->map);
> >> + if (err) {
> >> + bpf_prog_put(prog);
> >> + goto reset_unlock;
> >> + }
> >
> > Can this double-put the prog reference? In bpf_struct_ops_map_update_elem:
> >
> > prog = bpf_prog_get(prog_fd); // refcnt = 1
> > bpf_link_init(..., prog, ...); // link takes ownership
> > *plink++ = &link->link; // link stored in st_map
> > err = bpf_prog_assoc_struct_ops(prog, ...);
> > if (err) {
> > bpf_prog_put(prog); // refcnt = 0, prog freed
> > goto reset_unlock;
> > }
> >
> > Then at reset_unlock:
> >
> > bpf_struct_ops_map_put_progs(st_map); // calls bpf_link_put(link)
> >
> > The bpf_link_put will eventually call bpf_link_dealloc()->bpf_prog_put(link->prog),
> > attempting to put the already-freed prog reference. This looks like a use-after-free
> > if bpf_prog_assoc_struct_ops fails (which can happen when a non-struct_ops program
> > is already associated and returns -EBUSY).
>
> The ai-review should be correct in general on the double bpf_prog_put.
>
> >
> > Should the error path skip the bpf_prog_put and let bpf_struct_ops_map_put_progs
> > handle the cleanup via the link?
>
> bpf_prog_assoc_struct_ops will never return error for
> BPF_PROG_TYPE_STRUCT_OPS. If that is the case, maybe completely remove
> the err check.
Thanks for reviewing
Will remove the check and add comments about why the error can be ignored.
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-11-17 21:53 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-14 22:17 [PATCH bpf-next v6 0/7] Support associating BPF programs with struct_ops Amery Hung
2025-11-14 22:17 ` [PATCH bpf-next v6 1/6] bpf: Allow verifier to fixup kernel module kfuncs Amery Hung
2025-11-14 23:06 ` bot+bpf-ci
2025-11-14 22:17 ` [PATCH bpf-next v6 2/6] bpf: Support associating BPF program with struct_ops Amery Hung
2025-11-14 22:51 ` bot+bpf-ci
2025-11-17 21:39 ` Martin KaFai Lau
2025-11-17 21:53 ` Amery Hung
2025-11-14 22:17 ` [PATCH bpf-next v6 3/6] libbpf: Add support for " Amery Hung
2025-11-14 22:58 ` bot+bpf-ci
2025-11-14 22:17 ` [PATCH bpf-next v6 4/6] selftests/bpf: Test BPF_PROG_ASSOC_STRUCT_OPS command Amery Hung
2025-11-14 22:17 ` [PATCH bpf-next v6 5/6] selftests/bpf: Test ambiguous associated struct_ops Amery Hung
2025-11-14 22:17 ` [PATCH bpf-next v6 6/6] selftests/bpf: Test getting associated struct_ops in timer callback Amery Hung
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).