On Tue, 24 Mar 2020, Oliver Neukum wrote:
> Am Montag, den 23.03.2020, 02:18 -0400 schrieb Kyungtae Kim:
> > We report a bug (in linux-5.5.11) found by FuzzUSB (a modified version
> > of syzkaller)
>
> Hi,
>
> thank you for the report. Is this a reproducible bug?
>
> > In function usb_hcd_unlink_urb (driver/usb/core/hcd.c:1607), it tries to
> > read "urb->use_count". But it seems the instance "urb" was
> > already freed (right after urb->dev at line 1597) by the function "urb_destroy"
> > in a different thread, which caused memory access violation.
>
> Yes.
>
> > To solve, it may need to check if urb is valid before urb->use_count,
> > to avoid such freed memory access.
>
> Difficult to do as the URB itself would be invalid.
>
> I am afraid there is a race in here:
>
>
> if (test_bit(US_FLIDX_ABORTING, &us->dflags)) {
> /* cancel the request, if it hasn't been cancelled already */
> if (test_and_clear_bit(US_FLIDX_SG_ACTIVE, &us->dflags)) {
> usb_stor_dbg(us, "-- cancelling sg request\n");
> usb_sg_cancel(&us->current_sg);
> }
> }
>
> /* wait for the completion of the transfer */
> usb_sg_wait(&us->current_sg);
> clear_bit(US_FLIDX_SG_ACTIVE, &us->dflags);
>
>
> What keeps the request alive while usb_sg_wait() is running?
It's a bug in the SG library code. I'll post a patch later on,
although it's not clear whether anyone will be able to test it
properly.
Alan Stern
