On Wed, Sep 28, 2016 at 08:38:07AM +0200, Ladislav Michl wrote: > On Thu, Aug 04, 2016 at 10:15:33PM +0200, Ladislav Michl wrote: > > On Thu, Aug 04, 2016 at 06:59:30AM +0200, Greg Kroah-Hartman wrote: > > > On Wed, Aug 03, 2016 at 09:06:25AM +0200, Ladislav Michl wrote: > > > > Yes, also was lacking proper description and signoff. So, I'm considering > > > > ioctls based approach okay, although that question (the only one I really > > > > had) was never answered. > > > > > > > > After re-reading specifications [*] I decided to allow arbitrary MsgID > > > > selection, as USB488 adds MsgID=TRIGGER (128) and other subclass > > > > specifications may add other values. > > > > > > > > [*] http://www.usb.org/developers/docs/devclass_docs/USBTMC_1_006a.zip > > > > > > > > After sorting out all eventual objections, patch bellow will be turned > > > > into proper one. > > > > > > Looks reasonable to me. > > > > After all, it is not that reasonable. Spec does not define anything like > > EOM or TermCharEnabled bits in bmTransferAttributes nor TermChar field > > in vendor specific messages - usbtmc_read and usbtmc_write are using > > these fields when concatenating usb transfers. I'll need to think a bit > > more about it... > > Hmm, sorry for a delay, I was working on something else for a while. > So far using libusb seems to be better approach, but I do not want to > leave bugs I know about unfixed, so here we go... > > > Meanwhile, there's a small race condition, which needs to be fixed first. > > When device is disconnected there's a chance usbtmc_read tries to lock > > already destroyd mutex, see bellow. Fix will come later in separate patch. > > Problem comes with commit e6c7efdcb76f11b04e3d3f71c8d764ab75c9423b (usbtmc: > convert to devm_kzalloc). As memory allocated with devm_kzalloc is > automatically freed on driver detach, later call to usbtmc_read will crash > as bellow. Anyone has better fix than reverting that patch? 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. Nice catch, thanks. 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
