On 9/25/23 16:23, Martin KaFai Lau wrote:
On 9/20/23 8:59 AM, thinker.li@xxxxxxxxx wrote:
From: Kui-Feng Lee <thinker.li@xxxxxxxxx>
Ensure a module doesn't go away when a struct_ops object is still alive,
being a struct_ops type that is registered by the module.
Signed-off-by: Kui-Feng Lee <thinker.li@xxxxxxxxx>
---
include/linux/bpf.h | 1 +
kernel/bpf/bpf_struct_ops.c | 6 ++++++
2 files changed, 7 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 0776cb584b3f..faaec20156f1 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1627,6 +1627,7 @@ struct bpf_struct_ops {
int (*update)(void *kdata, void *old_kdata);
int (*validate)(void *kdata);
const struct btf *btf;
+ struct module *owner;
const struct btf_type *type;
const struct btf_type *value_type;
const char *name;
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index 7c2ef53687ef..ef8a1edec891 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -632,6 +632,8 @@ static void __bpf_struct_ops_map_free(struct
bpf_map *map)
static void bpf_struct_ops_map_free(struct bpf_map *map)
{
+ struct bpf_struct_ops_map *st_map = (struct bpf_struct_ops_map
*)map;
+
/* The struct_ops's function may switch to another struct_ops.
*
* For example, bpf_tcp_cc_x->init() may switch to
@@ -649,6 +651,7 @@ static void bpf_struct_ops_map_free(struct bpf_map
*map)
*/
synchronize_rcu_mult(call_rcu, call_rcu_tasks);
+ module_put(st_map->st_ops->owner);
__bpf_struct_ops_map_free(map);
}
@@ -673,6 +676,9 @@ static struct bpf_map
*bpf_struct_ops_map_alloc(union bpf_attr *attr)
if (!st_ops)
return ERR_PTR(-ENOTSUPP);
+ if (!try_module_get(st_ops->owner))
+ return ERR_PTR(-EINVAL);
The module can be gone at this point?
I don't think try_module_get is safe. btf_try_get_module should be used
instead.
At this point, it holds btf, but not module. Module can go away while
some one still holding a refcount to the btf.
And, you are right, I should use btf_try_get_module().
+
vt = st_ops->value_type;
if (attr->value_size != vt->size)
return ERR_PTR(-EINVAL);
