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. Ben. > Yes, the USB console is a bit of a hack. -- Ben Hutchings Never attribute to conspiracy what can adequately be explained by stupidity.
Attachment:
signature.asc
Description: This is a digitally signed message part