• Alan Stern's avatar
    p54usb: Fix race between disconnect and firmware loading · 87874179
    Alan Stern authored
    BugLink: https://bugs.launchpad.net/bugs/1860681
    
    commit 6e41e225 upstream.
    
    The syzbot fuzzer found a bug in the p54 USB wireless driver.  The
    issue involves a race between disconnect and the firmware-loader
    callback routine, and it has several aspects.
    
    One big problem is that when the firmware can't be loaded, the
    callback routine tries to unbind the driver from the USB _device_ (by
    calling device_release_driver) instead of from the USB _interface_ to
    which it is actually bound (by calling usb_driver_release_interface).
    
    The race involves access to the private data structure.  The driver's
    disconnect handler waits for a completion that is signalled by the
    firmware-loader callback routine.  As soon as the completion is
    signalled, you have to assume that the private data structure may have
    been deallocated by the disconnect handler -- even if the firmware was
    loaded without errors.  However, the callback routine does access the
    private data several times after that point.
    
    Another problem is that, in order to ensure that the USB device
    structure hasn't been freed when the callback routine runs, the driver
    takes a reference to it.  This isn't good enough any more, because now
    that the callback routine calls usb_driver_release_interface, it has
    to ensure that the interface structure hasn't been freed.
    
    Finally, the driver takes an unnecessary reference to the USB device
    structure in the probe function and drops the reference in the
    disconnect handler.  This extra reference doesn't accomplish anything,
    because the USB core already guarantees that a device structure won't
    be deallocated while a driver is still bound to any of its interfaces.
    
    To fix these problems, this patch makes the following changes:
    
    	Call usb_driver_release_interface() rather than
    	device_release_driver().
    
    	Don't signal the completion until after the important
    	information has been copied out of the private data structure,
    	and don't refer to the private data at all thereafter.
    
    	Lock udev (the interface's parent) before unbinding the driver
    	instead of locking udev->parent.
    
    	During the firmware loading process, take a reference to the
    	USB interface instead of the USB device.
    
    	Don't take an unnecessary reference to the device during probe
    	(and then don't drop it during disconnect).
    Signed-off-by: default avatarAlan Stern <stern@rowland.harvard.edu>
    Reported-and-tested-by: syzbot+200d4bb11b23d929335f@syzkaller.appspotmail.com
    Acked-by: default avatarChristian Lamparter <chunkeey@gmail.com>
    Signed-off-by: default avatarKalle Valo <kvalo@codeaurora.org>
    [bwh: Backported to 4.4: adjust filename]
    Signed-off-by: default avatarBen Hutchings <ben.hutchings@codethink.co.uk>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    Signed-off-by: default avatarConnor Kuehl <connor.kuehl@canonical.com>
    Signed-off-by: default avatarKhalid Elmously <khalid.elmously@canonical.com>
    87874179
p54usb.c 29.1 KB