Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

USB: core: Fix races in character device registration and deregistraion

The syzbot fuzzer has found two (!) races in the USB character device
registration and deregistration routines. This patch fixes the races.

The first race results from the fact that usb_deregister_dev() sets
usb_minors[intf->minor] to NULL before calling device_destroy() on the
class device. This leaves a window during which another thread can
allocate the same minor number but will encounter a duplicate name
error when it tries to register its own class device. A typical error
message in the system log would look like:

sysfs: cannot create duplicate filename '/class/usbmisc/ldusb0'

The patch fixes this race by destroying the class device first.

The second race is in usb_register_dev(). When that routine runs, it
first allocates a minor number, then drops minor_rwsem, and then
creates the class device. If the device creation fails, the minor
number is deallocated and the whole routine returns an error. But
during the time while minor_rwsem was dropped, there is a window in
which the minor number is allocated and so another thread can
successfully open the device file. Typically this results in
use-after-free errors or invalid accesses when the other thread closes
its open file reference, because the kernel then tries to release
resources that were already deallocated when usb_register_dev()
failed. The patch fixes this race by keeping minor_rwsem locked
throughout the entire routine.

Reported-and-tested-by: syzbot+30cf45ebfe0b0c4847a1@syzkaller.appspotmail.com
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/Pine.LNX.4.44L0.1908121607590.1659-100000@iolanthe.rowland.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Alan Stern and committed by
Greg Kroah-Hartman
303911cf 0cf25bc5

+5 -5
+5 -5
drivers/usb/core/file.c
··· 193 193 intf->minor = minor; 194 194 break; 195 195 } 196 - up_write(&minor_rwsem); 197 - if (intf->minor < 0) 196 + if (intf->minor < 0) { 197 + up_write(&minor_rwsem); 198 198 return -EXFULL; 199 + } 199 200 200 201 /* create a usb class device for this usb interface */ 201 202 snprintf(name, sizeof(name), class_driver->name, minor - minor_base); ··· 204 203 MKDEV(USB_MAJOR, minor), class_driver, 205 204 "%s", kbasename(name)); 206 205 if (IS_ERR(intf->usb_dev)) { 207 - down_write(&minor_rwsem); 208 206 usb_minors[minor] = NULL; 209 207 intf->minor = -1; 210 - up_write(&minor_rwsem); 211 208 retval = PTR_ERR(intf->usb_dev); 212 209 } 210 + up_write(&minor_rwsem); 213 211 return retval; 214 212 } 215 213 EXPORT_SYMBOL_GPL(usb_register_dev); ··· 234 234 return; 235 235 236 236 dev_dbg(&intf->dev, "removing %d minor\n", intf->minor); 237 + device_destroy(usb_class->class, MKDEV(USB_MAJOR, intf->minor)); 237 238 238 239 down_write(&minor_rwsem); 239 240 usb_minors[intf->minor] = NULL; 240 241 up_write(&minor_rwsem); 241 242 242 - device_destroy(usb_class->class, MKDEV(USB_MAJOR, intf->minor)); 243 243 intf->usb_dev = NULL; 244 244 intf->minor = -1; 245 245 destroy_usb_class();