Re: [PATCH] USB: console: fix potential use after free

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]