Re: use-after-free in usbnet

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On Wed, Mar 21, 2012 at 09:04:15AM +0800, Ming Lei wrote:
> On Tue, Mar 20, 2012 at 5:40 PM, Ming Lei <tom.leiming@xxxxxxxxx> wrote:
> > Hi,
> >
> > On Mon, Mar 19, 2012 at 11:12 PM, Dave Jones <davej@xxxxxxxxxx> wrote:
> >> We've had two reports of this use after free in Fedora now recently..
> >
> > Could you provide output of 'dmesg' and 'lsusb -v' from the reported machine?
> 
> Looks I have figured out why your problem is triggered.
> 
> If the URB being unlinked is freed before usb_put_dev
> inside usb_hcd_unlink_urb, the use-after-free will be triggered.
> And the below patch[1] should fix the problem.

With the reference counting we have, how can the urb be freed at this
point in time?  Is the driver doing wierd things with the urb reference
counts?

> Also there is another bug in tx_complete() of usbnet, the line below
> 
>                 urb->dev = NULL;
> 
> should be removed to avoid possible oops or memory leak in unlink path.
> 
> Please test the patch if you can reproduce the problem.
> 
> [1],
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index 59681f0..4f4e028 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -592,7 +592,9 @@ static int unlink_urbs (struct usbnet *dev, struct
> sk_buff_head *q)
>  		spin_unlock_irqrestore(&q->lock, flags);
>  		// during some PM-driven resume scenarios,
>  		// these (async) unlinks complete immediately
> +		local_bh_disable();
>  		retval = usb_unlink_urb (urb);
> +		local_bh_enable();

That doesn't seem right, as you point out in your follow-up message.
This shouldn't be needed, unless you are doing some really wierd things
with the urb :(

greg k-h
--
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


B and H Foto and Electronics Corp.

[Linux Media]     [Video for Linux]     [Linux Input]     [Linux Audio Users]     [Photo]     [Yosemite News]    [Yosemite Photos]    [Free Online Dating]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]     [More Archives]

Add to Google Powered by Linux