[PATCH] wifi: libertas_tf: kill shared URB before resubmitting it

[Steve deRosier]

On Thu, Jun 11, 2026 at 8:19 AM Runyu Xiao <runyu.xiao at seu.edu.cn> wrote:
>
> libertas_tf's usb_tx_block() reuses a shared send URB and immediately
> does usb_fill_bulk_urb() plus usb_submit_urb() on it. Depending on the
> caller, that shared carrier is either cardp->tx_urb or cardp->cmd_urb.
> There is no patch-local usb_kill_urb() before reuse, and the file-local
> completion path provides no busy flag, completion, or other ownership
> handoff that would make active reuse safe.
>
> A running system can reach this through if_usb_host_to_card() for normal
> data or command traffic, if_usb_issue_boot_command() for firmware boot
> commands, and if_usb_send_fw_pkt() for firmware download packets. Those
> paths all feed back into the same helper, so a second submission can
> refill and resubmit an URB while the previous transfer is still active.
>
> The issue was found by our static analysis tool and manually audited on
> Linux v6.18.21. It was further validated with a focused QEMU no-device KCSAN
> harness, which reproduced active reuse of both shared carriers:
> cardp->tx_urb through if_usb_host_to_card(), and cardp->cmd_urb through
> if_usb_issue_boot_command() and if_usb_send_fw_pkt().
>
> Call usb_kill_urb(urb) after selecting the shared target URB and before
> refilling it, so both tx_urb and cmd_urb are quiesced before reuse.
>
> Fixes: c305a19a0d0a ("libertas_tf: usb specific functions")
> Cc: stable at vger.kernel.org
> Signed-off-by: Runyu Xiao <runyu.xiao at seu.edu.cn>
> ---
>  drivers/net/wireless/marvell/libertas_tf/if_usb.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/marvell/libertas_tf/if_usb.c b/drivers/net/wireless/marvell/libertas_tf/if_usb.c
> index 5662a244f82a..7542956d3c47 100644
> --- a/drivers/net/wireless/marvell/libertas_tf/if_usb.c
> +++ b/drivers/net/wireless/marvell/libertas_tf/if_usb.c
> @@ -387,6 +387,8 @@ static int usb_tx_block(struct if_usb_card *cardp, uint8_t *payload,
>         else
>                 urb = cardp->cmd_urb;
>
> +       usb_kill_urb(urb);
> +
>         usb_fill_bulk_urb(urb, cardp->udev,
>                           usb_sndbulkpipe(cardp->udev,
>                                           cardp->ep_out),
> --
> 2.34.1

So, If I'm reading this right, you've basically compile checked and
such on this, but you haven't actually tried this on real hardware. If
you had, you'd have noticed it splats all over the place.
`usb_kill_urb()` can not be called from within an atomic context and 3
of the 5 contexts where this is called are atomic.

You've gone and fixed a very unlikely but theoretical race condition
in a way that causes actual real damage.

This is targeting an old Marvell 8388 which is a 802.11b/g chip. With
a custom thin-firmware created by Cozybit for OLPC and AFAIK only ever
used on those units. While I appreciate the "fix it for sake of
correctness" (if indeed this would fix it, which clearly it doesn't),
I'm not sure what the value proposition is for fixing something that
basically isn't used anymore.

Anyway, this is a hard NACK.

- Steve