On Mon, Feb 16, 2015 at 03:49:17AM +0000, Ben Hutchings wrote: > On Wed, 2015-02-11 at 14:55 +0800, Johan Hovold wrote: > > On Tue, Feb 10, 2015 at 08:39:26PM +0000, Ben Hutchings wrote: > > > On Mon, 2015-01-05 at 16:04 +0100, Johan Hovold wrote: > > > > Use tty kref to release the fake tty in usb_console_setup to avoid use > > > > after free if the underlying serial driver has acquired a reference. > > > > > > > > Note that using the tty destructor release_one_tty requires some more > > > > state to be initialised. > > > [...] > > > > --- a/drivers/usb/serial/console.c > > > > +++ b/drivers/usb/serial/console.c > > > [...] > > > > @@ -137,14 +139,17 @@ static int usb_console_setup(struct console *co, > > > > char *options) > > > > goto reset_open_count; > > > > } > > > > kref_init(&tty->kref); > > > > - tty_port_tty_set(&port->port, tty); > > > > tty->driver = usb_serial_tty_driver; > > > > tty->index = co->index; > > > > init_ldsem(&tty->ldisc_sem); > > > > + INIT_LIST_HEAD(&tty->tty_files); > > > > + kref_get(&tty->driver->kref); > > > > + tty->ops = &usb_console_fake_tty_ops; > > > [...] > > > > > > Do we also need: > > > __module_get(tty->driver->owner); > > > or am I missing something? > > > > When the console setup code is running, and while the console is open, > > everything is pinned through a reference to the usb-serial-driver module > > (the tty layer also pins usb-serial core directly). > > > > We should be holding a reference to the usb-serial driver module (which > > in turns pins usb-serial core, which is the tty driver) for when the > > console isn't open as well, although that is not directly related to the > > fix in question. > > Sure, but release_one_tty() expects that the tty has a reference to the > module and it will drop that reference. (Normally tty_init_dev() takes > that reference.) I think that means that for a USB serial console we > take one reference to the module when setting up but drop two references > when cleaning up.
Ah, but the tty-driver owner will be null as usb-serial has to be compiled in to enable the usb console (and therefore the point I made above about a missing reference is also moot). So nothing is leaking, but I should probably add a comment or a call to __module_get() nonetheless to make this clear. > > Yes, the USB console is a bit of a hack. Thanks, Johan -- 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
