linux-kernel - Re: [PATCH] fix urb unmapping issue when the uas device is remove during ongoing data transfer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aOpdRAGzuPViOYBu@oa-guhuinan-2.localdomain>
Date: Sat, 11 Oct 2025 21:36:04 +0800
From: Owen Gu <guhuinan@...omi.com>
To: Michal Pecio <michal.pecio@...il.com>
CC: Oliver Neukum <oneukum@...e.com>, Alan Stern <stern@...land.harvard.edu>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, <linux-usb@...r.kernel.org>,
	<linux-scsi@...r.kernel.org>, <usb-storage@...ts.one-eyed-alien.net>,
	<linux-kernel@...r.kernel.org>, Yu Chen <chenyu45@...omi.com>
Subject: Re: [PATCH] fix urb unmapping issue when the uas device is remove
 during ongoing data transfer

On Tue, Oct 07, 2025 at 10:57:18PM +0200, Michal Pecio wrote:
> On Tue, 30 Sep 2025 12:53:08 +0800, guhuinan wrote:
> > From: Owen Gu <guhuinan@...omi.com>
> > 
> > When a UAS device is unplugged during data transfer, there is
> > a probability of a system panic occurring. The root cause is
> > an access to an invalid memory address during URB callback handling.
> > Specifically, this happens when the dma_direct_unmap_sg() function
> > is called within the usb_hcd_unmap_urb_for_dma() interface, but the
> > sg->dma_address field is 0 and the sg data structure has already been
> > freed.
> > 
> > The SCSI driver sends transfer commands by invoking uas_queuecommand_lck()
> > in uas.c, using the uas_submit_urbs() function to submit requests to USB.
> > Within the uas_submit_urbs() implementation, three URBs (sense_urb,
> > data_urb, and cmd_urb) are sequentially submitted. Device removal may
> > occur at any point during uas_submit_urbs execution, which may result
> > in URB submission failure. However, some URBs might have been successfully
> > submitted before the failure, and uas_submit_urbs will return the -ENODEV
> > error code in this case. The current error handling directly calls
> > scsi_done(). In the SCSI driver, this eventually triggers scsi_complete()
> > to invoke scsi_end_request() for releasing the sgtable. The successfully
> > submitted URBs, when being completed (giveback), call
> > usb_hcd_unmap_urb_for_dma() in hcd.c, leading to exceptions during sg
> > unmapping operations since the sg data structure has already been freed.
> > 
> > This patch modifies the error condition check in the uas_submit_urbs()
> > function. When a UAS device is removed but one or more URBs have already
> > been successfully submitted to USB, it avoids immediately invoking
> > scsi_done(). Instead, it waits for the successfully submitted URBs to
> > complete , and then triggers the scsi_done() function call within
> > uas_try_complete() after all pending URB operations are finalized.
> > 
> > Signed-off-by: Yu Chen <chenyu45@...omi.com>
> > Signed-off-by: Owen Gu <guhuinan@...omi.com>
> 
> Hi,
> 
> Was this situation seen in the wild and/or reproduced, or only
> predicted theoretically? Was the patch tested?
>

This problem occurs in real testing environments with low probability
when the UAS device is unplugged during data transmission. Current
patches have been tested and can resolve this issue. However, as
Oliver mentioned in his response, if the resetting flag is set to 1
at this time, it might prevent scsi_done() from being called, which
was not considered in Patch V1.

> I wonder what happens to the submitted URBs when scsi_done() is
> not called. Since the command URB was not submitted (or else we
> wouldn't be here I guess?) the device shouldn't have selected this
> stream before disconnection and it seems that the xHC won't try
> to move data on those URBs, so they won't complete with -EPROTO.
> 

Submitted URBs will be processed through usb_disable_endpoint()
during device removal, eventually triggering usb_kill_urb() in the
usb_disconnect() workflow. This will invoke the completion interface
through usb_hcd_giveback_urb(), returning -ESHUTDOWN to the UAS driver. 

> Will they sit there stuck until SCSI core times out and aborts
> the command? That's poor UX, speaking from experience here.
> 

After applying the current patch, testing has shown that failed commands
in the UAS driver for which scsi_done() was not called after submission
will be completed 30 seconds after a SCSI command timeout occurs.
We will optimize this issue in patch V2, which is currently under testing.

> Maybe it would make sense to unlink them? Unlinking Streams URBs
> is a sensitive topic because it's forbidden if they can become
> the Current Stream, but in this case it looks like they can't.
> 
> Or am I missing something?
> 
> Regards,
> Michal
Dear,

Sorry for the delayed response to this issue.We anticipate
uploading an updated patch V2 next week.

Regards