Re: [PATCH v3 04/15] NTB: ntb_transport: Avoid deadlock when cancelling link work

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ntb: TOCTOU race in link cleanup permanently breaks link
--

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

NTB: ntb_transport: Avoid deadlock when cancelling link work

This patch moves the conditional cancellation of the link work outside of
the link_event_lock to avoid a deadlock when ntb_transport_link_work()
retries. QP cleanup and MW release are kept under the lock.

> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -962,6 +962,11 @@ static void ntb_transport_link_cleanup(struct ntb_transport_ctx *nt)
>  	u64 qp_bitmap_alloc;
>  	unsigned int i, count;
>  
> +	if (!atomic_read(&nt->link_is_up))
> +		cancel_delayed_work_sync(&nt->link_work);
> +
> +	guard(mutex)(&nt->link_event_lock);

[Severity: High]
Does moving the lock to after the cancellation check create a race condition
where the link state can become permanently broken?

If the hardware link flaps quickly, ntb_transport_link_cleanup() can be
queued, followed by ntb_transport_link_work(). When
ntb_transport_link_cleanup() evaluates the condition here, it might skip
cancellation or cancel an old work item, and then wait for the mutex.

The newly queued ntb_transport_link_work() could then win the lock,
establish the link, and drop the lock. Afterward,
ntb_transport_link_cleanup() acquires the lock and unconditionally tears
down the newly established QPs. Because the hardware link is already up,
no further interrupts will arrive to restart the software link, leaving
it unresponsive.

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