On Fri, Sep 02, 2016 at 01:20:21AM +0100, James wrote:
> This patch fixes a NULL pointer dereference caused by a race codition in the
> probe
> function of the legousbtower driver. The probe function does not deregister
> the
> usb interface after an error receiving the devices firmware ID. The device
> file
> registered (/dev/usb/legousbtower%d) may be read/written globally before the
> probe
> function returns; then when tower_delete is called in the probe function
> (after an
> r/w has been initiated), core dev structures are deleted while the file
> operation
> functions are still running. If the 0 address is mappable on the machine, this
> vulnerability can be used to create a Local Priviege Escalation exploit via a
> write-what-where condition by remapping dev->interrupt_out_buffer in
> tower_write.
> A forged USB device and local program execution would be required for LPE.
> The USB device would have to delay the control message in tower_probe and
> accept the control urb in tower_open whilst guest code initiated a write to
> the device file as tower_delete is called from the error in tower_probe.
How about this simpler patch, that moves the registering for the minor
number as the last thing that happens in the probe function (which is
what it should be).
Can you test this to verify it prevents the above problem?
And this driver really needs an overhaul one of these days...
thanks,
greg k-h
diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
index 7771be3ac178..4dd531ac5a7f 100644
--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -898,24 +898,6 @@ static int tower_probe (struct usb_interface *interface,
const struct usb_device
dev->interrupt_in_interval = interrupt_in_interval ?
interrupt_in_interval : dev->interrupt_in_endpoint->bInterval;
dev->interrupt_out_interval = interrupt_out_interval ?
interrupt_out_interval : dev->interrupt_out_endpoint->bInterval;
- /* we can register the device now, as it is ready */
- usb_set_intfdata (interface, dev);
-
- retval = usb_register_dev (interface, &tower_class);
-
- if (retval) {
- /* something prevented us from registering this driver */
- dev_err(idev, "Not able to get a minor for this device.\n");
- usb_set_intfdata (interface, NULL);
- goto error;
- }
- dev->minor = interface->minor;
-
- /* let the user know what node this device is now attached to */
- dev_info(&interface->dev, "LEGO USB Tower #%d now attached to major "
- "%d minor %d\n", (dev->minor - LEGO_USB_TOWER_MINOR_BASE),
- USB_MAJOR, dev->minor);
-
/* get the firmware version and log it */
result = usb_control_msg (udev,
usb_rcvctrlpipe(udev, 0),
@@ -936,6 +918,23 @@ static int tower_probe (struct usb_interface *interface,
const struct usb_device
get_version_reply.minor,
le16_to_cpu(get_version_reply.build_no));
+ /* we can register the device now, as it is ready */
+ usb_set_intfdata (interface, dev);
+
+ retval = usb_register_dev (interface, &tower_class);
+
+ if (retval) {
+ /* something prevented us from registering this driver */
+ dev_err(idev, "Not able to get a minor for this device.\n");
+ usb_set_intfdata (interface, NULL);
+ goto error;
+ }
+ dev->minor = interface->minor;
+
+ /* let the user know what node this device is now attached to */
+ dev_info(&interface->dev, "LEGO USB Tower #%d now attached to major "
+ "%d minor %d\n", (dev->minor - LEGO_USB_TOWER_MINOR_BASE),
+ USB_MAJOR, dev->minor);
exit:
return retval;
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
