Re: [PATCH v2 09/14] NTB: ntb_transport: Drain RX tasklets during link cleanup

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] TOCTOU race in ntb_qp_link_cleanup() on qp->event_handler leading to NULL pointer dereference or execution of unloaded module
--

commit c58f5dc864a1fc465af61fd23b23c2a8badd19e7
Author: Koichiro Den <den@valinux.co.jp>

NTB: ntb_transport: Drain RX tasklets during link cleanup

This commit drains the RX tasklets during QP link cleanup to prevent them
from continuing to process the ring while the QP is reset. It uses
disable_delayed_work_sync() and tasklet_kill() before bringing down the QP
link, then re-enables the delayed work.

Cc: stable@vger.kernel.org

> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 45d4365becac5..36797ea3ff45c 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -956,11 +956,16 @@ static void ntb_qp_link_cleanup(struct ntb_transport_qp *qp)
>  
>  	dev_info(&pdev->dev, "qp %d: Link Cleanup\n", qp->qp_num);
>  
> -	cancel_delayed_work_sync(&qp->link_work);
> +	disable_delayed_work_sync(&qp->link_work);
> +	ntb_transport_set_qp_active(qp, false);
> +	tasklet_kill(&qp->rxc_db_work);
> +
>  	ntb_qp_link_down_reset(qp);
>  
>  	if (qp->event_handler)
>  		qp->event_handler(qp->cb_data, qp->link_is_up);

[Severity: High]
This is a pre-existing issue, but is this check of qp->event_handler
vulnerable to a TOCTOU race with client driver unbinding?

Looking at ntb_qp_link_cleanup() called directly from
ntb_transport_link_cleanup_work(), it evaluates if (qp->event_handler)
without any transport-level locks.

Concurrently, if a user unbinds the client driver, ntb_transport_free_queue()
runs and nullifies these fields:

drivers/ntb/ntb_transport.c:ntb_transport_free_queue() {
    ...
    qp->cb_data = NULL;
    qp->rx_handler = NULL;
    qp->tx_handler = NULL;
    qp->event_handler = NULL;
    ...
}

If the nullification in ntb_transport_free_queue() happens after the
if (qp->event_handler) check but before the call in ntb_qp_link_cleanup(),
could this execute a NULL pointer or unloaded module code?

> +
> +	enable_delayed_work(&qp->link_work);
>  }

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