On Wed, Sep 28, 2016 at 12:29:35PM +0200, Ladislav Michl wrote: > On Wed, Sep 28, 2016 at 11:45:02AM +0200, Greg Kroah-Hartman wrote: > > If it is freed, why is a read even able to happen? Ah, ick, no proper > > reference counting is happening here :( > > > > Oh, no, wait, it is happening properly, it's just that it's not the > > lifespan that the devm_kzalloc() is attached to, so yes, the correct fix > > here is to revert that patch as it is incorrect. > > Well, reference counting is also suspicious as kref_get(&data->kref) in > probe function with comment "will reference data in int urb" gives no > clue why there's explicit reference. Also what if we add classic > error unwinding and leave usb_submit_urb to open time? But wait, this > driver allows multiple opens? Is it intentional? It could be done this > way (note patch is only for reference as there's nothing to prevent > multiple open and therefore multiple usb_submit_urb): > The usbtmc_interrupt routine will reference data in the int urb from the interrupt context. Also submitting the urb on probe makes for simpler housekeeping. Only one interrupt urb needs to be submitted no matter how many concurrent opens there are for the minor. I will submit a patch to revert the "convert to devm_kzalloc" patch and make the comment more explicit. cheers, -dave -- 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
