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.
>
> NB: This has existed since 2003.
There are some patch style issues for your patch.
- The subject for your patch, it should be the same with subject at your
commit log, and use your module name for it. Eg, usb: misc:
legousbtower: xxxx
- You may use mutt to send patch, then, the whole patch content will be
in body.
- The description of your commit log should be no more than 75 characters
per line
- If it is a bug fix, you may cc to stable tree.
Check: Documentation/SubmittingPatches for detail.
>
> Signed-off-by: James Patrick-Evans <james@xxxxxxxxx>
> ---
> commit e65459348a2cb61bce6c7eae3ea26bb7a07e1255
> Author: James Patrick-Evans <james@xxxxxxxxx>
> Date: Fri Sep 2 00:36:01 2016 +0100
>
> legousbtower: Fixes race condition leading to NULL pointer dereference
> in tower_probe
>
> diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
> index 7771be3..51fa183 100644
> --- a/drivers/usb/misc/legousbtower.c
> +++ b/drivers/usb/misc/legousbtower.c
> @@ -898,6 +898,10 @@ 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;
>
> + /* Lock device mutexes */
> + mutex_lock(&dev->lock);
> + mutex_lock(&open_disc_mutex);
> +
> /* we can register the device now, as it is ready */
> usb_set_intfdata (interface, dev);
>
> @@ -907,7 +911,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
> /* 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;
> + goto unlock_error;
> }
> dev->minor = interface->minor;
>
> @@ -928,18 +932,23 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
> 1000);
> if (result < 0) {
> dev_err(idev, "LEGO USB Tower get version control request failed\n");
> + usb_deregister_dev (interface, &tower_class);
> retval = result;
> - goto error;
> + goto unlock_error;
> }
> dev_info(&interface->dev, "LEGO USB Tower firmware version is %d.%d "
> "build %d\n", get_version_reply.major,
> get_version_reply.minor,
> le16_to_cpu(get_version_reply.build_no));
>
> -
> + mutex_lock(&open_disc_mutex);
> + mutex_lock(&dev->lock);
mutex_unlock?
> exit:
> return retval;
>
> +unlock_error:
> + mutex_lock(&open_disc_mutex);
> + mutex_lock(&dev->lock);
mutex_unlock?
> error:
> tower_delete(dev);
> return retval;
>
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
