Linux USB
 help / color / mirror / Atom feed
* Re: [PATCH net-next 1/3] net: phy: phy-c45: add OATC10 Sleep/Wakeup support in 10BASE-T1S PHYs
From: Parthiban.Veerasooran @ 2026-04-01  3:12 UTC (permalink / raw)
  To: andrew
  Cc: piergiorgio.beruto, hkallweit1, linux, davem, edumazet, kuba,
	pabeni, steve.glendinning, UNGLinuxDriver, netdev, linux-usb
In-Reply-To: <239a7455-5cb6-4cbb-b9d6-1d48a0cadbc8@lunn.ch>

Hi Andrew,

On 30/03/26 8:16 pm, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
>> +     /* Cache PLCA settings for later use. These values must be restored when
>> +      * the PHY wakes up from the low-power sleep state, as all configured
>> +      * settings are lost.
>> +      */
> 
> Does the standard define that this configuration is lost in low power
> mode? If the standard says this, then fine. But if not, it should be
> the PHY driver which saves its state during suspend, and restores it
> during resume.

Thank you for the review and pointing this out.

You are correct that the Open Alliance TC10 specification does not 
explicitly state that PHY configuration is lost when entering the 
low‑power state. My earlier assumption was based on the LAN8670/1/2 PHY 
implementation, where the 'monitor and react to wake event' block 
remains active while other power supplies are switched off, resulting in 
configuration loss.

However, since this behavior is not mandated by the specification and 
may vary across PHY implementations, it should not be handled within the 
generic PHY framework.

Given this, I agree that state save and restore is PHY‑specific 
behavior. I will move this logic into the lan867x PHY driver in the next 
revision.

Moreover I will revisit the implementation with internal review and then 
post the next version. Thank you for your support.

Best regards,
Parthiban V
> 
>      Andrew


^ permalink raw reply

* Re: [PATCH net-next 3/3] net: usb: smsc95xx: suspend PHY during USB suspend
From: Parthiban.Veerasooran @ 2026-04-01  3:18 UTC (permalink / raw)
  To: oneukum
  Cc: netdev, linux-usb, piergiorgio.beruto, andrew, hkallweit1, linux,
	davem, edumazet, kuba, pabeni, steve.glendinning, UNGLinuxDriver
In-Reply-To: <1c660862-de30-428d-a772-8bee9b990a59@suse.com>

Hi Oliver

Thank you for reviewing the patch.

