[PATCH] ieee802154: atusb: drop the URB reference after a failed resubmit

[PATCH] ieee802154: atusb: drop the URB reference after a failed resubmit

[Wentao Liang]

usb_get_from_anchor() returns the URB with a reference held by the
caller.  When atusb_submit_rx_urb() fails, that reference is not consumed,
and re-anchoring the URB takes an additional one.  The caller's reference
is then never dropped.

Release it after putting the URB back on the idle anchor.

Fixes: 7490b008d123 ("ieee802154: add support for atusb transceiver")
Cc: stable@vger.kernel.org
Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/net/ieee802154/atusb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
index 5f7fc4ee7a07..c924f17171f4 100644
--- a/drivers/net/ieee802154/atusb.c
+++ b/drivers/net/ieee802154/atusb.c
@@ -183,6 +183,7 @@ static void atusb_work_urbs(struct work_struct *work)
 	} while (!ret);
 
 	usb_anchor_urb(urb, &atusb->idle_urbs);
+	usb_free_urb(urb);
 	dev_warn_ratelimited(&usb_dev->dev,
 			     "atusb_in: can't allocate/submit URB (%d)\n", ret);
 	schedule_delayed_work(&atusb->work,
-- 
2.34.1

Re: [PATCH] ieee802154: atusb: drop the URB reference after a failed resubmit

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH] ieee802154: atusb: drop the URB reference after a failed resubmit

[Krzysztof Kozlowski]

On Wed, 04 Nov 2026 02:14:58 +0000, Wentao Liang wrote:
> usb_get_from_anchor() returns the URB with a reference held by the
> caller.  When atusb_submit_rx_urb() fails, that reference is not consumed,
> and re-anchoring the URB takes an additional one.  The caller's reference
> is then never dropped.
> 
> Release it after putting the URB back on the idle anchor.
> 
> Fixes: 7490b008d123 ("ieee802154: add support for atusb transceiver")
> Cc: stable@vger.kernel.org
> Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  drivers/net/ieee802154/atusb.c | 1 +
>  1 file changed, 1 insertion(+)
> 

Multiple things here:
1. Your team ignored completely previous feedback. Multiple times.

2. You use multiple identities with this email address, thus I doubt that we
   speak with an actual person.

3. Your bot does not even bother running checkpatch, right?

4. Nothing of this was reproduced nor tested.

5. Finally, the same feedback:
You sent multiple independent patches, to multiple independent
subsystems. The amount of these patches clearly suggest this was
AI generated and most likely not tested.

More importantly, you sent all this work without properly organizing
relevant patches into patchsets. This makes reviewing difficult
and might cause multiple reviewers to address the same issue.
Replying to the entire set is impossible and requires handling each
patch independently, instead of applying or discarding the set.
Maintainers also won't see the bigger picture of your work. Quite
worrying.

This is on the verge of hostile patch: bomb us with so many
contributions, we won't be able to handle them in efficient manner,
like responding ONCE to ask you to slow down.  Considering all this
is untested and LLM generated, I have even more doubts whether this
should be considered for review.

Please read kernel documentation BEFORE posting more work. It will
explain you how to identify subsystems, how to organize your work per
subsystem (so a patchset grouping multiple patches with a short cover
letter), how to document the issue discovery method and what was
or was not tested.

Best regards,
Krzysztof

Re: [PATCH] ieee802154: atusb: drop the URB reference after a failed resubmit

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 (2) · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete, and the commit message describes the bug
  wrongly.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but
  atusb_disconnect() (drivers/net/ieee802154/atusb.c) kills and frees…
- [High] This problem wasn't introduced by this patch, but the lifetime
  of the delayed work atusb->work (embedded in hw->priv) is guarded only…

--

ieee802154: atusb: drop the URB reference after a failed resubmit

This patch adds a usb_free_urb() call to atusb_work_urbs(). The call runs
after a failed atusb_submit_rx_urb() has put the URB back on the idle
anchor. Its purpose is to drop the reference that usb_get_from_anchor()
hands to the caller.

> diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
> index 5f7fc4ee7a071..c924f17171f46 100644
> --- a/drivers/net/ieee802154/atusb.c
> +++ b/drivers/net/ieee802154/atusb.c
> @@ -183,6 +183,7 @@ static void atusb_work_urbs(struct work_struct *work)
>  	} while (!ret);
>  
>  	usb_anchor_urb(urb, &atusb->idle_urbs);
> +	usb_free_urb(urb);

[Severity: High]
Doesn't the reference from usb_get_from_anchor() also leak when
atusb_submit_rx_urb() succeeds?

The commit message says:

    When atusb_submit_rx_urb() fails, that reference is not consumed,

On success, atusb_submit_rx_urb() doesn't consume it either.
usb_anchor_urb(urb, &atusb->rx_urbs) takes its own reference, and
usb_hcd_submit_urb() takes another. The loop then fetches the next URB
without dropping the caller's reference:

	do {
		urb = usb_get_from_anchor(&atusb->idle_urbs);
		if (!urb)
			return;
		ret = atusb_submit_rx_urb(atusb, urb);
	} while (!ret);

The refcount through one successful RX cycle:

  on idle_urbs                                   1
  usb_get_from_anchor() (caller owns it)         1
  usb_anchor_urb(urb, &atusb->rx_urbs)           2
  usb_hcd_submit_urb()                           3
  __usb_hcd_giveback_urb()->usb_unanchor_urb()   2
  atusb_in()->usb_anchor_urb(idle_urbs)          3
  __usb_hcd_giveback_urb()->usb_put_urb()        2

