On 10/09/2014 10:26 AM, Alan Stern wrote: > On Thu, 9 Oct 2014, Andre Wolokita wrote: > >>>>> Isn't this now a "use-after-free" issue? >>>>> >>>> >>>> Are you referring to the subsequent call to wait event() on gs_closed()? >>> >>> Yes. >>> >>>> Testing the use-case with this patch applied seemed to work without any >>>> issues. The ttyGS0 reference in /dev/ is gone after running modprobe -r >>>> but I'm just a newbie, so I could be doing sometime horrible here. >>> >>> Hm, I dug into the tty core, it should be ok, but it just seems really >>> odd, and bad-form to be doing something with port->port after calling a >>> "destroy" function with it, don't you agree? >> >> I do. The call to wait_event() can be removed as we no longer care whether >> gs_closed(port) is returning true - if it even can, having destroyed the >> tty_port. > > Maybe you don't care whether gs_closed(port) returns true, but you > should care about whether gs_closed() crashes -- which it might well do > if it tries to access deallocated memory. > > Did you test your patch by unloading the module while there were > pending opens?
gs_closed() won't crash because that's not the memory being deallocated. tty_port_destroy() is somewhat misnamed; should be more like tty_port_cleanup(), as it doesn't actually deallocate the port (because tty_port could be an embedded structure in a larger 'port' which is the most common usage). I've been struggling to understand how this patch could even cause the wait_event() to complete and the only thing I can hypothesize is that the deallocation in tty_port_destroy() somehow causes the process with the open tty to crash but I don't see how. Regards, Peter Hurley PS - My earlier email was greylisted so that's why I appeared to reply well after your answer :) X-Greylist: delayed 4693 seconds by postgrey-1.27 at vger.kernel.org; Thu, 09 Oct 2014 11:06:05 EDT -- 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
