Re: [PATCH v3] usb: usbip: let urb_destroy kfree the transfer_buffer

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

 



On Wed, May 17, 2017 at 12:07:50PM +0200, Michael Grzeschik wrote:
> The usbip stack dynamically allocates the transfer_buffer of each urb in
> stub_recv_cmd_submit depending on the requested transfer_buffer_length
> set via the received tcp packet.
> 
> As the usbip layer never reuses the always different sized
> transfer_buffer it can add URB_FREE_BUFFER to every urbs transfer_flags
> and let urb_destroy call the kfree for it.
> 
> This patch fixes double kfree situations where the usbip remote side
> added the URB_FREE_BUFFER.
> 
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> ---
> v1 -> v2: - rephrased patch subject from:
>             "usb: usbip: avoid the usb layer to kfree our allocated buffer"
>           - changed to always let urb_destoy remove the transfer_buffer
> v2 -> v3: - added stable to cc
>           - wraped long line with over 80 chars
> 
>  drivers/usb/usbip/stub_main.c    | 1 -
>  drivers/usb/usbip/stub_tx.c      | 1 -
>  drivers/usb/usbip/usbip_common.c | 9 ++++++++-
>  3 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/usbip/stub_main.c b/drivers/usb/usbip/stub_main.c
> index 44ab43fc4fcc7..898e0d271fd47 100644
> --- a/drivers/usb/usbip/stub_main.c
> +++ b/drivers/usb/usbip/stub_main.c
> @@ -261,7 +261,6 @@ void stub_device_cleanup_urbs(struct stub_device *sdev)
>  
>  		kmem_cache_free(stub_priv_cache, priv);
>  
> -		kfree(urb->transfer_buffer);
>  		kfree(urb->setup_packet);
>  		usb_free_urb(urb);
>  	}

I found the corresponding code in vudc_dev.c where the
urb->transfer_buffer is set to NULL. This also solves the double kfree
situations. I don't know if we also should prefer that fix in stub_main
and stub_tx aswell or instead also remove the explicit kfree of
urb->transfer_buffer in the vudc code.

> diff --git a/drivers/usb/usbip/stub_tx.c b/drivers/usb/usbip/stub_tx.c
> index 6b1e8c3f0e4b2..23d1d76766d2c 100644
> --- a/drivers/usb/usbip/stub_tx.c
> +++ b/drivers/usb/usbip/stub_tx.c
> @@ -28,7 +28,6 @@ static void stub_free_priv_and_urb(struct stub_priv *priv)
>  	struct urb *urb = priv->urb;
>  
>  	kfree(urb->setup_packet);
> -	kfree(urb->transfer_buffer);
>  	list_del(&priv->list);
>  	kmem_cache_free(stub_priv_cache, priv);
>  	usb_free_urb(urb);
> diff --git a/drivers/usb/usbip/usbip_common.c b/drivers/usb/usbip/usbip_common.c
> index cab2b71a80d02..829b608135989 100644
> --- a/drivers/usb/usbip/usbip_common.c
> +++ b/drivers/usb/usbip/usbip_common.c
> @@ -381,6 +381,12 @@ static unsigned int tweak_transfer_flags(unsigned int flags)
>  	return flags;
>  }
>  
> +static unsigned int tweak_submit_transfer_flags(unsigned int flags)
> +{
> +	flags |= URB_FREE_BUFFER;
> +	return flags;
> +}
> +
>  static void usbip_pack_cmd_submit(struct usbip_header *pdu, struct urb *urb,
>  				  int pack)
>  {
> @@ -398,7 +404,8 @@ static void usbip_pack_cmd_submit(struct usbip_header *pdu, struct urb *urb,
>  		spdu->number_of_packets		= urb->number_of_packets;
>  		spdu->interval			= urb->interval;
>  	} else  {
> -		urb->transfer_flags         = spdu->transfer_flags;
> +		urb->transfer_flags         =
> +			tweak_submit_transfer_flags(spdu->transfer_flags);
>  		urb->transfer_buffer_length = spdu->transfer_buffer_length;
>  		urb->start_frame            = spdu->start_frame;
>  		urb->number_of_packets      = spdu->number_of_packets;
> -- 
> 2.11.0
> 
> --
> 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

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]