Re: [PATCH] blk-iocost: add BPF struct_ops cost model support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: premature removal from lifecycle list causes map/memory leaks
--

commit e92f1828b80c32e5c3b26c9b38e384109bedf0de
Author: Tao Cui <cuitao@kylinos.cn>

blk-iocost: add BPF struct_ops cost model support

Add the iocost_model_ops struct_ops: a bound BPF model fully replaces
the builtin linear model on a device.

> diff --git a/block/blk-iocost-bpf.c b/block/blk-iocost-bpf.c
> new file mode 100644
> index 0000000000000..7f1f9a2e51cb2
> --- /dev/null
> +++ b/block/blk-iocost-bpf.c

[ ... ]

> +void iocost_bpf_model_put(const struct iocost_model_ops *ops)
> +{
> +	struct iocost_bpf_model *m;
> +
> +	mutex_lock(&iocost_bpf_reg_lock);
> +	list_for_each_entry(m, &iocost_bpf_lifecycle, lifecycle) {
> +		if (m->ops == ops)
> +			break;
> +	}
> +	if (&m->lifecycle != &iocost_bpf_lifecycle) {
[Severity: High]
If a single registered BPF iocost model is bound to multiple block devices
and one of them unbinds it, does this skip the cleanup block entirely on
subsequent unbinds?

Because the model is prematurely removed from the lifecycle list below,
subsequent unbinds will fail to find it in the loop above. 

This causes iocost_bpf_model_put() to return early, permanently leaking BPF
struct_ops map references and iocost_bpf_model structures, as well as missing
blkcg online/offline notifications for active bindings.

> +		bool freed = refcount_dec_and_test(&m->refs);
> +
> +		/* leave the notify list with the last binding: the
> +		 * remaining ref is either the registry's (node still on
> +		 * iocost_bpf_models) or zero and the node is freed
> +		 */
> +		if (freed || !list_empty(&m->list))
> +			list_del(&m->lifecycle);
[Severity: High]
Can this condition cause the model to be removed from the lifecycle list on
the very first unbind?

The condition !list_empty(&m->list) evaluates to true unconditionally for
any registered model because m->list remains actively linked in the
registry (iocost_bpf_models) until it is unregistered.

> +		mutex_unlock(&iocost_bpf_reg_lock);
> +		bpf_struct_ops_put(ops);
> +		if (freed)
> +			kfree(m);
> +		return;
> +	}
> +	mutex_unlock(&iocost_bpf_reg_lock);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916071740.1068341-2-cui.tao@linux.dev?part=1