* [PATCH bpf-next v4 0/6] Support associating BPF programs with struct_ops
@ 2025-10-24 21:29 Amery Hung
2025-10-24 21:29 ` [PATCH bpf-next v4 1/6] bpf: Allow verifier to fixup kernel module kfuncs Amery Hung
` (5 more replies)
0 siblings, 6 replies; 9+ messages in thread
From: Amery Hung @ 2025-10-24 21:29 UTC (permalink / raw)
To: bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, tj, martin.lau,
ameryhung, kernel-team
v4 -> v3
- 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/
---
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 exmample, 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 speical
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 guarantted that the struct_ops is attached. The
struct_ops implementer still need to take stepis 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
---
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 | 98 +++++++++
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 | 30 +++
tools/lib/bpf/libbpf.h | 16 ++
tools/lib/bpf/libbpf.map | 2 +
.../bpf/prog_tests/test_struct_ops_assoc.c | 190 ++++++++++++++++++
.../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 | 19 ++
.../bpf/test_kmods/bpf_testmod_kfunc.h | 1 +
18 files changed, 753 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] 9+ messages in thread
* [PATCH bpf-next v4 1/6] bpf: Allow verifier to fixup kernel module kfuncs
2025-10-24 21:29 [PATCH bpf-next v4 0/6] Support associating BPF programs with struct_ops Amery Hung
@ 2025-10-24 21:29 ` Amery Hung
2025-10-24 21:29 ` [PATCH bpf-next v4 2/6] bpf: Support associating BPF program with struct_ops Amery Hung
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Amery Hung @ 2025-10-24 21:29 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 e892df386eed..d5f1046d08b7 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -21889,8 +21889,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] 9+ messages in thread
* [PATCH bpf-next v4 2/6] bpf: Support associating BPF program with struct_ops
2025-10-24 21:29 [PATCH bpf-next v4 0/6] Support associating BPF programs with struct_ops Amery Hung
2025-10-24 21:29 ` [PATCH bpf-next v4 1/6] bpf: Allow verifier to fixup kernel module kfuncs Amery Hung
@ 2025-10-24 21:29 ` Amery Hung
2025-10-28 16:53 ` Andrii Nakryiko
2025-10-24 21:29 ` [PATCH bpf-next v4 3/6] libbpf: Add support for " Amery Hung
` (3 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Amery Hung @ 2025-10-24 21:29 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. The call must be paired with bpf_struct_ops_put() once the
caller is done with the struct_ops.
To make sure the returned pointer to be valid, the command increases the
refcount of the map for every associated non-struct_ops programs. For
struct_ops programs, since they do not increase the refcount of
struct_ops map, bpf_prog_get_assoc_struct_ops() has to bump the refcount
of the map to prevent a map from being freed while the program runs.
This can happen if a struct_ops program schedules a time callback that
runs after the struct_ops map is freed.
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 | 98 ++++++++++++++++++++++++++++++++++
kernel/bpf/core.c | 3 ++
kernel/bpf/syscall.c | 46 ++++++++++++++++
tools/include/uapi/linux/bpf.h | 17 ++++++
6 files changed, 197 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index a98c83346134..adef02556e95 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1710,6 +1710,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 {
@@ -2010,6 +2012,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
@@ -2057,6 +2062,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 ae83d8649ef1..41cacdbd7bd5 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,
};
@@ -1890,6 +1901,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..50ed675b4bfd 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;
@@ -801,6 +812,12 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
goto reset_unlock;
}
+ err = bpf_prog_assoc_struct_ops(prog, &st_map->map);
+ if (err) {
+ bpf_prog_put(prog);
+ goto reset_unlock;
+ }
+
link = kzalloc(sizeof(*link), GFP_USER);
if (!link) {
bpf_prog_put(prog);
@@ -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.
@@ -1173,6 +1192,7 @@ void bpf_struct_ops_put(const void *kdata)
bpf_map_put(&st_map->map);
}
+EXPORT_SYMBOL_GPL(bpf_struct_ops_put);
u32 bpf_struct_ops_id(const void *kdata)
{
@@ -1394,6 +1414,84 @@ 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_access_pointer(prog->aux->st_ops_assoc);
+
+ 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 {
+ 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_access_pointer(prog->aux->st_ops_assoc);
+
+ 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. Must be paired with bpf_struct_ops_put().
+ *
+ * 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 *map;
+
+ scoped_guard(rcu) {
+ map = rcu_dereference(aux->st_ops_assoc);
+ if (!map || map == BPF_PTR_POISON)
+ return NULL;
+
+ map = bpf_map_inc_not_zero(map);
+ if (IS_ERR(map))
+ return NULL;
+ }
+
+ st_map = (struct bpf_struct_ops_map *)map;
+
+ if (smp_load_acquire(&st_map->kvalue.common.state) == BPF_STRUCT_OPS_STATE_INIT) {
+ bpf_map_put(map);
+ 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 a48fa86f82a7..c40fc1e50934 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -6092,6 +6092,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;
@@ -6231,6 +6274,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 ae83d8649ef1..41cacdbd7bd5 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,
};
@@ -1890,6 +1901,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] 9+ messages in thread
* [PATCH bpf-next v4 3/6] libbpf: Add support for associating BPF program with struct_ops
2025-10-24 21:29 [PATCH bpf-next v4 0/6] Support associating BPF programs with struct_ops Amery Hung
2025-10-24 21:29 ` [PATCH bpf-next v4 1/6] bpf: Allow verifier to fixup kernel module kfuncs Amery Hung
2025-10-24 21:29 ` [PATCH bpf-next v4 2/6] bpf: Support associating BPF program with struct_ops Amery Hung
@ 2025-10-24 21:29 ` Amery Hung
2025-10-24 21:29 ` [PATCH bpf-next v4 4/6] selftests/bpf: Test BPF_PROG_ASSOC_STRUCT_OPS command Amery Hung
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Amery Hung @ 2025-10-24 21:29 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 | 30 ++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 16 ++++++++++++++++
tools/lib/bpf/libbpf.map | 2 ++
5 files changed, 88 insertions(+)
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 339b19797237..885b0f891443 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 f92083f51bdb..863372bfde23 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -13895,6 +13895,36 @@ 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;
+
+ 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;
+ }
+
+ 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..45720b7c2aaa 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 error code; or 0 if no error occurred.
+ */
+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] 9+ messages in thread
* [PATCH bpf-next v4 4/6] selftests/bpf: Test BPF_PROG_ASSOC_STRUCT_OPS command
2025-10-24 21:29 [PATCH bpf-next v4 0/6] Support associating BPF programs with struct_ops Amery Hung
` (2 preceding siblings ...)
2025-10-24 21:29 ` [PATCH bpf-next v4 3/6] libbpf: Add support for " Amery Hung
@ 2025-10-24 21:29 ` Amery Hung
2025-10-24 21:29 ` [PATCH bpf-next v4 5/6] selftests/bpf: Test ambiguous associated struct_ops Amery Hung
2025-10-24 21:29 ` [PATCH bpf-next v4 6/6] selftests/bpf: Test getting associated struct_ops in timer callback Amery Hung
5 siblings, 0 replies; 9+ messages in thread
From: Amery Hung @ 2025-10-24 21:29 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 | 19 ++++
.../bpf/test_kmods/bpf_testmod_kfunc.h | 1 +
4 files changed, 197 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 6df6475f5dbc..8f6be2ced4c6 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
@@ -1101,6 +1101,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)
@@ -1143,6 +1144,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)
@@ -1604,6 +1606,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;
@@ -1619,6 +1622,22 @@ 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);
+ bpf_struct_ops_put(st_ops);
+ }
+
+ 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] 9+ messages in thread
* [PATCH bpf-next v4 5/6] selftests/bpf: Test ambiguous associated struct_ops
2025-10-24 21:29 [PATCH bpf-next v4 0/6] Support associating BPF programs with struct_ops Amery Hung
` (3 preceding siblings ...)
2025-10-24 21:29 ` [PATCH bpf-next v4 4/6] selftests/bpf: Test BPF_PROG_ASSOC_STRUCT_OPS command Amery Hung
@ 2025-10-24 21:29 ` Amery Hung
2025-10-24 21:29 ` [PATCH bpf-next v4 6/6] selftests/bpf: Test getting associated struct_ops in timer callback Amery Hung
5 siblings, 0 replies; 9+ messages in thread
From: Amery Hung @ 2025-10-24 21:29 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] 9+ messages in thread
* [PATCH bpf-next v4 6/6] selftests/bpf: Test getting associated struct_ops in timer callback
2025-10-24 21:29 [PATCH bpf-next v4 0/6] Support associating BPF programs with struct_ops Amery Hung
` (4 preceding siblings ...)
2025-10-24 21:29 ` [PATCH bpf-next v4 5/6] selftests/bpf: Test ambiguous associated struct_ops Amery Hung
@ 2025-10-24 21:29 ` Amery Hung
5 siblings, 0 replies; 9+ messages in thread
From: Amery Hung @ 2025-10-24 21:29 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 | 80 +++++++++++++++++++
.../bpf/progs/struct_ops_assoc_in_timer.c | 77 ++++++++++++++++++
2 files changed, 157 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..3a08d3afc0c6 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,89 @@ 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 syscall_prog that calls .test_1 and checks return */
+ err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.syscall_prog), NULL);
+ ASSERT_OK(err, "bpf_prog_test_run_opts");
+
+ /*
+ * .test_1 has scheduled timer_cb that calls bpf_kfunc_multi_st_ops_test_1_prog_arg()
+ * again. Check the return of the kfunc after timer_cb run.
+ */
+ 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_after_detach(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;
+
+ /* timer_cb will run 500ms after syscall_prog_run when st_ops_map is gone */
+ skel->bss->timer_ns = 500000000;
+
+ /* run syscall_prog that calls .test_1 and checks return */
+ err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.syscall_prog), NULL);
+ ASSERT_OK(err, "bpf_prog_test_run_opts");
+
+ /* detach and free struct_ops map */
+ bpf_link__destroy(link);
+ close(bpf_program__fd(skel->progs.syscall_prog));
+ close(bpf_map__fd(skel->maps.st_ops_map));
+
+ /*
+ * .test_1 has scheduled timer_cb that calls bpf_kfunc_multi_st_ops_test_1_prog_arg()
+ * again. Check the return of the kfunc after timer_cb run.
+ */
+ 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_after_detach"))
+ test_st_ops_assoc_in_timer_after_detach();
}
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] 9+ messages in thread
* Re: [PATCH bpf-next v4 2/6] bpf: Support associating BPF program with struct_ops
2025-10-24 21:29 ` [PATCH bpf-next v4 2/6] bpf: Support associating BPF program with struct_ops Amery Hung
@ 2025-10-28 16:53 ` Andrii Nakryiko
2025-10-31 22:09 ` Amery Hung
0 siblings, 1 reply; 9+ messages in thread
From: Andrii Nakryiko @ 2025-10-28 16:53 UTC (permalink / raw)
To: Amery Hung
Cc: bpf, netdev, alexei.starovoitov, andrii, daniel, tj, martin.lau,
kernel-team
On Fri, Oct 24, 2025 at 2:29 PM Amery Hung <ameryhung@gmail.com> wrote:
>
> 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. The call must be paired with bpf_struct_ops_put() once the
> caller is done with the struct_ops.
>
> To make sure the returned pointer to be valid, the command increases the
> refcount of the map for every associated non-struct_ops programs. For
> struct_ops programs, since they do not increase the refcount of
> struct_ops map, bpf_prog_get_assoc_struct_ops() has to bump the refcount
> of the map to prevent a map from being freed while the program runs.
> This can happen if a struct_ops program schedules a time callback that
> runs after the struct_ops map is freed.
>
> 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 | 98 ++++++++++++++++++++++++++++++++++
> kernel/bpf/core.c | 3 ++
> kernel/bpf/syscall.c | 46 ++++++++++++++++
> tools/include/uapi/linux/bpf.h | 17 ++++++
> 6 files changed, 197 insertions(+)
>
[...]
> @@ -1394,6 +1414,84 @@ 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_access_pointer(prog->aux->st_ops_assoc);
we don't have RCU lock here, can this trigger lockdep warnings due to
rcu_access_pointer() use?
> +
> + if (st_ops_assoc && st_ops_assoc == map)
> + return 0;
> +
> + if (st_ops_assoc) {
> + if (prog->type != BPF_PROG_TYPE_STRUCT_OPS)
> + return -EBUSY;
> +
put st_ops_assoc map (if it's not BPF_PTR_POISON already, of course),
otherwise we are leaking refcount
pw-bot: cr
> + rcu_assign_pointer(prog->aux->st_ops_assoc, BPF_PTR_POISON);
> + } else {
> + 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_access_pointer(prog->aux->st_ops_assoc);
> +
> + 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. Must be paired with bpf_struct_ops_put().
> + *
> + * 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 *map;
> +
> + scoped_guard(rcu) {
> + map = rcu_dereference(aux->st_ops_assoc);
> + if (!map || map == BPF_PTR_POISON)
> + return NULL;
> +
> + map = bpf_map_inc_not_zero(map);
I think this is buggy. When timer callback happens, the map can be
long gone, and its underlying memory reused for something else. So
this bpf_map_inc_not_zero() can crash or just corrupt some memory. RCU
inside this function doesn't do much for us, it happens way too late.
It's also suboptimal that we now require callers of
bpf_prog_get_assoc_struct_ops() to do manual ref put.
Have you considered getting prog->aux->st_ops_assoc ref incremented
when scheduling async callback instead? Then we won't need all this
hackery and caller will just be working with borrowed map reference?
> + if (IS_ERR(map))
> + return NULL;
> + }
> +
> + st_map = (struct bpf_struct_ops_map *)map;
> +
> + if (smp_load_acquire(&st_map->kvalue.common.state) == BPF_STRUCT_OPS_STATE_INIT) {
> + bpf_map_put(map);
> + 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)
> {
[...]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v4 2/6] bpf: Support associating BPF program with struct_ops
2025-10-28 16:53 ` Andrii Nakryiko
@ 2025-10-31 22:09 ` Amery Hung
0 siblings, 0 replies; 9+ messages in thread
From: Amery Hung @ 2025-10-31 22:09 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, netdev, alexei.starovoitov, andrii, daniel, tj, martin.lau,
kernel-team
On Tue, Oct 28, 2025 at 9:53 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Oct 24, 2025 at 2:29 PM Amery Hung <ameryhung@gmail.com> wrote:
> >
> > 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. The call must be paired with bpf_struct_ops_put() once the
> > caller is done with the struct_ops.
> >
> > To make sure the returned pointer to be valid, the command increases the
> > refcount of the map for every associated non-struct_ops programs. For
> > struct_ops programs, since they do not increase the refcount of
> > struct_ops map, bpf_prog_get_assoc_struct_ops() has to bump the refcount
> > of the map to prevent a map from being freed while the program runs.
> > This can happen if a struct_ops program schedules a time callback that
> > runs after the struct_ops map is freed.
> >
> > 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 | 98 ++++++++++++++++++++++++++++++++++
> > kernel/bpf/core.c | 3 ++
> > kernel/bpf/syscall.c | 46 ++++++++++++++++
> > tools/include/uapi/linux/bpf.h | 17 ++++++
> > 6 files changed, 197 insertions(+)
> >
>
> [...]
>
> > @@ -1394,6 +1414,84 @@ 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_access_pointer(prog->aux->st_ops_assoc);
>
> we don't have RCU lock here, can this trigger lockdep warnings due to
> rcu_access_pointer() use?
>
> > +
> > + if (st_ops_assoc && st_ops_assoc == map)
> > + return 0;
> > +
> > + if (st_ops_assoc) {
> > + if (prog->type != BPF_PROG_TYPE_STRUCT_OPS)
> > + return -EBUSY;
> > +
>
> put st_ops_assoc map (if it's not BPF_PTR_POISON already, of course),
> otherwise we are leaking refcount
struct_ops programs do not take refcount on struct_ops map, so we
shouldn't need to drop refcount here.
>
> pw-bot: cr
>
> > + rcu_assign_pointer(prog->aux->st_ops_assoc, BPF_PTR_POISON);
> > + } else {
> > + 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_access_pointer(prog->aux->st_ops_assoc);
> > +
> > + 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. Must be paired with bpf_struct_ops_put().
> > + *
> > + * 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 *map;
> > +
> > + scoped_guard(rcu) {
> > + map = rcu_dereference(aux->st_ops_assoc);
> > + if (!map || map == BPF_PTR_POISON)
> > + return NULL;
> > +
> > + map = bpf_map_inc_not_zero(map);
>
> I think this is buggy. When timer callback happens, the map can be
> long gone, and its underlying memory reused for something else. So
> this bpf_map_inc_not_zero() can crash or just corrupt some memory. RCU
> inside this function doesn't do much for us, it happens way too late.
>
> It's also suboptimal that we now require callers of
> bpf_prog_get_assoc_struct_ops() to do manual ref put.
>
> Have you considered getting prog->aux->st_ops_assoc ref incremented
> when scheduling async callback instead? Then we won't need all this
> hackery and caller will just be working with borrowed map reference?
>
Will change kfunc facing APIs to not expose struct_map lifecycle management.
> > + if (IS_ERR(map))
> > + return NULL;
> > + }
> > +
> > + st_map = (struct bpf_struct_ops_map *)map;
> > +
> > + if (smp_load_acquire(&st_map->kvalue.common.state) == BPF_STRUCT_OPS_STATE_INIT) {
> > + bpf_map_put(map);
> > + 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)
> > {
>
> [...]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-10-31 22:09 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-24 21:29 [PATCH bpf-next v4 0/6] Support associating BPF programs with struct_ops Amery Hung
2025-10-24 21:29 ` [PATCH bpf-next v4 1/6] bpf: Allow verifier to fixup kernel module kfuncs Amery Hung
2025-10-24 21:29 ` [PATCH bpf-next v4 2/6] bpf: Support associating BPF program with struct_ops Amery Hung
2025-10-28 16:53 ` Andrii Nakryiko
2025-10-31 22:09 ` Amery Hung
2025-10-24 21:29 ` [PATCH bpf-next v4 3/6] libbpf: Add support for " Amery Hung
2025-10-24 21:29 ` [PATCH bpf-next v4 4/6] selftests/bpf: Test BPF_PROG_ASSOC_STRUCT_OPS command Amery Hung
2025-10-24 21:29 ` [PATCH bpf-next v4 5/6] selftests/bpf: Test ambiguous associated struct_ops Amery Hung
2025-10-24 21:29 ` [PATCH bpf-next v4 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).