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

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

 



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 stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




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