Maverick CVE: tty: Make tiocgicount a handler, CVE-2010-4076

Brad Figg brad.figg at canonical.com
Tue Feb 22 16:22:55 UTC 2011


On 02/18/2011 05:07 AM, Stefan Bader wrote:
> On 02/17/2011 07:45 PM, Tim Gardner wrote:
>> The following changes since commit d19552cc47cd3ad6e1b8b5662b31f943be68262d:
>>    Tim Gardner (1):
>>          UBUNTU: Bump ABI
>>
>> are available in the git repository at:
>>
>>    git://kernel.ubuntu.com/rtg/ubuntu-maverick.git CVE-2010-4076
>>
>> Alan Cox (1):
>>        tty: Make tiocgicount a handler, CVE-2010-4076
>>
>>   drivers/char/tty_io.c           |   21 +++++++++++++++++++++
>>   drivers/serial/serial_core.c    |   37 +++++++++++++++++--------------------
>>   drivers/usb/serial/usb-serial.c |   13 +++++++++++++
>>   include/linux/tty_driver.h      |    9 +++++++++
>>   include/linux/usb/serial.h      |    2 ++
>>   5 files changed, 62 insertions(+), 20 deletions(-)
>>
>>  From fa3da07bd08bd979c6b8d98c93acaefcbfeb7d90 Mon Sep 17 00:00:00 2001
>> From: Alan Cox<alan at linux.intel.com>
>> Date: Thu, 16 Sep 2010 18:21:24 +0100
>> Subject: [PATCH] tty: Make tiocgicount a handler, CVE-2010-4076
>>
>> BugLink: http://bugs.launchpad.net/bugs/720189
>>
>> CVE-2010-4076
>>
>> Dan Rosenberg noted that various drivers return the struct with uncleared
>> fields. Instead of spending forever trying to stomp all the drivers that
>> get it wrong (and every new driver) do the job in one place.
>>
>> This first patch adds the needed operations and hooks them up, including
>> the needed USB midlayer and serial core plumbing.
>>
>> Signed-off-by: Alan Cox<alan at linux.intel.com>
>> Signed-off-by: Greg Kroah-Hartman<gregkh at suse.de>
>>
>> (cherry picked from commit d281da7ff6f70efca0553c288bb883e8605b3862)
>>
>> Signed-off-by: Tim Gardner<tim.gardner at canonical.com>
> Acked-by: Stefan Bader<stefan.bader at canonical.com>
>> ---
>>   drivers/char/tty_io.c           |   21 +++++++++++++++++++++
>>   drivers/serial/serial_core.c    |   37 +++++++++++++++++--------------------
>>   drivers/usb/serial/usb-serial.c |   13 +++++++++++++
>>   include/linux/tty_driver.h      |    9 +++++++++
>>   include/linux/usb/serial.h      |    2 ++
>>   5 files changed, 62 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
>> index 507441a..3a69c39 100644
>> --- a/drivers/char/tty_io.c
>> +++ b/drivers/char/tty_io.c
>> @@ -96,6 +96,7 @@
>>   #include<linux/bitops.h>
>>   #include<linux/delay.h>
>>   #include<linux/seq_file.h>
>> +#include<linux/serial.h>
>>
>>   #include<linux/uaccess.h>
>>   #include<asm/system.h>
>> @@ -2456,6 +2457,20 @@ static int tty_tiocmset(struct tty_struct *tty, struct file *file, unsigned int
>>   	return tty->ops->tiocmset(tty, file, set, clear);
>>   }
>>
>> +static int tty_tiocgicount(struct tty_struct *tty, void __user *arg)
>> +{
>> +	int retval = -EINVAL;
>> +	struct serial_icounter_struct icount;
>> +	memset(&icount, 0, sizeof(icount));
>> +	if (tty->ops->get_icount)
>> +		retval = tty->ops->get_icount(tty,&icount);
>> +	if (retval != 0)
>> +		return retval;
>> +	if (copy_to_user(arg,&icount, sizeof(icount)))
>> +		return -EFAULT;
>> +	return 0;
>> +}
>> +
>>   struct tty_struct *tty_pair_get_tty(struct tty_struct *tty)
>>   {
>>   	if (tty->driver->type == TTY_DRIVER_TYPE_PTY&&
>> @@ -2576,6 +2591,12 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>>   	case TIOCMBIC:
>>   	case TIOCMBIS:
>>   		return tty_tiocmset(tty, file, cmd, p);
>> +	case TIOCGICOUNT:
>> +		retval = tty_tiocgicount(tty, p);
>> +		/* For the moment allow fall through to the old method */
>> +        	if (retval != -EINVAL)
>> +			return retval;
>> +		break;
>>   	case TCFLSH:
>>   		switch (arg) {
>>   		case TCIFLUSH:
>> diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
>> index 7f28307..232e2bb 100644
>> --- a/drivers/serial/serial_core.c
>> +++ b/drivers/serial/serial_core.c
>> @@ -1074,10 +1074,10 @@ uart_wait_modem_status(struct uart_state *state, unsigned long arg)
>>    * NB: both 1->0 and 0->1 transitions are counted except for
>>    *     RI where only 0->1 is counted.
>>    */
>> -static int uart_get_count(struct uart_state *state,
>> -			  struct serial_icounter_struct __user *icnt)
>> +static int uart_get_icount(struct tty_struct *tty,
>> +			  struct serial_icounter_struct *icount)
>>   {
>> -	struct serial_icounter_struct icount;
>> +	struct uart_state *state = tty->driver_data;
>>   	struct uart_icount cnow;
>>   	struct uart_port *uport = state->uart_port;
>>
>> @@ -1085,19 +1085,19 @@ static int uart_get_count(struct uart_state *state,
>>   	memcpy(&cnow,&uport->icount, sizeof(struct uart_icount));
>>   	spin_unlock_irq(&uport->lock);
>>
>> -	icount.cts         = cnow.cts;
>> -	icount.dsr         = cnow.dsr;
>> -	icount.rng         = cnow.rng;
>> -	icount.dcd         = cnow.dcd;
>> -	icount.rx          = cnow.rx;
>> -	icount.tx          = cnow.tx;
>> -	icount.frame       = cnow.frame;
>> -	icount.overrun     = cnow.overrun;
>> -	icount.parity      = cnow.parity;
>> -	icount.brk         = cnow.brk;
>> -	icount.buf_overrun = cnow.buf_overrun;
>> +	icount->cts         = cnow.cts;
>> +	icount->dsr         = cnow.dsr;
>> +	icount->rng         = cnow.rng;
>> +	icount->dcd         = cnow.dcd;
>> +	icount->rx          = cnow.rx;
>> +	icount->tx          = cnow.tx;
>> +	icount->frame       = cnow.frame;
>> +	icount->overrun     = cnow.overrun;
>> +	icount->parity      = cnow.parity;
>> +	icount->brk         = cnow.brk;
>> +	icount->buf_overrun = cnow.buf_overrun;
>>
>> -	return copy_to_user(icnt,&icount, sizeof(icount)) ? -EFAULT : 0;
>> +	return 0;
>>   }
>>
>>   /*
>> @@ -1150,10 +1150,6 @@ uart_ioctl(struct tty_struct *tty, struct file *filp, unsigned int cmd,
>>   	case TIOCMIWAIT:
>>   		ret = uart_wait_modem_status(state, arg);
>>   		break;
>> -
>> -	case TIOCGICOUNT:
>> -		ret = uart_get_count(state, uarg);
>> -		break;
>>   	}
>>
>>   	if (ret != -ENOIOCTLCMD)
>> @@ -2305,6 +2301,7 @@ static const struct tty_operations uart_ops = {
>>   #endif
>>   	.tiocmget	= uart_tiocmget,
>>   	.tiocmset	= uart_tiocmset,
>> +	.get_icount	= uart_get_icount,
>>   #ifdef CONFIG_CONSOLE_POLL
>>   	.poll_init	= uart_poll_init,
>>   	.poll_get_char	= uart_poll_get_char,
>> diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
>> index 8fadc13..e8d141d 100644
>> --- a/drivers/usb/serial/usb-serial.c
>> +++ b/drivers/usb/serial/usb-serial.c
>> @@ -520,6 +520,18 @@ static int serial_tiocmset(struct tty_struct *tty, struct file *file,
>>   	return -EINVAL;
>>   }
>>
>> +static int serial_get_icount(struct tty_struct *tty,
>> +				struct serial_icounter_struct *icount)
>> +{
>> +	struct usb_serial_port *port = tty->driver_data;
>> +
>> +	dbg("%s - port %d", __func__, port->number);
>> +
>> +	if (port->serial->type->get_icount)
>> +		return port->serial->type->get_icount(tty, icount);
>> +	return -EINVAL;
>> +}
>> +
>>   /*
>>    * We would be calling tty_wakeup here, but unfortunately some line
>>    * disciplines have an annoying habit of calling tty->write from
>> @@ -1209,6 +1221,7 @@ static const struct tty_operations serial_ops = {
>>   	.chars_in_buffer =	serial_chars_in_buffer,
>>   	.tiocmget =		serial_tiocmget,
>>   	.tiocmset =		serial_tiocmset,
>> +	.get_icount = 		serial_get_icount,
>>   	.cleanup = 		serial_cleanup,
>>   	.install = 		serial_install,
>>   	.proc_fops =		&serial_proc_fops,
>> diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
>> index b086779..db2d227 100644
>> --- a/include/linux/tty_driver.h
>> +++ b/include/linux/tty_driver.h
>> @@ -224,6 +224,12 @@
>>    *	unless the tty also has a valid tty->termiox pointer.
>>    *
>>    *	Optional: Called under the termios lock
>> + *
>> + * int (*get_icount)(struct tty_struct *tty, struct serial_icounter *icount);
>> + *
>> + *	Called when the device receives a TIOCGICOUNT ioctl. Passed a kernel
>> + *	structure to complete. This method is optional and will only be called
>> + *	if provided (otherwise EINVAL will be returned).
>>    */
>>
>>   #include<linux/fs.h>
>> @@ -232,6 +238,7 @@
>>
>>   struct tty_struct;
>>   struct tty_driver;
>> +struct serial_icounter_struct;
>>
>>   struct tty_operations {
>>   	struct tty_struct * (*lookup)(struct tty_driver *driver,
>> @@ -268,6 +275,8 @@ struct tty_operations {
>>   			unsigned int set, unsigned int clear);
>>   	int (*resize)(struct tty_struct *tty, struct winsize *ws);
>>   	int (*set_termiox)(struct tty_struct *tty, struct termiox *tnew);
>> +	int (*get_icount)(struct tty_struct *tty,
>> +				struct serial_icounter_struct *icount);
>>   #ifdef CONFIG_CONSOLE_POLL
>>   	int (*poll_init)(struct tty_driver *driver, int line, char *options);
>>   	int (*poll_get_char)(struct tty_driver *driver, int line);
>> diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
>> index 84a4c44..8288b57 100644
>> --- a/include/linux/usb/serial.h
>> +++ b/include/linux/usb/serial.h
>> @@ -271,6 +271,8 @@ struct usb_serial_driver {
>>   	int  (*tiocmget)(struct tty_struct *tty, struct file *file);
>>   	int  (*tiocmset)(struct tty_struct *tty, struct file *file,
>>   			 unsigned int set, unsigned int clear);
>> +	int  (*get_icount)(struct tty_struct *tty,
>> +			struct serial_icounter_struct *icount);
>>   	/* Called by the tty layer for port level work. There may or may not
>>   	   be an attached tty at this point */
>>   	void (*dtr_rts)(struct usb_serial_port *port, int on);
>
>

Acked-by: Brad Figg <brad.figg at canonical.com>

-- 
Brad Figg brad.figg at canonical.com http://www.canonical.com




More information about the kernel-team mailing list