On Mon, May 15, 2017 at 10:34:35AM +0200, Oliver Neukum wrote: > Am Freitag, den 12.05.2017, 15:00 +0200 schrieb Michael Grzeschik: > > The usbip stack handles the kmalloc and kfree of the transfered buffers. > > Some > > USB-Stacks add the flag URB_FREE_BUFFER to their urbs, so the usb layer > > removes > > it in usb_free_urb. This can lead to double free situations as the usbip > > stack > > already removes its created buffers. To avoid that we remove this flag from > > the > > usbip transfered urbs. > > Hi, > > something is fishy here. urb_destroy() frees the buffer and the URB. > If this leads to a double free ever, you are already accessing freed > memory. This patch is a definite NACK. The analysis may be right, but > the fix is wrong.
As I already mentioned in the patch, I had doubts that this was the right solution. We should probably add the flag URB_FREE_BUFFER to every usbip based urb and let the urb_destroy free them. As the USBIP stack itself is never reusing the transfer_buffer memory for new urbs. I will write a v2 patch. Regards, Michael -- 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 | -- 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