On 30/03/26 11:09 pm, Oliver Neukum wrote:
>> @@ -1550,6 +1550,12 @@ static int smsc95xx_suspend(struct 
>> usb_interface *intf, pm_message_t message)
>>
>>       pdata->pm_task = current;
>>
>> +     if (pdata->phydev) {
>> +             ret = phy_suspend(pdata->phydev);
>> +             if (ret)
>> +                     return ret;
>> +     }
> 
> At this point you have suspended the phy.
> Hence the device can no longer transmit
> 
>> +
>>       ret = usbnet_suspend(intf, message);
> 
> This wants to
> 
> 1. drain the queue if you do runtime PM
> 2. can return -EBUSY
> 
>>       if (ret < 0) {
>>               netdev_warn(dev->net, "usbnet_suspend error\n");
> 
> And here it will return in the error case. With the phy
> already suspended.

Thank you for pointing it out. I agree with you. I didn’t note it 
earlier since the issue did not occur during my testing. I will move the 
phy_suspend() to the appropriate place.

> 
> And, as a question of principle: Why do you suspend the phy
> in suspend(), but take no action in resume()?

In resume(), I did not call phy_resume() because the resume path already 
invokes phy_init_hw(), which internally calls 
phydev->drv->config_init(). This reinitializes and reconfigures the PHY. 
This is the reason why I didn't call phy_resume(). If there is a 
preference or expectation to use phy_resume() for symmetry or to better 
align with the PHY framework’s suspend/resume semantics, I’m happy to 
update the code accordingly.

Moreover I will revisit the implementation with internal review and then 
post the next version. Thank you for your support.

Best regards,
Parthiban V
> 
>         Regards
>                 Oliver
> 
> 


^ permalink raw reply

* Re: [PATCH] usb: chipidea: udc: reject non-control requests while controller is suspended
From: Andreea.Popescu @ 2026-04-01  6:45 UTC (permalink / raw)
  To: Alan Stern
  Cc: Peter Chen, Greg Kroah-Hartman, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <04f515ee-2213-4489-9734-d871e0d3d682@rowland.harvard.edu>

On Tue, Mar 31, 2026 at 12:21:45PM +0000, Andreea.Popescu@aumovio.com wrote:
>> When Linux runtime PM autosuspends a ChipIdea UDC that is still
>> enumerated by the host, the driver gates the PHY clocks and marks
>> the controller as suspended (ci->in_lpm = 1) but deliberately leaves
>> gadget.speed unchanged so upper-layer gadget drivers do not see a
>> spurious disconnect.
>>
>> The problem is that those same drivers may continue to call
>> usb_ep_queue() during the autosuspend window.  _hardware_enqueue()
>> silently adds the request to the endpoint queue and returns 0, but
>> hw_ep_prime() cannot succeed with gated clocks, so the completion
>> interrupt never fires.  The request — and its backing buffer — is
>> permanently lost.  The caller sees a successful return and never
>> frees the buffer.
>Won't the request complete normally after the gadget is resumed, or
>abnormally after a reset, disconnect, or shutdown?  Either way, it
>wouldn't be lost permanently.
>
>Alan Stern
Thank you very much for the review!
On "complete normally after resume":
This would be true only if the runtime-resume path reprimed the pending endpoints. It does not. ci_controller_resume() clears PORTSC_PHCD and ci->in_lpm, restoring the PHY, but it performs no endpoint repriming. The TD that was enqueued during the suspended window has its DMA node linked in hwep->qh.queue and the QH's td.next is written, but the OP_ENDPTPRIME write inside hw_ep_prime() was a no-op against gated clocks. After resume the controller has no knowledge of that TD — the ENDPTPRIME/ENDPTSTAT bits are clean — so it never processes it. The request is not picked up automatically.
A subsequent request on the same endpoint would be appended to the existing TD chain via the "non-empty queue" branch of _hardware_enqueue(), which does not issue a fresh prime either; it relies on the hardware already being active on that endpoint. Since the first prime was lost, that chain never becomes active.

On "abnormally after reset/disconnect/shutdown":
Correct: _gadget_stop_activity() → _ep_nuke() would drain the queues. However the target deployment is a fixed embedded topology: the USB cable is permanently wired between the device and the host, no physical disconnect occurs. The gadget function carries a continuous data stream (a transport-layer protocol that generates background traffic — keepalives, ACKs — even during otherwise-idle periods). In this environment runtime PM autosuspend fires on the 2-second idle timeout, the host continues to trigger upper-layer submissions, and the connection runs uninterrupted for hours. Without an explicit disconnect or reset there is no natural cleanup path, so the stranded requests accumulate for the lifetime of the session.
The fix returns -ESHUTDOWN immediately, which is the same status _ep_nuke() sets. The caller treats it as a completed-with-error request, frees the buffer, and does not re-queue — matching the behaviour that would occur if the device had actually disconnected.

Concrete example — usb_diag_write() transmit path during autosuspend:
The device exposes a USB diagnostic interface (f_diag). A diagnostic session is running between the device's diagnostic daemon and the host-side tool. The controller autosuspends after 2 seconds of idle activity. Shortly after, the diagnostic core generates a log packet — for example a response to a pending request — and calls usb_diag_write().
The call sequence inside usb_diag_write() in f_diag.c without the patch:
ci_runtime_suspend() has already fired: PORTSC_PHCD set, ci->in_lpm = 1, gadget.speed still USB_SPEED_HIGH.
usb_diag_write() checks !ctxt->configured || !ctxt->in — both are valid since the gadget did not disconnect. Proceeds.
Dequeues a usb_request from ctxt->write_pool, sets req->buf = d_req->buf and calls kref_get(&ctxt->kref).
ctxt->dpkts_tolaptop_pending++ — pending counter incremented.
usb_ep_queue(in, req, GFP_ATOMIC) → ep_queue() → _ep_queue() → _hardware_enqueue(). hw_ep_prime() writes ENDPTPRIME against gated clocks — no-op. Returns 0.
usb_ep_queue returns 0. The error-recovery branch (list_add_tail back to write_pool, dpkts_tolaptop_pending--, kref_put) is not taken. usb_diag_write() returns 0.
The complete callback (diag_write_complete) never fires because the hardware never processed the TD. Consequences:
req is permanently removed from write_pool. After a handful of autosuspend cycles, write_pool is exhausted and every subsequent usb_diag_write() returns -EAGAIN.
kref_get called but kref_put in the complete callback never called — diag_context reference count is unbalanced.
dpkts_tolaptop_pending is permanently incremented — counter drifts.
d_req->buf (the diagnostic buffer allocated by the diag core) is never returned via the completion callback — the diag core's buffer pool drains.
The diagnostic session stalls silently: the host tool stops receiving responses, with no error indication on either side.

With the patch, step 5 returns -ESHUTDOWN instead of 0. The error path in usb_diag_write() executes: req is returned to write_pool, dpkts_tolaptop_pending is decremented, kref_put is called, and -EIO is returned to the diag core. The diag core can retry the write after runtime resume, write_pool remains intact, and the diagnostic session survives the autosuspend cycle.



________________________________________
De la: Alan Stern <stern@rowland.harvard.edu>
Trimis: marți, 31 martie 2026 18:39
Către: Popescu, Andreea
Cc: Peter Chen; Greg Kroah-Hartman; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
Subiect: Re: [PATCH] usb: chipidea: udc: reject non-control requests while controller is suspended

CAUTION: This is an external email. Do not click or open any attachments unless you recognize the sender and know that the content is safe. (http://links.aumovio-internal.com/mailcheck)


On Tue, Mar 31, 2026 at 12:21:45PM +0000, Andreea.Popescu@aumovio.com wrote:
> When Linux runtime PM autosuspends a ChipIdea UDC that is still
> enumerated by the host, the driver gates the PHY clocks and marks
> the controller as suspended (ci->in_lpm = 1) but deliberately leaves
> gadget.speed unchanged so upper-layer gadget drivers do not see a
> spurious disconnect.
>
> The problem is that those same drivers may continue to call
> usb_ep_queue() during the autosuspend window.  _hardware_enqueue()
> silently adds the request to the endpoint queue and returns 0, but
> hw_ep_prime() cannot succeed with gated clocks, so the completion
> interrupt never fires.  The request — and its backing buffer — is
> permanently lost.  The caller sees a successful return and never
> frees the buffer.

Won't the request complete normally after the gadget is resumed, or
abnormally after a reset, disconnect, or shutdown?  Either way, it
wouldn't be lost permanently.

Alan Stern

^ permalink raw reply

* Re: [PATCH net-next v4 1/2] r8152: Add support for 5Gbit Link Speeds and EEE
From: Birger Koblitz @ 2026-04-01  6:55 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: andrew+netdev, davem, edumazet, pabeni, linux-usb, netdev,
	linux-kernel
In-Reply-To: <20260331175649.729e60a7@kernel.org>

On 4/1/26 02:56, Jakub Kicinski wrote:
> On Tue, 31 Mar 2026 17:38:02 +0200 Birger Koblitz wrote:
>>> Are there missing switch cases for NWAY_5000M_FULL in the hardware UPS
>>> flag configurations, such as r8156_ups_flags?
>> This question has now been raised the third time, see here:
>> https://lkml.org/lkml/2026/3/24/1902 and here https://lkml.org/lkml/2026/3/19/1765
>> However, while I share the concern, I cannot provide a better answer than
>> the first time or second time.
> 
> Apologies for the repeats, I tend to send out the full review as
> soon as I spot at least one chunk that looks plausible.
> I should do better but re-checking the AI reviews takes so much
> of my time already.. :(
No problem, it also took me a couple of minutes to understand that it
was in fact the same issue :-)

I appreciate a lot the time and effort spent by reviewers going through
patches, and reviewing AI output probably just adds on top of that. I
see that some of the issues found would have been hard for a human to spot.
On the other hand, some of the issues raised make little sense when giving
them a second look, and would probably not have been brought up by a human
reviewer. Still, they take quit some time to answer properly. One thing I
noticed in particular is that the AI does not seem to take into account
that the code actually works: suspecting broken SRAM-access or wrong
descriptor-size settings would break the driver at such a fundamental level,
it would not be able to work at all.

Birger

^ permalink raw reply

* [syzbot] Monthly usb report (Apr 2026)
From: syzbot @ 2026-04-01  7:42 UTC (permalink / raw)
  To: linux-kernel, linux-usb, syzkaller-bugs

Hello usb maintainers/developers,

This is a 31-day syzbot report for the usb subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/usb

During the period, 4 new issues were detected and 1 were fixed.
In total, 80 issues are still open and 415 have already been fixed.

Some of the still happening issues:

Ref  Crashes Repro Title
<1>  6855    Yes   INFO: task hung in lbs_remove_card
                   https://syzkaller.appspot.com/bug?extid=c99d17aa44dbdba16ad2
<2>  5232    Yes   WARNING in usb_free_urb
                   https://syzkaller.appspot.com/bug?extid=b466336413a1fba398a5
<3>  3941    Yes   KASAN: use-after-free Read in v4l2_fh_open
                   https://syzkaller.appspot.com/bug?extid=b2391895514ed9ef4a8e
<4>  3547    Yes   WARNING in cm109_urb_irq_callback/usb_submit_urb
                   https://syzkaller.appspot.com/bug?extid=2d6d691af5ab4b7e66df
<5>  2716    Yes   possible deadlock in input_inject_event
                   https://syzkaller.appspot.com/bug?extid=79c403850e6816dc39cf
<6>  2701    Yes   INFO: task hung in usbdev_open (2)
                   https://syzkaller.appspot.com/bug?extid=b73659f5bb96fac34820
<7>  2177    Yes   INFO: task hung in jbd2_journal_commit_transaction (5)
                   https://syzkaller.appspot.com/bug?extid=3071bdd0a9953bc0d177
<8>  1441    Yes   WARNING in enable_work
                   https://syzkaller.appspot.com/bug?extid=7053fbd8757fecbbe492
<9>  1436    Yes   KASAN: use-after-free Read in em28xx_init_extension (2)
                   https://syzkaller.appspot.com/bug?extid=99d6c66dbbc484f50e1c
<10> 1400    No    KASAN: vmalloc-out-of-bounds Read in kcov_remote_start
                   https://syzkaller.appspot.com/bug?extid=8a173e13208949931dc7

---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders

To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem

You may send multiple commands in a single email message.

^ permalink raw reply

* Re: [PATCH] usb: chipidea: udc: reject non-control requests while controller is suspended
From: Xu Yang @ 2026-04-01  8:22 UTC (permalink / raw)
  To: Andreea.Popescu@aumovio.com
  Cc: Peter Chen, Greg Kroah-Hartman, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <FRYP281MB261885D9851351180B1EC037EA53A@FRYP281MB2618.DEUP281.PROD.OUTLOOK.COM>

On Tue, Mar 31, 2026 at 12:21:45PM +0000, Andreea.Popescu@aumovio.com wrote:
> When Linux runtime PM autosuspends a ChipIdea UDC that is still
> enumerated by the host, the driver gates the PHY clocks and marks
> the controller as suspended (ci->in_lpm = 1) but deliberately leaves
> gadget.speed unchanged so upper-layer gadget drivers do not see a
> spurious disconnect.

It's strange that chipidea UDC will runtime suspend even it's already
enumerated by the host. AFAIK, the udc driver will call pm_runtime_get_sync()
in ci_hdrc_gadget_connect(is_active = true), so it will be in runtime active
state all the time unless a explicit pm_runtime_put/_autosuspend() is called
in somewhere.

Would you share more details how device controller go to runtime suspended?

Thanks,
Xu Yang

^ permalink raw reply

* Re: [PATCH v2] usb: misc: usbio: Fix URB memory leak on submit failure
From: Hans de Goede @ 2026-04-01  8:40 UTC (permalink / raw)
  To: Felix Gu, Israel Cepeda, Sakari Ailus, Greg Kroah-Hartman
  Cc: linux-usb, linux-kernel
In-Reply-To: <20260331-usbio-v2-1-d8c48dad9463@gmail.com>

Hi,

On 31-Mar-26 14:05, Felix Gu wrote:
> When usb_submit_urb() fails in usbio_probe(), the previously allocated
> URB is never freed, causing a memory leak.
> 
> Fix this by jumping to err_free_urb label to properly release the URB
> on the error path.
> 
> Fixes: 121a0f839dbb ("usb: misc: Add Intel USBIO bridge driver")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---
> Changes in v2:
> - Fix Oliver's comment.
> - Link to v1: https://lore.kernel.org/lkml/20260330-usbio-v1-1-7141b6dc612a@gmail.com

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

Regards,

Hans



> ---
>  drivers/usb/misc/usbio.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/misc/usbio.c b/drivers/usb/misc/usbio.c
> index 2e68d48a2cc0..02d1e0760f0c 100644
> --- a/drivers/usb/misc/usbio.c
> +++ b/drivers/usb/misc/usbio.c
> @@ -614,8 +614,10 @@ static int usbio_probe(struct usb_interface *intf, const struct usb_device_id *i
>  	usb_fill_bulk_urb(usbio->urb, udev, usbio->rx_pipe, usbio->rxbuf,
>  			  usbio->rxbuf_len, usbio_bulk_recv, usbio);
>  	ret = usb_submit_urb(usbio->urb, GFP_KERNEL);
> -	if (ret)
> -		return dev_err_probe(dev, ret, "Submitting usb urb\n");
> +	if (ret) {
> +		dev_err_probe(dev, ret, "Submitting usb urb\n");
> +		goto err_free_urb;
> +	}
>  
>  	mutex_lock(&usbio->ctrl_mutex);
>  
> @@ -663,6 +665,7 @@ static int usbio_probe(struct usb_interface *intf, const struct usb_device_id *i
>  err_unlock:
>  	mutex_unlock(&usbio->ctrl_mutex);
>  	usb_kill_urb(usbio->urb);
> +err_free_urb:
>  	usb_free_urb(usbio->urb);
>  
>  	return ret;
> 
> ---
> base-commit: 3b058d1aeeeff27a7289529c4944291613b364e9
> change-id: 20260330-usbio-d70e15c97a7a
> 
> Best regards,


^ permalink raw reply

* Re: [PATCH 5/9] usb: xhci: move ring initialization
From: Michal Pecio @ 2026-04-01  9:31 UTC (permalink / raw)
  To: Neronin, Niklas; +Cc: mathias.nyman, linux-usb, raoxu
In-Reply-To: <33373f50-7e86-4854-a651-577662d54811@linux.intel.com>

On Mon, 30 Mar 2026 11:53:31 +0300, Neronin, Niklas wrote:
> xhci_clear_command_ring() clears the command ring specifically.
> 
> The idea is to have (naming not set in stone):
> ring_alloc() <- allocate necessary ring memory
> ring_free()  <- free all ring memory
> ring_init()  <- initialize necessary ring memory
> ring_reset() <- resets the ring

Not sure what reset() means, simply clear()? Would it include init()?

What I'm saying is that alloc() and init() almost always need to happen
together and the verbosity of this patch demonstrates widespread demand
for one function which can do both things.

Is this patch even correct? To know, I would need to find all callers
of ring_alloc() and verify that ring_init() has been added. OTOH, a
rename of ring_alloc() to ring_create() wouldn't compile if the patch
frogot to update some callers.

Similarly, separation of setting up links (ring_init) from clearing the
rest adds complexity and bug opportunities. Usually (always?) callers
will need to do both of those things for correct outcome.

I think it would be safer not to have rings in an invalid state, unless
it's some exceptional case like a very short-lived ring allocated for
expansion. And  even then I'm not sure why expansion allocates a new
ring and splices it into the old one, instead of just adding segments.

Regards,
Michal

^ permalink raw reply

* [syzbot] [input?] [usb?] KASAN: slab-use-after-free Read in hiddev_disconnect (4)
From: syzbot @ 2026-04-01  9:33 UTC (permalink / raw)
  To: bentiss, jikos, linux-input, linux-kernel, linux-usb,
	syzkaller-bugs

Hello,

syzbot found the following issue on:

HEAD commit:    46b513250491 Merge tag 'v7.0-rc5-smb3-client-fix' of git:/..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=177c11da580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=5a3e5e8c17cc174e
dashboard link: https://syzkaller.appspot.com/bug?extid=563191a4939ddbfe73d4
compiler:       gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44

Unfortunately, I don't have any reproducer for this issue yet.

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/d900f083ada3/non_bootable_disk-46b51325.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/22fdb6baf246/vmlinux-46b51325.xz
kernel image: https://storage.googleapis.com/syzbot-assets/a3a69d9008ed/bzImage-46b51325.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+563191a4939ddbfe73d4@syzkaller.appspotmail.com

usb 6-1: USB disconnect, device number 25
==================================================================
BUG: KASAN: slab-use-after-free in debug_spin_lock_before kernel/locking/spinlock_debug.c:86 [inline]
BUG: KASAN: slab-use-after-free in do_raw_spin_lock+0x23b/0x260 kernel/locking/spinlock_debug.c:115
Read of size 4 at addr ffff8880289f981c by task kworker/1:3/6000

CPU: 1 UID: 0 PID: 6000 Comm: kworker/1:3 Tainted: G             L      syzkaller #0 PREEMPT(full) 
Tainted: [L]=SOFTLOCKUP
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
Workqueue: usb_hub_wq hub_event
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:94 [inline]
 dump_stack_lvl+0x100/0x190 lib/dump_stack.c:120
 print_address_description mm/kasan/report.c:378 [inline]
 print_report+0x156/0x4c9 mm/kasan/report.c:482
 kasan_report+0xdf/0x1e0 mm/kasan/report.c:595
 debug_spin_lock_before kernel/locking/spinlock_debug.c:86 [inline]
 do_raw_spin_lock+0x23b/0x260 kernel/locking/spinlock_debug.c:115
 __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:133 [inline]
 _raw_spin_lock_irqsave+0x42/0x60 kernel/locking/spinlock.c:162
 __mutex_unlock_slowpath+0x18b/0x790 kernel/locking/mutex.c:963
 hiddev_disconnect+0x15b/0x1d0 drivers/hid/usbhid/hiddev.c:940
 hid_disconnect+0xbe/0x1b0 drivers/hid/hid-core.c:2353
 hid_hw_stop drivers/hid/hid-core.c:2400 [inline]
 hid_device_remove+0x1b2/0x270 drivers/hid/hid-core.c:2834
 device_remove+0xcb/0x180 drivers/base/dd.c:631
 __device_release_driver drivers/base/dd.c:1344 [inline]
 device_release_driver_internal+0x44e/0x620 drivers/base/dd.c:1367
 bus_remove_device+0x2bc/0x560 drivers/base/bus.c:657
 device_del+0x376/0x9b0 drivers/base/core.c:3880
 hid_remove_device drivers/hid/hid-core.c:3009 [inline]
 hid_destroy_device+0x19c/0x240 drivers/hid/hid-core.c:3031
 usbhid_disconnect+0xa0/0xe0 drivers/hid/usbhid/hid-core.c:1477
 usb_unbind_interface+0x1dd/0x9e0 drivers/usb/core/driver.c:458
 device_remove drivers/base/dd.c:633 [inline]
 device_remove+0x12a/0x180 drivers/base/dd.c:625
 __device_release_driver drivers/base/dd.c:1344 [inline]
 device_release_driver_internal+0x44e/0x620 drivers/base/dd.c:1367
 bus_remove_device+0x2bc/0x560 drivers/base/bus.c:657
 device_del+0x376/0x9b0 drivers/base/core.c:3880
 usb_disable_device+0x367/0x810 drivers/usb/core/message.c:1476
 usb_disconnect+0x2e2/0x9a0 drivers/usb/core/hub.c:2345
 hub_port_connect drivers/usb/core/hub.c:5407 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:5707 [inline]
 port_event drivers/usb/core/hub.c:5871 [inline]
 hub_event+0x1d0c/0x4af0 drivers/usb/core/hub.c:5953
 process_one_work+0xa23/0x19a0 kernel/workqueue.c:3276
 process_scheduled_works kernel/workqueue.c:3359 [inline]
 worker_thread+0x5ef/0xe50 kernel/workqueue.c:3440
 kthread+0x370/0x450 kernel/kthread.c:436
 ret_from_fork+0x754/0xd80 arch/x86/kernel/process.c:158
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
 </TASK>

Allocated by task 24:
 kasan_save_stack+0x30/0x50 mm/kasan/common.c:57
 kasan_save_track+0x14/0x30 mm/kasan/common.c:78
 poison_kmalloc_redzone mm/kasan/common.c:398 [inline]
 __kasan_kmalloc+0xaa/0xb0 mm/kasan/common.c:415
 kmalloc_noprof include/linux/slab.h:950 [inline]
 kzalloc_noprof include/linux/slab.h:1188 [inline]
 hiddev_connect+0x259/0x5d0 drivers/hid/usbhid/hiddev.c:893
 hid_connect+0x241/0x1750 drivers/hid/hid-core.c:2267
 hid_hw_start+0xaa/0x140 drivers/hid/hid-core.c:2380
 plantronics_probe+0x2b9/0x3a0 drivers/hid/hid-plantronics.c:209
 __hid_device_probe drivers/hid/hid-core.c:2776 [inline]
 hid_device_probe+0x50e/0x800 drivers/hid/hid-core.c:2813
 call_driver_probe drivers/base/dd.c:643 [inline]
 really_probe+0x241/0xa60 drivers/base/dd.c:721
 __driver_probe_device+0x1de/0x400 drivers/base/dd.c:863
 driver_probe_device+0x4c/0x1b0 drivers/base/dd.c:893
 __device_attach_driver+0x1df/0x340 drivers/base/dd.c:1021
 bus_for_each_drv+0x159/0x1e0 drivers/base/bus.c:500
 __device_attach+0x1e4/0x4d0 drivers/base/dd.c:1093
 device_initial_probe+0xaf/0xd0 drivers/base/dd.c:1148
 bus_probe_device+0x64/0x160 drivers/base/bus.c:613
 device_add+0x11d9/0x1950 drivers/base/core.c:3691
 hid_add_device+0x2bf/0x440 drivers/hid/hid-core.c:2952
 usbhid_probe+0xd57/0x1350 drivers/hid/usbhid/hid-core.c:1450
 usb_probe_interface+0x303/0x8f0 drivers/usb/core/driver.c:396
 call_driver_probe drivers/base/dd.c:643 [inline]
 really_probe+0x241/0xa60 drivers/base/dd.c:721
 __driver_probe_device+0x1de/0x400 drivers/base/dd.c:863
 driver_probe_device+0x4c/0x1b0 drivers/base/dd.c:893
 __device_attach_driver+0x1df/0x340 drivers/base/dd.c:1021
 bus_for_each_drv+0x159/0x1e0 drivers/base/bus.c:500
 __device_attach+0x1e4/0x4d0 drivers/base/dd.c:1093
 device_initial_probe+0xaf/0xd0 drivers/base/dd.c:1148
 bus_probe_device+0x64/0x160 drivers/base/bus.c:613
 device_add+0x11d9/0x1950 drivers/base/core.c:3691
 usb_set_configuration+0xd97/0x1c60 drivers/usb/core/message.c:2266
 usb_generic_driver_probe+0xa1/0xe0 drivers/usb/core/generic.c:250
 usb_probe_device+0xef/0x400 drivers/usb/core/driver.c:291
 call_driver_probe drivers/base/dd.c:643 [inline]
 really_probe+0x241/0xa60 drivers/base/dd.c:721
 __driver_probe_device+0x1de/0x400 drivers/base/dd.c:863
 driver_probe_device+0x4c/0x1b0 drivers/base/dd.c:893
 __device_attach_driver+0x1df/0x340 drivers/base/dd.c:1021
 bus_for_each_drv+0x159/0x1e0 drivers/base/bus.c:500
 __device_attach+0x1e4/0x4d0 drivers/base/dd.c:1093
 device_initial_probe+0xaf/0xd0 drivers/base/dd.c:1148
 bus_probe_device+0x64/0x160 drivers/base/bus.c:613
 device_add+0x11d9/0x1950 drivers/base/core.c:3691
 usb_new_device.cold+0x685/0x115c drivers/usb/core/hub.c:2695
 hub_port_connect drivers/usb/core/hub.c:5567 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:5707 [inline]
 port_event drivers/usb/core/hub.c:5871 [inline]
 hub_event+0x314d/0x4af0 drivers/usb/core/hub.c:5953
 process_one_work+0xa23/0x19a0 kernel/workqueue.c:3276
 process_scheduled_works kernel/workqueue.c:3359 [inline]
 worker_thread+0x5ef/0xe50 kernel/workqueue.c:3440
 kthread+0x370/0x450 kernel/kthread.c:436
 ret_from_fork+0x754/0xd80 arch/x86/kernel/process.c:158
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245

Freed by task 11803:
 kasan_save_stack+0x30/0x50 mm/kasan/common.c:57
 kasan_save_track+0x14/0x30 mm/kasan/common.c:78
 kasan_save_free_info+0x3b/0x70 mm/kasan/generic.c:584
 poison_slab_object mm/kasan/common.c:253 [inline]
 __kasan_slab_free+0x5f/0x80 mm/kasan/common.c:285
 kasan_slab_free include/linux/kasan.h:235 [inline]
 slab_free_hook mm/slub.c:2685 [inline]
 slab_free mm/slub.c:6165 [inline]
 kfree+0x1f6/0x6b0 mm/slub.c:6483
 hiddev_release+0x40d/0x520 drivers/hid/usbhid/hiddev.c:232
 __fput+0x3ff/0xb40 fs/file_table.c:469
 task_work_run+0x150/0x240 kernel/task_work.c:233
 resume_user_mode_work include/linux/resume_user_mode.h:50 [inline]
 __exit_to_user_mode_loop kernel/entry/common.c:67 [inline]
 exit_to_user_mode_loop+0x100/0x4a0 kernel/entry/common.c:98
 __exit_to_user_mode_prepare include/linux/irq-entry-common.h:226 [inline]
 syscall_exit_to_user_mode_prepare include/linux/irq-entry-common.h:256 [inline]
 syscall_exit_to_user_mode include/linux/entry-common.h:325 [inline]
 do_syscall_64+0x67c/0xf80 arch/x86/entry/syscall_64.c:100
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

The buggy address belongs to the object at ffff8880289f9800
 which belongs to the cache kmalloc-512 of size 512
The buggy address is located 28 bytes inside of
 freed 512-byte region [ffff8880289f9800, ffff8880289f9a00)

The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0xffff8880289f8c00 pfn:0x289f8
head: order:2 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
flags: 0xfff00000000240(workingset|head|node=0|zone=1|lastcpupid=0x7ff)
page_type: f5(slab)
raw: 00fff00000000240 ffff88801b842c80 ffffea0000b3f710 ffffea00015f9410
raw: ffff8880289f8c00 0000000800100008 00000000f5000000 0000000000000000
head: 00fff00000000240 ffff88801b842c80 ffffea0000b3f710 ffffea00015f9410
head: ffff8880289f8c00 0000000800100008 00000000f5000000 0000000000000000
head: 00fff00000000002 ffffea0000a27e01 00000000ffffffff 00000000ffffffff
head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000004
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 2, migratetype Unmovable, gfp_mask 0x1d20c0(__GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC|__GFP_HARDWALL), pid 10245, tgid 10245 (udevd), ts 189371739713, free_ts 189294134261
 set_page_owner include/linux/page_owner.h:32 [inline]
 post_alloc_hook+0x153/0x170 mm/page_alloc.c:1889
 prep_new_page mm/page_alloc.c:1897 [inline]
 get_page_from_freelist+0x111d/0x3140 mm/page_alloc.c:3962
 __alloc_frozen_pages_noprof+0x27c/0x2ba0 mm/page_alloc.c:5250
 alloc_slab_page mm/slub.c:3292 [inline]
 allocate_slab mm/slub.c:3481 [inline]
 new_slab+0xa6/0x6b0 mm/slub.c:3539
 refill_objects+0x26b/0x400 mm/slub.c:7175
 refill_sheaf mm/slub.c:2812 [inline]
 __pcs_replace_empty_main+0x1ab/0x660 mm/slub.c:4615
 alloc_from_pcs mm/slub.c:4717 [inline]
 slab_alloc_node mm/slub.c:4851 [inline]
 __kmalloc_cache_noprof+0x493/0x6f0 mm/slub.c:5375
 kmalloc_noprof include/linux/slab.h:950 [inline]
 kzalloc_noprof include/linux/slab.h:1188 [inline]
 kernfs_fop_open+0x23d/0xd50 fs/kernfs/file.c:641
 do_dentry_open+0x6d8/0x1660 fs/open.c:949
 vfs_open+0x82/0x3f0 fs/open.c:1081
 do_open fs/namei.c:4671 [inline]
 path_openat+0x208c/0x31a0 fs/namei.c:4830
 do_file_open+0x20e/0x430 fs/namei.c:4859
 do_sys_openat2+0x10d/0x1e0 fs/open.c:1366
 do_sys_open fs/open.c:1372 [inline]
 __do_sys_openat fs/open.c:1388 [inline]
 __se_sys_openat fs/open.c:1383 [inline]
 __x64_sys_openat+0x12d/0x210 fs/open.c:1383
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0x106/0xf80 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
page last free pid 11125 tgid 11125 stack trace:
 reset_page_owner include/linux/page_owner.h:25 [inline]
 __free_pages_prepare mm/page_alloc.c:1433 [inline]
 __free_frozen_pages+0x7e1/0x10d0 mm/page_alloc.c:2978
 stack_depot_save_flags+0x435/0x9d0 lib/stackdepot.c:735
 kasan_save_stack+0x3f/0x50 mm/kasan/common.c:58
 kasan_save_track+0x14/0x30 mm/kasan/common.c:78
 poison_kmalloc_redzone mm/kasan/common.c:398 [inline]
 __kasan_kmalloc+0xaa/0xb0 mm/kasan/common.c:415
 kmalloc_noprof include/linux/slab.h:950 [inline]
 kzalloc_noprof include/linux/slab.h:1188 [inline]
 ida_alloc_range+0x464/0x830 lib/idr.c:420
 ida_alloc include/linux/idr.h:293 [inline]
 create_worker+0x99/0x750 kernel/workqueue.c:2796
 maybe_create_worker kernel/workqueue.c:3075 [inline]
 manage_workers kernel/workqueue.c:3152 [inline]
 worker_thread+0x8e2/0xe50 kernel/workqueue.c:3415
 kthread+0x370/0x450 kernel/kthread.c:436
 ret_from_fork+0x754/0xd80 arch/x86/kernel/process.c:158
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245

Memory state around the buggy address:
 ffff8880289f9700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
 ffff8880289f9780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>ffff8880289f9800: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                            ^
 ffff8880289f9880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff8880289f9900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

^ permalink raw reply

* Re: [PATCH v2] USB: serial: option: Add MeiG Smart SRM825WN
From: Johan Hovold @ 2026-04-01  9:37 UTC (permalink / raw)
  To: Ernestas Kulik; +Cc: linux-usb, linux-kernel
In-Reply-To: <20260324110716.970023-1-ernestas.k@iconn-networks.com>

On Tue, Mar 24, 2026 at 01:07:16PM +0200, Ernestas Kulik wrote:
> This commit adds support for the SDX62-based MeiG Smart SRM825WN module.

> v2: Use SRM825WN instead of SRT853, which is based on it.

Thanks for the update. I removed the changelog when applying (it should
generally go below the --- line so that it does not make it into the
commit message).

I also reworded the first sentence to use imperative mood (see
Documentation/process/submitting-patches.rst).

> Signed-off-by: Ernestas Kulik <ernestas.k@iconn-networks.com>
> ---

Johan

^ permalink raw reply

* Re: [PATCH 3/9] usb: xhci: factor out roothub bandwidth cleanup
From: Michal Pecio @ 2026-04-01  9:58 UTC (permalink / raw)
  To: Neronin, Niklas; +Cc: mathias.nyman, linux-usb, raoxu
In-Reply-To: <108b65c0-349b-4854-b703-f6951b53bc33@linux.intel.com>

On Tue, 31 Mar 2026 12:34:36 +0300, Neronin, Niklas wrote:
> > This loop used to run before xhci_free_virt_devices_depth_first(),
> > but now it will run after. It seems that the endpoints here are
> > virt_ep which also should be gone already, so this loop likely does
> > nothing (empty list) or writes to virtual devices after free
> > (somebody forgot to unlink some endpoints from the list).  
> 
> In my testing, when xhci_rh_bw_cleanup() is called after
> xhci_free_virt_devices_depth_first() in xhci_resume(), all related
> resources have already been freed.
> That said, I have chosen to keep the existing freeing in this patch
> set. Removing it would introduce an additional behavioral change and a
> potential regression point, which I prefer to avoid at this stage.

Well, reordering these loops is also a potential behavior change.

As vdevs and tt_infos are closely tied together, I think it would make
sense for one function to free all of that stuff.

A non-behavior-changing way of doing it would be to extract the three
existing loops to such a function, in the exact order they run today.

> Do we trust xhci_free_virt_devices_depth_first() to work correctly?
> If yes then it seems this whole function is unnecessary.  

I think it's correct now, in the sense that all vdevs are removed and
there is no UAF along the way. Although a few months ago an unrelated
patch did break it unexpectedly, including UAF in some edge cases.

In principle it should be possible to drop separate tt_info cleanup,
because removing vdevs achieves the same. And if it doesn't then things
would also be broken under normal operation, not only suspend, as the
same xhci_free_virt_device() is used in both situations.

Regards,
Michal

^ permalink raw reply

* Re: [PATCH 9/9] usb: xhci: optimize resuming from S4 (suspend-to-disk)
From: Michal Pecio @ 2026-04-01 10:38 UTC (permalink / raw)
  To: Neronin, Niklas; +Cc: mathias.nyman, linux-usb, raoxu
In-Reply-To: <b3a945bf-65e9-4a8e-9a88-341bc59f6c8d@linux.intel.com>

On Tue, 31 Mar 2026 12:59:50 +0300, Neronin, Niklas wrote:
> On 30/03/2026 12.45, Michal Pecio wrote:
> >   
> >>  
> >> -		xhci_dbg(xhci, "// Disabling event ring interrupts\n");
> >> -		temp = readl(&xhci->op_regs->status);
> >> -		writel((temp & ~0x1fff) | STS_EINT, &xhci->op_regs->status);
> >> -		xhci_disable_interrupter(xhci, xhci->interrupters[0]);
> >> +		cancel_delayed_work_sync(&xhci->cmd_timer);
> >> +
> >> +		/* Delete all remaining commands */
> >> +		xhci_cleanup_command_queue(xhci);  
> > 
> > Considering that xhci_suspend() clears the command ring anyway, it
> > could probably do this too so we don't need to.   
> 
> It makes more sense to have all clearing in one place, instead of
> spread out over suspend and resume. This will be addressed in the
> next patch set, i.e. remove clearing from suspend (if possible).

And this patch leaves command cleanup spread across suspend and resume.

If a future patch moves clearing the ring from suspend to resume then
it can also move completing and freeing stale commands with it.

> >> +		xhci_for_each_ring_seg(xhci->cmd_ring->first_seg, seg)
> >> +			memset(seg->trbs, 0, sizeof(union xhci_trb) * TRBS_PER_SEGMENT);  
> > 
> > This looks like a bug because it nukes link TRBs. I know that
> > xhci_init() will fix this up (unless somebody changes that without
> > updating here), but it looks confusing.  

And it's unnecessary because xhci_suspend() wiped the ring already.
Which means that xhci_init() calling ring_init() is unnecessary too.

> I would like to remove xhci_clear_command_ring() eventually,
> instead have:
> 
>    xhci_ring_reset()
>    ...
>    xhci_set_cmd_ring_deq()

Fair enough, in some cases one may be useful without the other.

> or
> 
>    xhci_ring_clear()
>    ...
>    xhci_ring_init()
>    xhci_set_cmd_ring_deq()

Not sure what ... would be, however?

If clear() doesn't include init(), is there ever a good reason to
maintain the ring in an invalid state? A state where forgetting
about ring_init() would cause the HC to escape the ring and IOMMU
fault or interpret random memory as commands?

Regards,
Michal

^ permalink raw reply

* [PATCH] usb: gadget: f_uac1_legacy: validate control request size
From: Taegu Ha @ 2026-04-01 10:46 UTC (permalink / raw)
  To: linux-usb; +Cc: gregkh, kees, linux-kernel, Taegu Ha

f_audio_complete() copies req->length bytes into a 4-byte stack
variable:

  u32 data = 0;
  memcpy(&data, req->buf, req->length);

req->length is derived from the host-controlled USB request path,
which can lead to a stack out-of-bounds write.

Validate req->actual against the expected payload size for the
supported control selectors and decode only the expected amount
of data.

This avoids copying a host-influenced length into a fixed-size
stack object.

Signed-off-by: Taegu Ha <hataegu0826@gmail.com>
---
 drivers/usb/gadget/function/f_uac1_legacy.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uac1_legacy.c b/drivers/usb/gadget/function/f_uac1_legacy.c
index a0c953a99727..6d6fe5db99f5 100644
--- a/drivers/usb/gadget/function/f_uac1_legacy.c
+++ b/drivers/usb/gadget/function/f_uac1_legacy.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/atomic.h>
+#include <asm/unaligned.h>
 
 #include "u_uac1_legacy.h"
 
@@ -370,9 +371,21 @@ static void f_audio_complete(struct usb_ep *ep, struct usb_request *req)
 		if (ep == out_ep)
 			f_audio_out_ep_complete(ep, req);
 		else if (audio->set_con) {
-			memcpy(&data, req->buf, req->length);
-			audio->set_con->set(audio->set_con, audio->set_cmd,
-					le16_to_cpu(data));
+			struct usb_audio_control *con = audio->set_con;
+
+			if ((con->type == UAC_FU_MUTE && req->actual != sizeof(u8)) ||
+				(con->type == UAC_FU_VOLUME && req->actual != sizeof(__le16)) ||
+				(con->type != UAC_FU_MUTE && con->type != UAC_FU_VOLUME)) {
+				usb_ep_set_halt(ep);
+				audio->set_con = NULL;
+				break;
+			}
+
+			if (con->type == UAC_FU_MUTE)
+				data = *(u8 *)req->buf;
+			else
+				data = get_unaligned_le16(req->buf);
+			con->set(con, audio->set_cmd, data);
 			audio->set_con = NULL;
 		}
 		break;
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH] usb: chipidea: udc: reject non-control requests while controller is suspended
From: Andreea.Popescu @ 2026-04-01 10:47 UTC (permalink / raw)
  To: Xu Yang
  Cc: Peter Chen, Greg Kroah-Hartman, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <yel7zijkcua3cdn646fadp2pebatkbt4vracrpsbcdtmpahzxn@dhiapuctxbfe>

On Tue, Mar 31, 2026 at 12:21:45PM +0000, Andreea.Popescu@aumovio.com wrote:
>> When Linux runtime PM autosuspends a ChipIdea UDC that is still
>> enumerated by the host, the driver gates the PHY clocks and marks
>> the controller as suspended (ci->in_lpm = 1) but deliberately leaves
>> gadget.speed unchanged so upper-layer gadget drivers do not see a
>> spurious disconnect.
>
>It's strange that chipidea UDC will runtime suspend even it's already
>enumerated by the host. AFAIK, the udc driver will call pm_runtime_get_sync()
>in ci_hdrc_gadget_connect(is_active = true), so it will be in runtime active
>state all the time unless a explicit pm_runtime_put/_autosuspend() is called
>in somewhere.
>
>Would you share more details how device controller go to runtime suspended?
>Thanks,
>Xu Yang
Thank you very much for taking the time and pointing this. It made me realize a very important distinction. I am using an I.MX board, due to this I will split my answer, so you can decide if it's still worth what I am proposing or you can just reject it. Either way, I am most grateful.
Still applicable to 6.19 mainline:
ep_queue returning 0 for USB_SPEED_UNKNOWN: I believe there might be the following window: _gadget_stop_activity() sets gadget.speed = USB_SPEED_UNKNOWN, but ep_queue is called before that completes from a concurrent context. The return 0 is misleading and should be -ESHUTDOWN.
I.MX specific: On i.MX SoCs the chipidea controller sits inside a power domain managed by imx-blk-ctrl or the GPC. When that parent domain is shut down by the platform PM framework, pm_runtime_force_suspend() is called on the chipidea device, bypassing usage_count entirely and invoking ci_runtime_suspend → ci_controller_suspend → ci->in_lpm = true. This happens while VBUS is still present and the gadget is enumerated. This is the actual path I observed and it is platform-specific, not a general chipidea mainline issue. Due to this, please disregard the proposed change with _ep_queue guard on ci->in_lpm

________________________________________
De la: Xu Yang <xu.yang_2@nxp.com>
Trimis: miercuri, 1 aprilie 2026 11:22
Către: Popescu, Andreea
Cc: Peter Chen; Greg Kroah-Hartman; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
Subiect: Re: [PATCH] usb: chipidea: udc: reject non-control requests while controller is suspended

On Tue, Mar 31, 2026 at 12:21:45PM +0000, Andreea.Popescu@aumovio.com wrote:
> When Linux runtime PM autosuspends a ChipIdea UDC that is still
> enumerated by the host, the driver gates the PHY clocks and marks
> the controller as suspended (ci->in_lpm = 1) but deliberately leaves
> gadget.speed unchanged so upper-layer gadget drivers do not see a
> spurious disconnect.

It's strange that chipidea UDC will runtime suspend even it's already
enumerated by the host. AFAIK, the udc driver will call pm_runtime_get_sync()
in ci_hdrc_gadget_connect(is_active = true), so it will be in runtime active
state all the time unless a explicit pm_runtime_put/_autosuspend() is called
in somewhere.

Would you share more details how device controller go to runtime suspended?

Thanks,
Xu Yang

^ permalink raw reply

* Re: [PATCH] thunderbolt: tunnel: simplify allocation
From: Mika Westerberg @ 2026-04-01 11:08 UTC (permalink / raw)
  To: Rosen Penev
  Cc: linux-usb, Andreas Noever, Mika Westerberg, Yehezkel Bernat,
	Kees Cook, Gustavo A. R. Silva, open list,
	open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b
In-Reply-To: <20260330211014.12121-1-rosenp@gmail.com>

On Mon, Mar 30, 2026 at 02:10:14PM -0700, Rosen Penev wrote:
> +++ b/drivers/thunderbolt/tunnel.h
> @@ -75,7 +75,6 @@ struct tb_tunnel {
>  	struct tb *tb;
>  	struct tb_port *src_port;
>  	struct tb_port *dst_port;
> -	struct tb_path **paths;
>  	size_t npaths;
>  	int (*pre_activate)(struct tb_tunnel *tunnel);
>  	int (*activate)(struct tb_tunnel *tunnel, bool activate);
> @@ -107,6 +106,8 @@ struct tb_tunnel {
>  	struct delayed_work dprx_work;
>  	void (*callback)(struct tb_tunnel *tunnel, void *data);
>  	void *callback_data;
> +
> +	struct tb_path *paths[] __counted_by(npaths);

Please move the kernel-doc to the right place as well.

^ permalink raw reply

* Re: [PATCH] usb: gadget: f_uac1_legacy: validate control request size
From: Greg KH @ 2026-04-01 11:30 UTC (permalink / raw)
  To: Taegu Ha; +Cc: linux-usb, kees, linux-kernel
In-Reply-To: <20260401104611.3375330-1-hataegu0826@gmail.com>

On Wed, Apr 01, 2026 at 07:46:11PM +0900, Taegu Ha wrote:
> f_audio_complete() copies req->length bytes into a 4-byte stack
> variable:
> 
>   u32 data = 0;
>   memcpy(&data, req->buf, req->length);
> 
> req->length is derived from the host-controlled USB request path,
> which can lead to a stack out-of-bounds write.
> 
> Validate req->actual against the expected payload size for the
> supported control selectors and decode only the expected amount
> of data.
> 
> This avoids copying a host-influenced length into a fixed-size
> stack object.
> 
> Signed-off-by: Taegu Ha <hataegu0826@gmail.com>
> ---
>  drivers/usb/gadget/function/f_uac1_legacy.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_uac1_legacy.c b/drivers/usb/gadget/function/f_uac1_legacy.c
> index a0c953a99727..6d6fe5db99f5 100644
> --- a/drivers/usb/gadget/function/f_uac1_legacy.c
> +++ b/drivers/usb/gadget/function/f_uac1_legacy.c
> @@ -11,6 +11,7 @@
>  #include <linux/module.h>
>  #include <linux/device.h>
>  #include <linux/atomic.h>
> +#include <asm/unaligned.h>
>  
>  #include "u_uac1_legacy.h"
>  
> @@ -370,9 +371,21 @@ static void f_audio_complete(struct usb_ep *ep, struct usb_request *req)
>  		if (ep == out_ep)
>  			f_audio_out_ep_complete(ep, req);
>  		else if (audio->set_con) {
> -			memcpy(&data, req->buf, req->length);
> -			audio->set_con->set(audio->set_con, audio->set_cmd,
> -					le16_to_cpu(data));
> +			struct usb_audio_control *con = audio->set_con;
> +
> +			if ((con->type == UAC_FU_MUTE && req->actual != sizeof(u8)) ||
> +				(con->type == UAC_FU_VOLUME && req->actual != sizeof(__le16)) ||
> +				(con->type != UAC_FU_MUTE && con->type != UAC_FU_VOLUME)) {
> +				usb_ep_set_halt(ep);
> +				audio->set_con = NULL;
> +				break;
> +			}
> +
> +			if (con->type == UAC_FU_MUTE)
> +				data = *(u8 *)req->buf;
> +			else
> +				data = get_unaligned_le16(req->buf);

Very cool, thanks for finding and fixing this.  But there is a bit of a
coding style issue in that big if check, AND it's hard to follow, how
about doing something like this instead (totally untested and not even
compiled:
			struct usb_audio_control *con = audio->set_con;
			u8 type = con->type;
			bool valid_request = false;

			switch (type) {
			case: UAC_FU_MUTE:
				if (req->actual == sizeof(__u8)) {
					memcpy(&data, req->buf, sizeof(__u8));
					valid_request = true;
				}
				break;
			case UAC_FU_VOLUME:
				if (req->actual == sizeof(__le16) {
					memcpy(&data, req->buf, sizeof(__le16));
					valid_request = true;
				}
				break;
			}
			if (valid_request)
				con->set(con, audio->set_cmd, data);
			else
				usb_ep_set_halt(ep);

  			audio->set_con = NULL;
		}

Does that make it easier to read?  I don't know, maybe not, your call,
but this way the memcpy of a constant size should be equalivant to your
direct cast if the compiler is sane, right?

And as this is a __le16 value, should we be doing some endian conversion
somewhere?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v2] usb: gadget: rndis: validate query and set message buffers
From: Pengpeng Hou @ 2026-04-01 11:34 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, linux-kernel, kees, pengpeng
In-Reply-To: <20260323080845.40045-1-pengpeng@iscas.ac.cn>

Hi Greg,

I have not tested this against an actual RNDIS host/device setup yet.

For clarity, v2 did not change the code from v1; it only expanded the
commit message.

What I was trying to fix here is limited to two current-tree checks that
are missing today:

1. rndis_msg_parser() reads MsgLength from the request body but does not
   verify that it fits within the actual EP0 request buffer length.

2. rndis_set_response() validates the host-controlled
   InformationBufferOffset/InformationBufferLength pair before using it,
   but rndis_query_response() still passes the same fields directly into
   gen_ndis_query_resp() without corresponding bounds validation.

I do not mean this patch to claim that these are the only issues in the
RNDIS parser.

If you want runtime testing before considering this further, I can stop
here until I can test it properly.

Thanks,
Pengpeng


^ permalink raw reply

* Re: [PATCH v2] usb: gadget: rndis: validate query and set message buffers
From: Greg KH @ 2026-04-01 11:40 UTC (permalink / raw)
  To: Pengpeng Hou; +Cc: linux-usb, linux-kernel, kees
In-Reply-To: <20260401113410.66427-1-pengpeng@iscas.ac.cn>

On Wed, Apr 01, 2026 at 07:34:10PM +0800, Pengpeng Hou wrote:
> Hi Greg,
> 
> I have not tested this against an actual RNDIS host/device setup yet.
> 
> For clarity, v2 did not change the code from v1; it only expanded the
> commit message.
> 
> What I was trying to fix here is limited to two current-tree checks that
> are missing today:
> 
> 1. rndis_msg_parser() reads MsgLength from the request body but does not
>    verify that it fits within the actual EP0 request buffer length.

Yes, that's a good thing to check :)

> 2. rndis_set_response() validates the host-controlled
>    InformationBufferOffset/InformationBufferLength pair before using it,
>    but rndis_query_response() still passes the same fields directly into
>    gen_ndis_query_resp() without corresponding bounds validation.

Ah, so that should be fixed up.

> I do not mean this patch to claim that these are the only issues in the
> RNDIS parser.

Oh, I did not think that, there are loads of issues with RNDIS that are
not covered by this patch :)

> If you want runtime testing before considering this further, I can stop
> here until I can test it properly.

Yes, please test this so that we know it doesn't break existing systems.

thanks,

greg k-h

^ permalink raw reply

* [PATCH net-next] r8152: Add helper functions for SRAM2
From: Chih Kai Hsu @ 2026-04-01 11:55 UTC (permalink / raw)
  To: kuba, davem
  Cc: netdev, nic_swsd, linux-kernel, linux-usb, edumazet, bjorn,
	pabeni, Chih Kai Hsu

Add the following helper functions for SRAM2 access to simplify the code
and improve readability:

- sram2_write() - write data to SRAM2 address
- sram2_read() - read data from SRAM2 address
- sram2_write_w0w1() - read-modify-write operation

Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>
---
 drivers/net/usb/r8152.c | 178 +++++++++++++++++-----------------------
 1 file changed, 75 insertions(+), 103 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 8747c55e0a48..1765da5bd6cf 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -213,6 +213,8 @@
 #define OCP_PHY_PATCH_STAT	0xb800
 #define OCP_PHY_PATCH_CMD	0xb820
 #define OCP_PHY_LOCK		0xb82e
+#define OCP_SRAM2_ADDR		0xb87c
+#define OCP_SRAM2_DATA		0xb87e
 #define OCP_ADC_IOFFSET		0xbcfc
 #define OCP_ADC_CFG		0xbc06
 #define OCP_SYSCLK_CFG		0xc416
@@ -1764,6 +1766,27 @@ static void sram_set_bits(struct r8152 *tp, u16 addr, u16 set)
 	sram_write_w0w1(tp, addr, 0, set);
 }
 
+static void sram2_write(struct r8152 *tp, u16 addr, u16 data)
+{
+	ocp_reg_write(tp, OCP_SRAM2_ADDR, addr);
+	ocp_reg_write(tp, OCP_SRAM2_DATA, data);
+}
+
+static u16 sram2_read(struct r8152 *tp, u16 addr)
+{
+	ocp_reg_write(tp, OCP_SRAM2_ADDR, addr);
+	return ocp_reg_read(tp, OCP_SRAM2_DATA);
+}
+
+static void sram2_write_w0w1(struct r8152 *tp, u16 addr, u16 clear, u16 set)
+{
+	u16 data;
+
+	data = sram2_read(tp, addr);
+	data = (data & ~clear) | set;
+	ocp_reg_write(tp, OCP_SRAM2_DATA, data);
+}
+
 static void r8152_mdio_clr_bit(struct r8152 *tp, u16 addr, u16 clear)
 {
 	int data;
@@ -7195,16 +7218,12 @@ static void r8156_hw_phy_cfg(struct r8152 *tp)
 		ocp_reg_write(tp, 0xad4c, 0x00a8);
 		ocp_reg_write(tp, 0xac5c, 0x01ff);
 		ocp_reg_w0w1(tp, 0xac8a, 0xf0, BIT(4) | BIT(5));
-		ocp_reg_write(tp, 0xb87c, 0x8157);
-		ocp_reg_w0w1(tp, 0xb87e, 0xff00, 0x0500);
-		ocp_reg_write(tp, 0xb87c, 0x8159);
-		ocp_reg_w0w1(tp, 0xb87e, 0xff00, 0x0700);
+		sram2_write_w0w1(tp, 0x8157, 0xff00, 0x0500);
+		sram2_write_w0w1(tp, 0x8159, 0xff00, 0x0700);
 
 		/* AAGC */
-		ocp_reg_write(tp, 0xb87c, 0x80a2);
-		ocp_reg_write(tp, 0xb87e, 0x0153);
-		ocp_reg_write(tp, 0xb87c, 0x809c);
-		ocp_reg_write(tp, 0xb87e, 0x0153);
+		sram2_write(tp, 0x80a2, 0x0153);
+		sram2_write(tp, 0x809c, 0x0153);
 
 		/* EEE parameter */
 		ocp_write_word(tp, MCU_TYPE_PLA, PLA_EEE_TXTWSYS_2P5G, 0x0056);
@@ -7402,82 +7421,48 @@ static void r8156b_hw_phy_cfg(struct r8152 *tp)
 		ocp_reg_write(tp, 0xacc8, 0xa0d3);
 		ocp_reg_write(tp, 0xad08, 0x0007);
 
-		ocp_reg_write(tp, 0xb87c, 0x8560);
-		ocp_reg_write(tp, 0xb87e, 0x19cc);
-		ocp_reg_write(tp, 0xb87c, 0x8562);
-		ocp_reg_write(tp, 0xb87e, 0x19cc);
-		ocp_reg_write(tp, 0xb87c, 0x8564);
-		ocp_reg_write(tp, 0xb87e, 0x19cc);
-		ocp_reg_write(tp, 0xb87c, 0x8566);
-		ocp_reg_write(tp, 0xb87e, 0x147d);
-		ocp_reg_write(tp, 0xb87c, 0x8568);
-		ocp_reg_write(tp, 0xb87e, 0x147d);
-		ocp_reg_write(tp, 0xb87c, 0x856a);
-		ocp_reg_write(tp, 0xb87e, 0x147d);
-		ocp_reg_write(tp, 0xb87c, 0x8ffe);
-		ocp_reg_write(tp, 0xb87e, 0x0907);
-		ocp_reg_write(tp, 0xb87c, 0x80d6);
-		ocp_reg_write(tp, 0xb87e, 0x2801);
-		ocp_reg_write(tp, 0xb87c, 0x80f2);
-		ocp_reg_write(tp, 0xb87e, 0x2801);
-		ocp_reg_write(tp, 0xb87c, 0x80f4);
-		ocp_reg_write(tp, 0xb87e, 0x6077);
+		sram2_write(tp, 0x8560, 0x19cc);
+		sram2_write(tp, 0x8562, 0x19cc);
+		sram2_write(tp, 0x8564, 0x19cc);
+		sram2_write(tp, 0x8566, 0x147d);
+		sram2_write(tp, 0x8568, 0x147d);
+		sram2_write(tp, 0x856a, 0x147d);
+		sram2_write(tp, 0x8ffe, 0x0907);
+		sram2_write(tp, 0x80d6, 0x2801);
+		sram2_write(tp, 0x80f2, 0x2801);
+		sram2_write(tp, 0x80f4, 0x6077);
 		ocp_reg_write(tp, 0xb506, 0x01e7);
 
-		ocp_reg_write(tp, 0xb87c, 0x8013);
-		ocp_reg_write(tp, 0xb87e, 0x0700);
-		ocp_reg_write(tp, 0xb87c, 0x8fb9);
-		ocp_reg_write(tp, 0xb87e, 0x2801);
-		ocp_reg_write(tp, 0xb87c, 0x8fba);
-		ocp_reg_write(tp, 0xb87e, 0x0100);
-		ocp_reg_write(tp, 0xb87c, 0x8fbc);
-		ocp_reg_write(tp, 0xb87e, 0x1900);
-		ocp_reg_write(tp, 0xb87c, 0x8fbe);
-		ocp_reg_write(tp, 0xb87e, 0xe100);
-		ocp_reg_write(tp, 0xb87c, 0x8fc0);
-		ocp_reg_write(tp, 0xb87e, 0x0800);
-		ocp_reg_write(tp, 0xb87c, 0x8fc2);
-		ocp_reg_write(tp, 0xb87e, 0xe500);
-		ocp_reg_write(tp, 0xb87c, 0x8fc4);
-		ocp_reg_write(tp, 0xb87e, 0x0f00);
-		ocp_reg_write(tp, 0xb87c, 0x8fc6);
-		ocp_reg_write(tp, 0xb87e, 0xf100);
-		ocp_reg_write(tp, 0xb87c, 0x8fc8);
-		ocp_reg_write(tp, 0xb87e, 0x0400);
-		ocp_reg_write(tp, 0xb87c, 0x8fca);
-		ocp_reg_write(tp, 0xb87e, 0xf300);
-		ocp_reg_write(tp, 0xb87c, 0x8fcc);
-		ocp_reg_write(tp, 0xb87e, 0xfd00);
-		ocp_reg_write(tp, 0xb87c, 0x8fce);
-		ocp_reg_write(tp, 0xb87e, 0xff00);
-		ocp_reg_write(tp, 0xb87c, 0x8fd0);
-		ocp_reg_write(tp, 0xb87e, 0xfb00);
-		ocp_reg_write(tp, 0xb87c, 0x8fd2);
-		ocp_reg_write(tp, 0xb87e, 0x0100);
-		ocp_reg_write(tp, 0xb87c, 0x8fd4);
-		ocp_reg_write(tp, 0xb87e, 0xf400);
-		ocp_reg_write(tp, 0xb87c, 0x8fd6);
-		ocp_reg_write(tp, 0xb87e, 0xff00);
-		ocp_reg_write(tp, 0xb87c, 0x8fd8);
-		ocp_reg_write(tp, 0xb87e, 0xf600);
+		sram2_write(tp, 0x8013, 0x0700);
+		sram2_write(tp, 0x8fb9, 0x2801);
+		sram2_write(tp, 0x8fba, 0x0100);
+		sram2_write(tp, 0x8fbc, 0x1900);
+		sram2_write(tp, 0x8fbe, 0xe100);
+		sram2_write(tp, 0x8fc0, 0x0800);
+		sram2_write(tp, 0x8fc2, 0xe500);
+		sram2_write(tp, 0x8fc4, 0x0f00);
+		sram2_write(tp, 0x8fc6, 0xf100);
+		sram2_write(tp, 0x8fc8, 0x0400);
+		sram2_write(tp, 0x8fca, 0xf300);
+		sram2_write(tp, 0x8fcc, 0xfd00);
+		sram2_write(tp, 0x8fce, 0xff00);
+		sram2_write(tp, 0x8fd0, 0xfb00);
+		sram2_write(tp, 0x8fd2, 0x0100);
+		sram2_write(tp, 0x8fd4, 0xf400);
+		sram2_write(tp, 0x8fd6, 0xff00);
+		sram2_write(tp, 0x8fd8, 0xf600);
 
 		ocp_byte_set_bits(tp, MCU_TYPE_PLA, PLA_USB_CFG,
 				  EN_XG_LIP | EN_G_LIP);
 
-		ocp_reg_write(tp, 0xb87c, 0x813d);
-		ocp_reg_write(tp, 0xb87e, 0x390e);
-		ocp_reg_write(tp, 0xb87c, 0x814f);
-		ocp_reg_write(tp, 0xb87e, 0x790e);
-		ocp_reg_write(tp, 0xb87c, 0x80b0);
-		ocp_reg_write(tp, 0xb87e, 0x0f31);
+		sram2_write(tp, 0x813d, 0x390e);
+		sram2_write(tp, 0x814f, 0x790e);
+		sram2_write(tp, 0x80b0, 0x0f31);
 		ocp_reg_set_bits(tp, 0xbf4c, BIT(1));
 		ocp_reg_set_bits(tp, 0xbcca, BIT(9) | BIT(8));
-		ocp_reg_write(tp, 0xb87c, 0x8141);
-		ocp_reg_write(tp, 0xb87e, 0x320e);
-		ocp_reg_write(tp, 0xb87c, 0x8153);
-		ocp_reg_write(tp, 0xb87e, 0x720e);
-		ocp_reg_write(tp, 0xb87c, 0x8529);
-		ocp_reg_write(tp, 0xb87e, 0x050e);
+		sram2_write(tp, 0x8141, 0x320e);
+		sram2_write(tp, 0x8153, 0x720e);
+		sram2_write(tp, 0x8529, 0x050e);
 		ocp_reg_clr_bits(tp, OCP_EEE_CFG, CTAP_SHORT_EN);
 
 		sram_write(tp, 0x816c, 0xc4a0);
@@ -7489,27 +7474,17 @@ static void r8156b_hw_phy_cfg(struct r8152 *tp)
 		sram_write(tp, 0x8ff1, 0x0404);
 
 		ocp_reg_write(tp, 0xbf4a, 0x001b);
-		ocp_reg_write(tp, 0xb87c, 0x8033);
-		ocp_reg_write(tp, 0xb87e, 0x7c13);
-		ocp_reg_write(tp, 0xb87c, 0x8037);
-		ocp_reg_write(tp, 0xb87e, 0x7c13);
-		ocp_reg_write(tp, 0xb87c, 0x803b);
-		ocp_reg_write(tp, 0xb87e, 0xfc32);
-		ocp_reg_write(tp, 0xb87c, 0x803f);
-		ocp_reg_write(tp, 0xb87e, 0x7c13);
-		ocp_reg_write(tp, 0xb87c, 0x8043);
-		ocp_reg_write(tp, 0xb87e, 0x7c13);
-		ocp_reg_write(tp, 0xb87c, 0x8047);
-		ocp_reg_write(tp, 0xb87e, 0x7c13);
-
-		ocp_reg_write(tp, 0xb87c, 0x8145);
-		ocp_reg_write(tp, 0xb87e, 0x370e);
-		ocp_reg_write(tp, 0xb87c, 0x8157);
-		ocp_reg_write(tp, 0xb87e, 0x770e);
-		ocp_reg_write(tp, 0xb87c, 0x8169);
-		ocp_reg_write(tp, 0xb87e, 0x0d0a);
-		ocp_reg_write(tp, 0xb87c, 0x817b);
-		ocp_reg_write(tp, 0xb87e, 0x1d0a);
+		sram2_write(tp, 0x8033, 0x7c13);
+		sram2_write(tp, 0x8037, 0x7c13);
+		sram2_write(tp, 0x803b, 0xfc32);
+		sram2_write(tp, 0x803f, 0x7c13);
+		sram2_write(tp, 0x8043, 0x7c13);
+		sram2_write(tp, 0x8047, 0x7c13);
+
+		sram2_write(tp, 0x8145, 0x370e);
+		sram2_write(tp, 0x8157, 0x770e);
+		sram2_write(tp, 0x8169, 0x0d0a);
+		sram2_write(tp, 0x817b, 0x1d0a);
 
 		sram_write_w0w1(tp, 0x8217, 0xff00, 0x5000);
 		sram_write_w0w1(tp, 0x821a, 0xff00, 0x5000);
@@ -7574,12 +7549,9 @@ static void r8156b_hw_phy_cfg(struct r8152 *tp)
 		fallthrough;
 	case RTL_VER_15:
 		/* EEE parameter */
-		ocp_reg_write(tp, 0xb87c, 0x80f5);
-		ocp_reg_write(tp, 0xb87e, 0x760e);
-		ocp_reg_write(tp, 0xb87c, 0x8107);
-		ocp_reg_write(tp, 0xb87e, 0x360e);
-		ocp_reg_write(tp, 0xb87c, 0x8551);
-		ocp_reg_w0w1(tp, 0xb87e, 0xff00, 0x0800);
+		sram2_write(tp, 0x80f5, 0x760e);
+		sram2_write(tp, 0x8107, 0x360e);
+		sram2_write_w0w1(tp, 0x8551, 0xff00, 0x0800);
 
 		/* ADC_PGA parameter */
 		ocp_reg_w0w1(tp, 0xbf00, 0xe000, 0xa000);
-- 
2.34.1


^ permalink raw reply related

* [PATCH] usbip: vhci: validate ret_submit number_of_packets
From: hkbinbin @ 2026-04-01 12:08 UTC (permalink / raw)
  To: valentina.manea.m, shuah, i, gregkh
  Cc: linux-usb, linux-kernel, hkbinbin, stable

vhci_recv_ret_submit() unpacks USBIP_RET_SUBMIT directly into the URB,
including number_of_packets from the remote server. For isochronous
URBs, iso_frame_desc[] was allocated using the original locally
submitted number_of_packets.

If a malicious or buggy USB/IP server returns a larger
number_of_packets, usbip_recv_iso() will iterate past the end of
urb->iso_frame_desc[] and write attacker-controlled ISO descriptors out
of bounds. Later completion paths may also walk past iso_frame_desc[]
if the poisoned number_of_packets is left in the URB after rejecting
the response.

Fix this by saving the original packet count before unpacking the PDU,
rejecting larger values from the server, restoring the original count
on error, and marking the connection as broken.

Fixes: 1325f85fa49f ("staging: usbip: bugfix add number of packets for isochronous frames")
Cc: stable@vger.kernel.org
Signed-off-by: hkbinbin <hkbinbinbin@gmail.com>
---
 drivers/usb/usbip/vhci_rx.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/usb/usbip/vhci_rx.c b/drivers/usb/usbip/vhci_rx.c
index a75f4a898a41..5bbfd5ae7755 100644
--- a/drivers/usb/usbip/vhci_rx.c
+++ b/drivers/usb/usbip/vhci_rx.c
@@ -60,6 +60,7 @@ static void vhci_recv_ret_submit(struct vhci_device *vdev,
 	struct usbip_device *ud = &vdev->ud;
 	struct urb *urb;
 	unsigned long flags;
+	int orig_number_of_packets;
 
 	spin_lock_irqsave(&vdev->priv_lock, flags);
 	urb = pickup_urb_and_free_priv(vdev, pdu->base.seqnum);
@@ -73,9 +74,33 @@ static void vhci_recv_ret_submit(struct vhci_device *vdev,
 		return;
 	}
 
+	/*
+	 * Save the original number_of_packets before it gets overwritten
+	 * by the server's response. The iso_frame_desc[] array was allocated
+	 * based on this value, so the server must not increase it.
+	 */
+	orig_number_of_packets = urb->number_of_packets;
+
 	/* unpack the pdu to a urb */
 	usbip_pack_pdu(pdu, urb, USBIP_RET_SUBMIT, 0);
 
+	/*
+	 * Validate number_of_packets from the server response against the
+	 * original URB allocation. A malicious server could set this to a
+	 * larger value, causing usbip_recv_iso() to write beyond the
+	 * iso_frame_desc[] array bounds.
+	 */
+	if (urb->number_of_packets < 0 ||
+	    urb->number_of_packets > orig_number_of_packets) {
+		dev_err(&urb->dev->dev,
+			"invalid number_of_packets in ret_submit: %d (max %d)\n",
+			urb->number_of_packets, orig_number_of_packets);
+		urb->number_of_packets = orig_number_of_packets;
+		urb->status = -EPROTO;
+		usbip_event_add(ud, VDEV_EVENT_ERROR_TCP);
+		goto error;
+	}
+
 	/* recv transfer buffer */
 	if (usbip_recv_xbuff(ud, urb) < 0) {
 		urb->status = -EPROTO;
-- 
2.51.0


^ permalink raw reply related

* Re: [PATCH v2] dma-debug: suppress cacheline overlap warning when arch has no DMA alignment requirement
From: Robin Murphy @ 2026-04-01 12:11 UTC (permalink / raw)
  To: Marek Szyprowski, Mikhail Gavrilov
  Cc: iommu, linux-kernel, linux-usb, linux-mm, harry, vbabka, akpm,
	stern, linux, andy.shevchenko, hch, Jeff.kirsher, catalin.marinas
In-Reply-To: <6270d4f0-85e4-496d-8db4-87ccb791ca4d@samsung.com>

On 2026-03-30 8:44 am, Marek Szyprowski wrote:
> On 27.03.2026 13:41, Mikhail Gavrilov wrote:
>> When CONFIG_DMA_API_DEBUG is enabled, the DMA debug infrastructure
>> tracks active mappings per cacheline and warns if two different DMA
>> mappings share the same cacheline ("cacheline tracking EEXIST,
>> overlapping mappings aren't supported").
>>
>> On x86_64, ARCH_KMALLOC_MINALIGN defaults to 8, so small kmalloc
>> allocations (e.g. the 8-byte hub->buffer and hub->status in the USB
>> hub driver) frequently land in the same 64-byte cacheline.  When both
>> are DMA-mapped, this triggers a false positive warning.
>>
>> This has been reported repeatedly since v5.14 (when the EEXIST check
>> was added) across various USB host controllers and devices including
>> xhci_hcd with USB hubs, USB audio devices, and USB ethernet adapters.
>>
>> The cacheline overlap is only a real concern on architectures that
>> require DMA buffer alignment to cacheline boundaries (i.e. where
>> ARCH_DMA_MINALIGN >= L1_CACHE_BYTES).  On architectures like x86_64
>> where dma_get_cache_alignment() returns 1, the hardware is
>> cache-coherent and overlapping cacheline mappings are harmless.
>>
>> Suppress the EEXIST warning when dma_get_cache_alignment() is less
>> than L1_CACHE_BYTES, indicating the architecture does not require
>> cacheline-aligned DMA buffers.

Really the value of this check is for mappings of structure members or 
array elements which have no inherent guarantee of being individually 
aligned to ARCH_DMA_MINALIGN, and while x86 can also get away with that, 
it represents a genuine issue for non-coherent architectures. USB's 
mapping of a dedicated tiny allocation is, if anything, rather the 
special case.

TBH I'd be inclined to have CONFIG_DMA_DEBUG raise ARCH_DMA_MINALIGN as 
appropriate such that genuine false-positives can't happen, rather than 
effectively defeat the whole check, but I suppose that might carry a 
risk of tripping up arch code that doesn't expect it... Oh well.

Thanks,
Robin.

>> Verified with a kernel module reproducer that performs two kmalloc(8)
>> allocations back-to-back and DMA-maps both:
>>
>>    Before: allocations share a cacheline, EEXIST fires within ~50 pairs
>>    After:  same cacheline pair found, but no warning emitted
>>
>> Fixes: 2b4bbc6231d7 ("dma-debug: report -EEXIST errors in add_dma_entry")
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215740
>> Suggested-by: Harry Yoo <harry@kernel.org>
>> Tested-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
>> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
> 
> Applied to dma-mapping-fixes. Thanks!
> 
>> ---
>>
>> v1 -> v2:
>>    - Moved fix from include/linux/slab.h (ARCH_KMALLOC_MINALIGN)
>>      to kernel/dma/debug.c per Harry Yoo's suggestion.
>>    - Instead of forcing cacheline-aligned allocations, suppress
>>      the warning when the architecture has no DMA alignment
>>      requirement (dma_get_cache_alignment() < L1_CACHE_BYTES).
>>
>> v1: https://lore.kernel.org/all/20260327055846.248829-1-mikhail.v.gavrilov@gmail.com/
>>
>> Reproducer module that triggers the bug reliably:
>>    https://bugzilla.kernel.org/attachment.cgi?id=309769
>>
>>   kernel/dma/debug.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
>> index 0677918f06a8..1a725edbbbf6 100644
>> --- a/kernel/dma/debug.c
>> +++ b/kernel/dma/debug.c
>> @@ -615,6 +615,7 @@ static void add_dma_entry(struct dma_debug_entry *entry, unsigned long attrs)
>>   	} else if (rc == -EEXIST &&
>>   		   !(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
>>   		   !(entry->is_cache_clean && overlap_cache_clean) &&
>> +		   dma_get_cache_alignment() >= L1_CACHE_BYTES &&
>>   		   !(IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) &&
>>   		     is_swiotlb_active(entry->dev))) {
>>   		err_printk(entry->dev, entry,
> 
> Best regards


^ permalink raw reply

* Re: [PATCH] usb: ulpi: fix double free in ulpi_register_interface() error path
From: Heikki Krogerus @ 2026-04-01 12:19 UTC (permalink / raw)
  To: Guangshuo Li
  Cc: Greg Kroah-Hartman, David Cohen, Felipe Balbi, linux-usb,
	linux-kernel, stable
In-Reply-To: <20260401025142.1398996-1-lgs201920130244@gmail.com>

On Wed, Apr 01, 2026 at 10:51:42AM +0800, Guangshuo Li wrote:
> When device_register() fails, ulpi_register() calls put_device() on
> ulpi->dev.
> 
> The device release callback ulpi_dev_release() drops the OF node
> reference and frees ulpi, but the current error path in
> ulpi_register_interface() then calls kfree(ulpi) again, causing a
> double free.
> 
> Let put_device() handle the cleanup through ulpi_dev_release() and
> avoid freeing ulpi again in ulpi_register_interface().
> 
> Fixes: 289fcff4bcdb1 ("usb: add bus type for USB ULPI")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/common/ulpi.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
> index 4a2ee447b213..d895cf6532a2 100644
> --- a/drivers/usb/common/ulpi.c
> +++ b/drivers/usb/common/ulpi.c
> @@ -331,10 +331,9 @@ struct ulpi *ulpi_register_interface(struct device *dev,
>  	ulpi->ops = ops;
>  
>  	ret = ulpi_register(dev, ulpi);
> -	if (ret) {
> -		kfree(ulpi);
> +	if (ret)
>  		return ERR_PTR(ret);
> -	}
> +
>  
>  	return ulpi;
>  }

-- 
heikki

^ permalink raw reply

* [PATCH v4 0/2] usb: offload: Decouple interrupter lifecycle and refactor usage tracking
From: Guan-Yu Lin @ 2026-04-01 12:32 UTC (permalink / raw)
  To: gregkh, mathias.nyman, perex, dominique.martinet, eadavis,
	hannelotta, tiwai, quic_wcheng, broonie, arnd, xiaopei01,
	wesley.cheng, sakari.ailus, stern, amardeep.rai, xu.yang_2,
	andriy.shevchenko, nkapron
  Cc: linux-usb, linux-kernel, linux-sound, Guan-Yu Lin

The current USB offload implementation couples the allocation of xHCI
sideband interrupters with the device's offload usage counter. This
coupling is conceptually incorrect, as hardware resource availability
and power management state serve distinct purposes.

Furthermore, the reliance on the coarse USB device lock for offload
state updates has led to potential recursive locking issues,
especially during device disconnect when the lock is already held
by the USB core.

This series refactors the offload synchronization by introducing a
dedicated spinlock for offload state, allowing for more granular
concurrency control and avoiding deadlocks. It also optimizes power
management by ensuring that offload state is only modified when the
device is already active, avoiding unnecessary auto-resumes.

Patch 1 introduces the `offload_lock` spinlock and `offload_pm_locked`
synchronization, replacing the coarse `udev->lock` and the legacy
`offload_at_suspend` flag. It also updates `usb_offload_get/put` to use
`pm_runtime_get_if_active()`.

Patch 2 removes the implicit usage tracking from the xHCI sideband layer
and delegates the responsibility to class drivers, who have the
correct context for managing offload data stream activity.

---
Changes in v4:
- remove spinlock in `usb_offload_check`.
- Collect the <Tested-by> tag from the OPPO team.
- Link to v3: https://lore.kernel.org/all/20260324203851.4091193-1-guanyulin@google.com/

Changes in v3:
- Replace the coarse USB device lock with a dedicated `offload_lock`
  spinlock to reduce contention and prevent recursive locking.
- Introduce `offload_pm_locked` to synchronize with PM transitions and
  replace the legacy `offload_at_suspend` flag.
- Optimize `usb_offload_get/put` by switching from auto-resume/suspend
  to `pm_runtime_get_if_active()`, avoiding unnecessary power transitions.
- Explicitly delegate `offload_usage` tracking to USB class drivers
  (e.g., the Qualcomm USB audio offload driver).
- Link to v2: https://lore.kernel.org/all/20260309022205.28136-1-guanyulin@google.com/

Changes in v2:
- Collect the <Tested-by> tag from the OPPO team.
- Link to v1: https://lore.kernel.org/all/20260225064601.270301-1-guanyulin@google.com/

Changes in v1:
- Fix build error when building sound/usb/qcom/qc_audio_offload.o.
- Link to RFC v2: https://lore.kernel.org/all/20260213100736.2914690-1-guanyulin@google.com/

Changes in RFC v2:
- Move device locking to callers.
- Decouple sideband from offload counting.
- Link to RFC v1: https://lore.kernel.org/all/20260130074746.287750-1-guanyulin@google.com/
---
Guan-Yu Lin (2):
  usb: core: use dedicated spinlock for offload state
  usb: host: xhci-sideband: delegate offload_usage tracking to class
    drivers

 drivers/usb/core/driver.c         |  23 ++++---
 drivers/usb/core/offload.c        | 102 +++++++++++++++++-------------
 drivers/usb/core/usb.c            |   1 +
 drivers/usb/host/xhci-sideband.c  |  18 +-----
 include/linux/usb.h               |  10 ++-
 sound/usb/qcom/qc_audio_offload.c |  10 ++-
 6 files changed, 94 insertions(+), 70 deletions(-)

-- 
2.53.0.1018.g2bb0e51243-goog


^ permalink raw reply

* [PATCH v4 1/2] usb: core: use dedicated spinlock for offload state
From: Guan-Yu Lin @ 2026-04-01 12:32 UTC (permalink / raw)
  To: gregkh, mathias.nyman, perex, dominique.martinet, eadavis,
	hannelotta, tiwai, quic_wcheng, broonie, arnd, xiaopei01,
	wesley.cheng, sakari.ailus, stern, amardeep.rai, xu.yang_2,
	andriy.shevchenko, nkapron
  Cc: linux-usb, linux-kernel, linux-sound, Guan-Yu Lin, stable,
	Hailong Liu
In-Reply-To: <20260401123238.3790062-1-guanyulin@google.com>

Replace the coarse USB device lock with a dedicated offload_lock
spinlock to reduce contention during offload operations. Use
offload_pm_locked to synchronize with PM transitions and replace
the legacy offload_at_suspend flag.

Optimize usb_offload_get/put by switching from auto-resume/suspend
to pm_runtime_get_if_active(). This ensures offload state is only
modified when the device is already active, avoiding unnecessary
power transitions.

Cc: stable@vger.kernel.org
Fixes: ef82a4803aab ("xhci: sideband: add api to trace sideband usage")
Signed-off-by: Guan-Yu Lin <guanyulin@google.com>
Tested-by: Hailong Liu <hailong.liu@oppo.com>
---
 drivers/usb/core/driver.c        |  23 ++++---
 drivers/usb/core/offload.c       | 102 ++++++++++++++++++-------------
 drivers/usb/core/usb.c           |   1 +
 drivers/usb/host/xhci-sideband.c |   4 +-
 include/linux/usb.h              |  10 ++-
 5 files changed, 84 insertions(+), 56 deletions(-)

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index d29edc7c616a..74b8bdc27dbf 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -1415,14 +1415,16 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
 	int			status = 0;
 	int			i = 0, n = 0;
 	struct usb_interface	*intf;
+	bool			offload_active = false;
 
 	if (udev->state == USB_STATE_NOTATTACHED ||
 			udev->state == USB_STATE_SUSPENDED)
 		goto done;
 
+	usb_offload_set_pm_locked(udev, true);
 	if (msg.event == PM_EVENT_SUSPEND && usb_offload_check(udev)) {
 		dev_dbg(&udev->dev, "device offloaded, skip suspend.\n");
-		udev->offload_at_suspend = 1;
+		offload_active = true;
 	}
 
 	/* Suspend all the interfaces and then udev itself */
@@ -1436,8 +1438,7 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
 			 * interrupt urbs, allowing interrupt events to be
 			 * handled during system suspend.
 			 */
-			if (udev->offload_at_suspend &&
-			    intf->needs_remote_wakeup) {
+			if (offload_active && intf->needs_remote_wakeup) {
 				dev_dbg(&intf->dev,
 					"device offloaded, skip suspend.\n");
 				continue;
@@ -1452,7 +1453,7 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
 		}
 	}
 	if (status == 0) {
-		if (!udev->offload_at_suspend)
+		if (!offload_active)
 			status = usb_suspend_device(udev, msg);
 
 		/*
@@ -1498,7 +1499,7 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
 	 */
 	} else {
 		udev->can_submit = 0;
-		if (!udev->offload_at_suspend) {
+		if (!offload_active) {
 			for (i = 0; i < 16; ++i) {
 				usb_hcd_flush_endpoint(udev, udev->ep_out[i]);
 				usb_hcd_flush_endpoint(udev, udev->ep_in[i]);
@@ -1507,6 +1508,8 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
 	}
 
  done:
+	if (status != 0)
+		usb_offload_set_pm_locked(udev, false);
 	dev_vdbg(&udev->dev, "%s: status %d\n", __func__, status);
 	return status;
 }
@@ -1536,16 +1539,19 @@ static int usb_resume_both(struct usb_device *udev, pm_message_t msg)
 	int			status = 0;
 	int			i;
 	struct usb_interface	*intf;
+	bool			offload_active = false;
 
 	if (udev->state == USB_STATE_NOTATTACHED) {
 		status = -ENODEV;
 		goto done;
 	}
 	udev->can_submit = 1;
+	if (msg.event == PM_EVENT_RESUME)
+		offload_active = usb_offload_check(udev);
 
 	/* Resume the device */
 	if (udev->state == USB_STATE_SUSPENDED || udev->reset_resume) {
-		if (!udev->offload_at_suspend)
+		if (!offload_active)
 			status = usb_resume_device(udev, msg);
 		else
 			dev_dbg(&udev->dev,
@@ -1562,8 +1568,7 @@ static int usb_resume_both(struct usb_device *udev, pm_message_t msg)
 			 * pending interrupt urbs, allowing interrupt events
 			 * to be handled during system suspend.
 			 */
-			if (udev->offload_at_suspend &&
-			    intf->needs_remote_wakeup) {
+			if (offload_active && intf->needs_remote_wakeup) {
 				dev_dbg(&intf->dev,
 					"device offloaded, skip resume.\n");
 				continue;
@@ -1572,11 +1577,11 @@ static int usb_resume_both(struct usb_device *udev, pm_message_t msg)
 					udev->reset_resume);
 		}
 	}
-	udev->offload_at_suspend = 0;
 	usb_mark_last_busy(udev);
 
  done:
 	dev_vdbg(&udev->dev, "%s: status %d\n", __func__, status);
+	usb_offload_set_pm_locked(udev, false);
 	if (!status)
 		udev->reset_resume = 0;
 	return status;
diff --git a/drivers/usb/core/offload.c b/drivers/usb/core/offload.c
index 7c699f1b8d2b..9db3cfedd29c 100644
--- a/drivers/usb/core/offload.c
+++ b/drivers/usb/core/offload.c
@@ -25,33 +25,30 @@
  */
 int usb_offload_get(struct usb_device *udev)
 {
-	int ret;
+	int ret = 0;
 
-	usb_lock_device(udev);
-	if (udev->state == USB_STATE_NOTATTACHED) {
-		usb_unlock_device(udev);
+	if (!usb_get_dev(udev))
 		return -ENODEV;
-	}
 
-	if (udev->state == USB_STATE_SUSPENDED ||
-		   udev->offload_at_suspend) {
-		usb_unlock_device(udev);
-		return -EBUSY;
+	if (pm_runtime_get_if_active(&udev->dev) != 1) {
+		ret = -EBUSY;
+		goto err_rpm;
 	}
 
-	/*
-	 * offload_usage could only be modified when the device is active, since
-	 * it will alter the suspend flow of the device.
-	 */
-	ret = usb_autoresume_device(udev);
-	if (ret < 0) {
-		usb_unlock_device(udev);
-		return ret;
+	spin_lock(&udev->offload_lock);
+
+	if (udev->offload_pm_locked) {
+		ret = -EAGAIN;
+		goto err;
 	}
 
 	udev->offload_usage++;
-	usb_autosuspend_device(udev);
-	usb_unlock_device(udev);
+
+err:
+	spin_unlock(&udev->offload_lock);
+	pm_runtime_put_autosuspend(&udev->dev);
+err_rpm:
+	usb_put_dev(udev);
 
 	return ret;
 }
@@ -69,35 +66,32 @@ EXPORT_SYMBOL_GPL(usb_offload_get);
  */
 int usb_offload_put(struct usb_device *udev)
 {
-	int ret;
+	int ret = 0;
 
-	usb_lock_device(udev);
-	if (udev->state == USB_STATE_NOTATTACHED) {
-		usb_unlock_device(udev);
+	if (!usb_get_dev(udev))
 		return -ENODEV;
-	}
 
-	if (udev->state == USB_STATE_SUSPENDED ||
-		   udev->offload_at_suspend) {
-		usb_unlock_device(udev);
-		return -EBUSY;
+	if (pm_runtime_get_if_active(&udev->dev) != 1) {
+		ret = -EBUSY;
+		goto err_rpm;
 	}
 
-	/*
-	 * offload_usage could only be modified when the device is active, since
-	 * it will alter the suspend flow of the device.
-	 */
-	ret = usb_autoresume_device(udev);
-	if (ret < 0) {
-		usb_unlock_device(udev);
-		return ret;
+	spin_lock(&udev->offload_lock);
+
+	if (udev->offload_pm_locked) {
+		ret = -EAGAIN;
+		goto err;
 	}
 
 	/* Drop the count when it wasn't 0, ignore the operation otherwise. */
 	if (udev->offload_usage)
 		udev->offload_usage--;
-	usb_autosuspend_device(udev);
-	usb_unlock_device(udev);
+
+err:
+	spin_unlock(&udev->offload_lock);
+	pm_runtime_put_autosuspend(&udev->dev);
+err_rpm:
+	usb_put_dev(udev);
 
 	return ret;
 }
@@ -112,25 +106,47 @@ EXPORT_SYMBOL_GPL(usb_offload_put);
  * management.
  *
  * The caller must hold @udev's device lock. In addition, the caller should
- * ensure downstream usb devices are all either suspended or marked as
- * "offload_at_suspend" to ensure the correctness of the return value.
+ * ensure the device itself and the downstream usb devices are all marked as
+ * "offload_pm_locked" to ensure the correctness of the return value.
  *
  * Returns true on any offload activity, false otherwise.
  */
 bool usb_offload_check(struct usb_device *udev) __must_hold(&udev->dev->mutex)
 {
 	struct usb_device *child;
-	bool active;
+	bool active = false;
 	int port1;
 
+	if (udev->offload_usage)
+		return true;
+
 	usb_hub_for_each_child(udev, port1, child) {
 		usb_lock_device(child);
 		active = usb_offload_check(child);
 		usb_unlock_device(child);
+
 		if (active)
-			return true;
+			break;
 	}
 
-	return !!udev->offload_usage;
+	return active;
 }
 EXPORT_SYMBOL_GPL(usb_offload_check);
+
+/**
+ * usb_offload_set_pm_locked - set the PM lock state of a USB device
+ * @udev: the USB device to modify
+ * @locked: the new lock state
+ *
+ * Setting @locked to true prevents offload_usage from being modified. This
+ * ensures that offload activities cannot be started or stopped during critical
+ * power management transitions, maintaining a stable state for the duration
+ * of the transition.
+ */
+void usb_offload_set_pm_locked(struct usb_device *udev, bool locked)
+{
+	spin_lock(&udev->offload_lock);
+	udev->offload_pm_locked = locked;
+	spin_unlock(&udev->offload_lock);
+}
+EXPORT_SYMBOL_GPL(usb_offload_set_pm_locked);
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index e740f7852bcd..8f7ca084010f 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -671,6 +671,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
 	set_dev_node(&dev->dev, dev_to_node(bus->sysdev));
 	dev->state = USB_STATE_ATTACHED;
 	dev->lpm_disable_count = 1;
+	spin_lock_init(&dev->offload_lock);
 	dev->offload_usage = 0;
 	atomic_set(&dev->urbnum, 0);
 
diff --git a/drivers/usb/host/xhci-sideband.c b/drivers/usb/host/xhci-sideband.c
index 2bd77255032b..1ddb64b0a48e 100644
--- a/drivers/usb/host/xhci-sideband.c
+++ b/drivers/usb/host/xhci-sideband.c
@@ -291,8 +291,8 @@ EXPORT_SYMBOL_GPL(xhci_sideband_get_event_buffer);
  * Allow other drivers, such as usb controller driver, to check if there are
  * any sideband activity on the host controller. This information could be used
  * for power management or other forms of resource management. The caller should
- * ensure downstream usb devices are all either suspended or marked as
- * "offload_at_suspend" to ensure the correctness of the return value.
+ * ensure downstream usb devices are all marked as "offload_pm_locked" to ensure
+ * the correctness of the return value.
  *
  * Returns true on any active sideband existence, false otherwise.
  */
diff --git a/include/linux/usb.h b/include/linux/usb.h
index fbfcc70b07fb..a4b031196da3 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -21,6 +21,7 @@
 #include <linux/completion.h>	/* for struct completion */
 #include <linux/sched.h>	/* for current && schedule_timeout */
 #include <linux/mutex.h>	/* for struct mutex */
+#include <linux/spinlock.h>	/* for spinlock_t */
 #include <linux/pm_runtime.h>	/* for runtime PM */
 
 struct usb_device;
@@ -636,8 +637,9 @@ struct usb3_lpm_parameters {
  * @do_remote_wakeup:  remote wakeup should be enabled
  * @reset_resume: needs reset instead of resume
  * @port_is_suspended: the upstream port is suspended (L2 or U3)
- * @offload_at_suspend: offload activities during suspend is enabled.
+ * @offload_pm_locked: prevents offload_usage changes during PM transitions.
  * @offload_usage: number of offload activities happening on this usb device.
+ * @offload_lock: protects offload_usage and offload_pm_locked
  * @slot_id: Slot ID assigned by xHCI
  * @l1_params: best effor service latency for USB2 L1 LPM state, and L1 timeout.
  * @u1_params: exit latencies for USB3 U1 LPM state, and hub-initiated timeout.
@@ -726,8 +728,9 @@ struct usb_device {
 	unsigned do_remote_wakeup:1;
 	unsigned reset_resume:1;
 	unsigned port_is_suspended:1;
-	unsigned offload_at_suspend:1;
+	unsigned offload_pm_locked:1;
 	int offload_usage;
+	spinlock_t offload_lock;
 	enum usb_link_tunnel_mode tunnel_mode;
 	struct device_link *usb4_link;
 
@@ -849,6 +852,7 @@ static inline void usb_mark_last_busy(struct usb_device *udev)
 int usb_offload_get(struct usb_device *udev);
 int usb_offload_put(struct usb_device *udev);
 bool usb_offload_check(struct usb_device *udev);
+void usb_offload_set_pm_locked(struct usb_device *udev, bool locked);
 #else
 
 static inline int usb_offload_get(struct usb_device *udev)
@@ -857,6 +861,8 @@ static inline int usb_offload_put(struct usb_device *udev)
 { return 0; }
 static inline bool usb_offload_check(struct usb_device *udev)
 { return false; }
+static inline void usb_offload_set_pm_locked(struct usb_device *udev, bool locked)
+{ }
 #endif
 
 extern int usb_disable_lpm(struct usb_device *udev);
-- 
2.53.0.1018.g2bb0e51243-goog


^ permalink raw reply related

* [PATCH v4 2/2] usb: host: xhci-sideband: delegate offload_usage tracking to class drivers
From: Guan-Yu Lin @ 2026-04-01 12:32 UTC (permalink / raw)
  To: gregkh, mathias.nyman, perex, dominique.martinet, eadavis,
	hannelotta, tiwai, quic_wcheng, broonie, arnd, xiaopei01,
	wesley.cheng, sakari.ailus, stern, amardeep.rai, xu.yang_2,
	andriy.shevchenko, nkapron
  Cc: linux-usb, linux-kernel, linux-sound, Guan-Yu Lin, stable,
	Hailong Liu
In-Reply-To: <20260401123238.3790062-1-guanyulin@google.com>

Remove usb_offload_get() and usb_offload_put() from the xHCI sideband
interrupter creation and removal paths.

The responsibility of manipulating offload_usage now lies entirely with
the USB class drivers. They have the precise context of when an offload
data stream actually starts and stops, ensuring a much more accurate
representation of offload activity for power management.

Cc: stable@vger.kernel.org
Fixes: ef82a4803aab ("xhci: sideband: add api to trace sideband usage")
Signed-off-by: Guan-Yu Lin <guanyulin@google.com>
Tested-by: Hailong Liu <hailong.liu@oppo.com>
---
 drivers/usb/host/xhci-sideband.c  | 14 +-------------
 sound/usb/qcom/qc_audio_offload.c | 10 +++++++++-
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/host/xhci-sideband.c b/drivers/usb/host/xhci-sideband.c
index 1ddb64b0a48e..651973606137 100644
--- a/drivers/usb/host/xhci-sideband.c
+++ b/drivers/usb/host/xhci-sideband.c
@@ -93,8 +93,6 @@ __xhci_sideband_remove_endpoint(struct xhci_sideband *sb, struct xhci_virt_ep *e
 static void
 __xhci_sideband_remove_interrupter(struct xhci_sideband *sb)
 {
-	struct usb_device *udev;
-
 	lockdep_assert_held(&sb->mutex);
 
 	if (!sb->ir)
@@ -102,10 +100,6 @@ __xhci_sideband_remove_interrupter(struct xhci_sideband *sb)
 
 	xhci_remove_secondary_interrupter(xhci_to_hcd(sb->xhci), sb->ir);
 	sb->ir = NULL;
-	udev = sb->vdev->udev;
-
-	if (udev->state != USB_STATE_NOTATTACHED)
-		usb_offload_put(udev);
 }
 
 /* sideband api functions */
@@ -328,9 +322,6 @@ int
 xhci_sideband_create_interrupter(struct xhci_sideband *sb, int num_seg,
 				 bool ip_autoclear, u32 imod_interval, int intr_num)
 {
-	int ret = 0;
-	struct usb_device *udev;
-
 	if (!sb || !sb->xhci)
 		return -ENODEV;
 
@@ -348,12 +339,9 @@ xhci_sideband_create_interrupter(struct xhci_sideband *sb, int num_seg,
 	if (!sb->ir)
 		return -ENOMEM;
 
-	udev = sb->vdev->udev;
-	ret = usb_offload_get(udev);
-
 	sb->ir->ip_autoclear = ip_autoclear;
 
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(xhci_sideband_create_interrupter);
 
diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
index cfb30a195364..abafbc78dea9 100644
--- a/sound/usb/qcom/qc_audio_offload.c
+++ b/sound/usb/qcom/qc_audio_offload.c
@@ -699,6 +699,7 @@ static void uaudio_event_ring_cleanup_free(struct uaudio_dev *dev)
 		uaudio_iommu_unmap(MEM_EVENT_RING, IOVA_BASE, PAGE_SIZE,
 				   PAGE_SIZE);
 		xhci_sideband_remove_interrupter(uadev[dev->chip->card->number].sb);
+		usb_offload_put(dev->udev);
 	}
 }
 
@@ -1182,12 +1183,16 @@ static int uaudio_event_ring_setup(struct snd_usb_substream *subs,
 	dma_coherent = dev_is_dma_coherent(subs->dev->bus->sysdev);
 	er_pa = 0;
 
+	ret = usb_offload_get(subs->dev);
+	if (ret < 0)
+		goto exit;
+
 	/* event ring */
 	ret = xhci_sideband_create_interrupter(uadev[card_num].sb, 1, false,
 					       0, uaudio_qdev->data->intr_num);
 	if (ret < 0) {
 		dev_err(&subs->dev->dev, "failed to fetch interrupter\n");
-		goto exit;
+		goto put_offload;
 	}
 
 	sgt = xhci_sideband_get_event_buffer(uadev[card_num].sb);
@@ -1219,6 +1224,8 @@ static int uaudio_event_ring_setup(struct snd_usb_substream *subs,
 	mem_info->dma = 0;
 remove_interrupter:
 	xhci_sideband_remove_interrupter(uadev[card_num].sb);
+put_offload:
+	usb_offload_put(subs->dev);
 exit:
 	return ret;
 }
@@ -1483,6 +1490,7 @@ static int prepare_qmi_response(struct snd_usb_substream *subs,
 	uaudio_iommu_unmap(MEM_EVENT_RING, IOVA_BASE, PAGE_SIZE, PAGE_SIZE);
 free_sec_ring:
 	xhci_sideband_remove_interrupter(uadev[card_num].sb);
+	usb_offload_put(subs->dev);
 drop_sync_ep:
 	if (subs->sync_endpoint) {
 		uaudio_iommu_unmap(MEM_XFER_RING,
-- 
2.53.0.1018.g2bb0e51243-goog


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox