Re: [PATCH 10/11] NTB: ntb_transport: Use atomic MW translation groups

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing work cancellation in probe error path leads to Use-After-Free.
--

--- Patch [10]: [PATCH 10/11] NTB: ntb_transport: Use atomic MW translation groups ---
commit 92a31c981341f2c994839699e7b4fa179c125291
Author: Koichiro Den <den@valinux.co.jp>

NTB: ntb_transport: Use atomic MW translation groups

This commit modifies the ntb_transport driver to allocate and program MWs
in a translation group as one contiguous unit. It queries each group,
allocates its backing once, and uses group set and clear operations.

> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index f59f926d4bfaa..2c7728898890c 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c

[ ... ]

> @@ -1291,6 +1366,22 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
>  
>  	max_mw_count_for_spads = (spad_count - MW0_SZ_HIGH) / 2;
>  	nt->mw_count = min(mw_count, max_mw_count_for_spads);
> +	if (nt->mw_count) {
> +		rc = ntb_mw_get_trans_group(ndev, PIDX, nt->mw_count - 1,
> +					    &group_first, &group_count);
> +		if (rc)
> +			goto err;
> +
> +		if (group_first < 0 || group_count <= 0 ||
> +		    (unsigned int)group_first >= nt->mw_count ||
> +		    (unsigned int)group_count !=
> +		    nt->mw_count - (unsigned int)group_first) {
> +			dev_err(&ndev->dev,
> +				"Scratchpad limit splits an MW translation group\n");
> +			rc = -EOPNOTSUPP;
> +			goto err;
> +		}
> +	}
>  
>  	nt->msi_spad_offset = nt->mw_count * 2 + MW0_SZ_HIGH;
>  

[Severity: High]
This is a pre-existing issue, but does the probe error path need to cancel
link_work?

Looking at ntb_transport_probe(), ntb_set_ctx() registers device callbacks:

    rc = ntb_set_ctx(ndev, nt, &ntb_transport_ops);
    if (rc)
        goto err2;

    INIT_LIST_HEAD(&nt->client_devs);
    rc = ntb_bus_init(nt);
    if (rc)
        goto err3;

If an interrupt fires immediately after ntb_set_ctx() registers the
callbacks, the link_event callback could queue nt->link_work on the system
workqueue.

If ntb_bus_init() subsequently fails, the error path jumps to err3 and
frees the context without explicitly canceling the work via
cancel_delayed_work_sync():

err3:
    ntb_clear_ctx(ndev);
err2:
    kfree(nt->qp_vec);
err1:
    while (i--) {
        mw = &nt->mw_vec[i];
        iounmap(mw->vbase);
    }
    kfree(nt->mw_vec);
err:
    kfree(nt);

Could this leave link_work queued on freed memory, leading to a
use-after-free when the work queue executes it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803180439.1963864-1-den@valinux.co.jp?part=10