Re: [PATCH v3 15/15] NTB: ntb_transport: Remove clients before freeing transport resources

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ntb: use-after-free and DMA memory corruption in teardown due to bypassed wait loop
--

commit 51bc5e4115a3fe5b0d9105d1dfb2a458cfc0b016
Author: Koichiro Den <den@valinux.co.jp>
NTB: ntb_transport: Remove clients before freeing transport resources

This commit fixes a tear-down order issue where transport resources are
freed before unregistering clients. It modifies ntb_transport_free to
disable transport work and call ntb_bus_remove earlier. It also reorders
the transport link cleanup after queue freeing to prevent setup during
removal.

> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index af7f240479f4e..eef3a214d91a0 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
[ ... ]
> @@ -1497,11 +1499,10 @@ static void ntb_transport_free(struct ntb_client *self, struct ntb_dev *ndev)
>  			ntb_transport_free_queue(qp);
>  	}
>  
> +	ntb_transport_link_cleanup(nt);

[Severity: High]
Does this reordering inadvertently skip the DMA synchronization wait loop?

When ntb_transport_free_queue is called in the loop above, it frees the
queues and marks them as free by updating the bitmap:

void ntb_transport_free_queue(struct ntb_transport_qp *qp)
{
        ...
        qp->transport->qp_bitmap_free |= qp_bit;
}

Then, when ntb_transport_link_cleanup is called immediately after, it
evaluates the allocated queues using the updated bitmap:

static void ntb_transport_link_cleanup(struct ntb_transport_ctx *nt)
{
        ...
        qp_bitmap_alloc = nt->qp_bitmap & ~nt->qp_bitmap_free;
        
        for (i = 0; i < nt->qp_count; i++)
                if (qp_bitmap_alloc & BIT_ULL(i)) {
                        qp = &nt->qp_vec[i];
                        ntb_qp_link_cleanup(qp);
                        ...
}

Because the queues were just marked as free by ntb_transport_free_queue,
qp_bitmap_alloc evaluates to zero. This completely bypasses the loop that
calls ntb_qp_link_cleanup.

Bypassing ntb_qp_link_cleanup skips the critical DMA wait loop:

while (!ntb_transport_rx_idle(qp))
        fsleep(1000);

Without this synchronization, entries in rx_post_q are freed in
ntb_transport_free_queue while the hardware might still be actively
accessing them via DMA. Could this lead to a use-after-free or memory
corruption when ntb_rx_copy_callback later asynchronously executes and
operates on the freed queue entries?

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