Re: [PATCH] usb: misc: legousbtower: Fix NULL pointer deference

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Sep 19, 2016 at 07:09:51PM +0100, James wrote:
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> This patch fixes a NULL pointer dereference caused by a race codition in the
> probe function of the legousbtower driver. It re-structures the probe
> function to only register the interface after successfully reading the
> board's firmware ID.
> 
> 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. 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.
> 
> This bug has existed since 2003. Patch tested by emulated device.
> 
> Reported-by: James Patrick-Evans <james@jmp-e.com>
> Tested-by: James Patrick-Evans <james@jmp-e.com>
> Signed-off-by: James Patrick-Evans <james@jmp-e.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> drivers/usb/misc/legousbtower.c | 35 +++++++++++++++++------------------
> 1 file changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
> index 7771be3..4dd531a 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;
> --

Ugh, all of the tabs got turned into spaces, next time be more careful
with your email client :(

thanks,

greg k-h