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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251007225718.3c8b2cd8.michal.pecio@gmail.com>
Date: Tue, 7 Oct 2025 22:57:18 +0200
From: Michal Pecio <michal.pecio@...il.com>
To: guhuinan <guhuinan@...omi.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, 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?

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.

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

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