So the URB goes back on idle_urbs with 2 references instead of 1, and
each later successful cycle adds one more. atusb_in() reschedules the
work for every received frame, so this is the normal RX path, not an
error path.

On disconnect, atusb_in() does not re-anchor URBs that were killed with
-ENOENT, and atusb_free_urbs() drops only one reference per idle URB.
Wouldn't every URB that was ever submitted successfully then leak on each
disconnect or unbind?

Would it be better to drop the reference after every
atusb_submit_rx_urb() call, success or failure, re-anchoring to idle_urbs
first on failure? carl9170_usb_submit_rx_urb() and btusb's play_deferred()
both do this:

	urb = usb_get_from_anchor(&ar->rx_pool);
	if (urb) {
		usb_anchor_urb(urb, &ar->rx_anch);
		err = usb_submit_urb(urb, gfp);
		if (unlikely(err)) {
			usb_unanchor_urb(urb);
			usb_anchor_urb(urb, &ar->rx_pool);
		} else {
			...
		}
		usb_free_urb(urb);
	}

That would replace the failure-only drop, and the commit message would
need updating to match.

>  	dev_warn_ratelimited(&usb_dev->dev,
>  			     "atusb_in: can't allocate/submit URB (%d)\n", ret);
>  	schedule_delayed_work(&atusb->work,

[Severity: High]
This isn't a bug introduced by this patch, but can atusb_xmit() use
atusb->tx_urb after disconnect has freed it?

atusb_disconnect() frees tx_urb before it unregisters the hw:

drivers/net/ieee802154/atusb.c:atusb_disconnect() {
    ...
	usb_kill_urb(atusb->tx_urb);
	usb_free_urb(atusb->tx_urb);

	ieee802154_unregister_hw(atusb->hw);
    ...
}

The wpan interface stays registered and up until
ieee802154_unregister_hw()->ieee802154_remove_interfaces() runs. A frame
sent in that window goes through:

ieee802154_subif_start_xmit()
  ieee802154_hot_tx()
    ieee802154_tx()
      drv_xmit_async()
        atusb_xmit()

None of these functions checks for teardown. atusb_xmit() then writes
into the freed URB and submits it:

	usb_fill_control_urb(atusb->tx_urb, usb_dev,
			     usb_sndctrlpipe(usb_dev, 0),
			     (unsigned char *)&atusb->tx_dr, skb->data,
			     skb->len, atusb_xmit_complete, NULL);
	ret = usb_submit_urb(atusb->tx_urb, GFP_ATOMIC);

After a sysfs unbind the device is still attached, so the submit can
succeed and the host controller works on freed memory.

Should ieee802154_unregister_hw() be called before the URBs are killed
and freed?

[Severity: High]
This is a pre-existing issue, but can atusb->work still be queued when
ieee802154_free_hw() frees it?

The delayed work is embedded in hw->priv. Its lifetime is guarded only by
the plain atusb->shutdown flag, which is read without a lock, and by one
cancel_delayed_work_sync() early in atusb_disconnect():

	atusb->shutdown = 1;
	cancel_delayed_work_sync(&atusb->work);

	usb_kill_anchored_urbs(&atusb->rx_urbs);

There seem to be three ways for the work to be queued after that cancel,
or freed without ever being cancelled.

First, atusb_in() checks the flag and then acts on it with nothing
holding it stable:

	usb_anchor_urb(urb, &atusb->idle_urbs);
	if (!atusb->shutdown)
		schedule_delayed_work(&atusb->work, 0);

CPU1 (atusb_in)                 CPU2 (atusb_disconnect)
reads shutdown == 0
                                atusb->shutdown = 1;
                                cancel_delayed_work_sync() (nothing queued)
schedule_delayed_work()
                                usb_kill_anchored_urbs(&atusb->rx_urbs)

usb_kill_anchored_urbs() only waits for the completion handler to finish.
It does not cancel the work that the handler queued.

Second, atusb_start() queues the work without checking shutdown:

drivers/net/ieee802154/atusb.c:atusb_start() {
    ...
	schedule_delayed_work(&atusb->work, 0);
    ...
}

The netdev stays registered until ieee802154_unregister_hw(), which runs
after the cancel. Bringing the interface up in that window queues work
that nothing cancels, because atusb_stop() only kills idle_urbs.

Third, the atusb_probe() error path that runs after
ieee802154_register_hw() has succeeded:

	ieee802154_unregister_hw(hw);
fail:
	atusb_free_urbs(atusb);
	usb_kill_urb(atusb->tx_urb);
	usb_free_urb(atusb->tx_urb);
	ieee802154_free_hw(hw);

Once register_hw has succeeded, userspace can bring wpan0 up. That runs
atusb_start() and then the work, which submits RX URBs to rx_urbs. If the
final atusb_get_and_clear_error() then fails, this path never calls
cancel_delayed_work_sync() and never kills rx_urbs. The in-flight RX URBs
hold skbs with SKB_ATUSB(skb) == atusb.

Could the queued work, or those URB completions, then touch the freed
atusb?

Should the work be cancelled again after the last point that can queue
it, and should rx_urbs be killed, on both the disconnect path and the
probe error path?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261104021458.1468481-1-vulab%40iscas.ac.cn