Re: [PATCH bpf v2] bpf: Release module BTF IDR before module unload

[Martin KaFai Lau <martin.lau@linux.dev>]

On 3/12/26 11:06 AM, Kumar Kartikeya Dwivedi wrote:
> 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: 36e68442d1af ("bpf: Load and verify kernel module BTFs")
> 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>
> ---
> Changelog:
> v1 -> v2
> v1: https://lore.kernel.org/bpf/20260312002025.2495953-1-memxor@gmail.com
> 
>   * Remove special case from btf_free_id(), and call it unconditionally. (Alexei)
> ---
>   kernel/bpf/btf.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 4872d2a6c42d..d08ae973df69 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -1788,6 +1788,13 @@ static void btf_free_id(struct btf *btf)
>   	 */
>   	spin_lock_irqsave(&btf_idr_lock, flags);
>   	idr_remove(&btf_idr, btf->id);
> +	/*
> +	 * Clear the id here to make this function idempotent, since it will get
> +	 * called a couple of times for module BTFs: on module unload, and then
> +	 * the final btf_put(). btf_alloc_id() starts IDs with 1, so we can use
> +	 * 0 as sentinel value.
> +	 */
> +	btf->id = 0;


Acked-by: Martin KaFai Lau <martin.lau@kernel.org>

A minor nit. Theoretically, it probably needs WRITE/READ_ONCE().

>   	spin_unlock_irqrestore(&btf_idr_lock, flags);
>   }
> 
> @@ -8382,6 +8389,13 @@ static int btf_module_notify(struct notifier_block *nb, unsigned long op,
>   			if (btf_mod->module != module)
>   				continue;
> 
> +			/*
> +			 * For modules, we do the freeing of BTF IDR as soon as
> +			 * module goes away to disable BTF discovery, since the
> +			 * btf_try_get_module() on such BTFs will fail. This may
> +			 * be called again on btf_put(), but it's ok to do so.
> +			 */
> +			btf_free_id(btf_mod->btf);
>   			list_del(&btf_mod->list);
>   			if (btf_mod->sysfs_attr)
>   				sysfs_remove_bin_file(btf_kobj, btf_mod->sysfs_attr);
> --
> 2.52.0
>