Skip to content

Commit 146bd2a

Browse files
kkdwvdAlexei Starovoitov
authored andcommitted
bpf: Release module BTF IDR before module unload
Gregory reported in [0] that the global_map_resize test when run in repeatedly ends up failing during program load. This stems from the fact that BTF reference has not dropped to zero after the previous run's module is unloaded, and the older module's BTF is still discoverable and visible. Later, in libbpf, load_module_btfs() will find the ID for this stale BTF, open its fd, and then it will be used during program load where later steps taking module reference using btf_try_get_module() fail since the underlying module for the BTF is gone. Logically, once a module is unloaded, it's associated BTF artifacts should become hidden. The BTF object inside the kernel may still remain alive as long its reference counts are alive, but it should no longer be discoverable. To fix this, let us call btf_free_id() from the MODULE_STATE_GOING case for the module unload to free the BTF associated IDR entry, and disable its discovery once module unload returns to user space. If a race happens during unload, the outcome is non-deterministic anyway. However, user space should be able to rely on the guarantee that once it has synchronously established a successful module unload, no more stale artifacts associated with this module can be obtained subsequently. Note that we must be careful to not invoke btf_free_id() in btf_put() when btf_is_module() is true now. There could be a window where the module unload drops a non-terminal reference, frees the IDR, but the same ID gets reused and the second unconditional btf_free_id() ends up releasing an unrelated entry. To avoid a special case for btf_is_module() case, set btf->id to zero to make btf_free_id() idempotent, such that we can unconditionally invoke it from btf_put(), and also from the MODULE_STATE_GOING case. Since zero is an invalid IDR, the idr_remove() should be a noop. Note that we can be sure that by the time we reach final btf_put() for btf_is_module() case, the btf_free_id() is already done, since the module itself holds the BTF reference, and it will call this function for the BTF before dropping its own reference. [0]: https://lore.kernel.org/bpf/cover.1773170190.git.grbell@redhat.com Fixes: 36e6844 ("bpf: Load and verify kernel module BTFs") Acked-by: Martin KaFai Lau <martin.lau@kernel.org> Suggested-by: Martin KaFai Lau <martin.lau@kernel.org> Reported-by: Gregory Bell <grbell@redhat.com> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> Link: https://lore.kernel.org/r/20260312205307.1346991-1-memxor@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
1 parent e06e6b8 commit 146bd2a

1 file changed

Lines changed: 20 additions & 4 deletions

File tree

kernel/bpf/btf.c

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1787,7 +1787,16 @@ static void btf_free_id(struct btf *btf)
17871787
* of the _bh() version.
17881788
*/
17891789
spin_lock_irqsave(&btf_idr_lock, flags);
1790-
idr_remove(&btf_idr, btf->id);
1790+
if (btf->id) {
1791+
idr_remove(&btf_idr, btf->id);
1792+
/*
1793+
* Clear the id here to make this function idempotent, since it will get
1794+
* called a couple of times for module BTFs: on module unload, and then
1795+
* the final btf_put(). btf_alloc_id() starts IDs with 1, so we can use
1796+
* 0 as sentinel value.
1797+
*/
1798+
WRITE_ONCE(btf->id, 0);
1799+
}
17911800
spin_unlock_irqrestore(&btf_idr_lock, flags);
17921801
}
17931802

@@ -8115,7 +8124,7 @@ static void bpf_btf_show_fdinfo(struct seq_file *m, struct file *filp)
81158124
{
81168125
const struct btf *btf = filp->private_data;
81178126

8118-
seq_printf(m, "btf_id:\t%u\n", btf->id);
8127+
seq_printf(m, "btf_id:\t%u\n", READ_ONCE(btf->id));
81198128
}
81208129
#endif
81218130

@@ -8197,7 +8206,7 @@ int btf_get_info_by_fd(const struct btf *btf,
81978206
if (copy_from_user(&info, uinfo, info_copy))
81988207
return -EFAULT;
81998208

8200-
info.id = btf->id;
8209+
info.id = READ_ONCE(btf->id);
82018210
ubtf = u64_to_user_ptr(info.btf);
82028211
btf_copy = min_t(u32, btf->data_size, info.btf_size);
82038212
if (copy_to_user(ubtf, btf->data, btf_copy))
@@ -8260,7 +8269,7 @@ int btf_get_fd_by_id(u32 id)
82608269

82618270
u32 btf_obj_id(const struct btf *btf)
82628271
{
8263-
return btf->id;
8272+
return READ_ONCE(btf->id);
82648273
}
82658274

82668275
bool btf_is_kernel(const struct btf *btf)
@@ -8382,6 +8391,13 @@ static int btf_module_notify(struct notifier_block *nb, unsigned long op,
83828391
if (btf_mod->module != module)
83838392
continue;
83848393

8394+
/*
8395+
* For modules, we do the freeing of BTF IDR as soon as
8396+
* module goes away to disable BTF discovery, since the
8397+
* btf_try_get_module() on such BTFs will fail. This may
8398+
* be called again on btf_put(), but it's ok to do so.
8399+
*/
8400+
btf_free_id(btf_mod->btf);
83858401
list_del(&btf_mod->list);
83868402
if (btf_mod->sysfs_attr)
83878403
sysfs_remove_bin_file(btf_kobj, btf_mod->sysfs_attr);

0 commit comments

Comments
 (0)