Hardy CVE: tty: Make tiocgicount a handler, CVE-2010-4076
Stefan Bader
stefan.bader at canonical.com
Fri Feb 18 13:37:09 UTC 2011
On 02/18/2011 02:28 PM, Stefan Bader wrote:
> On 02/17/2011 06:34 PM, Tim Gardner wrote:
>> The following changes since commit ecccfa52383097f93172113f1e9dae76deeda904:
>> Tim Gardner (1):
>> UBUNTU: Bump ABI
>>
>> are available in the git repository at:
>>
>> git://kernel.ubuntu.com/rtg/ubuntu-hardy.git CVE-2010-4076
>>
>> Tim Gardner (1):
>> tty: Make tiocgicount a handler, CVE-2010-4076
>>
>> .../openvz/patchset/0001-2.6.24-ovz002.patch | 22 +++++++-----
>> drivers/char/tty_io.c | 18 +++++++++
>> drivers/serial/serial_core.c | 37 +++++++++-----------
>> drivers/usb/serial/usb-serial.c | 13 +++++++
>> include/linux/tty_driver.h | 11 ++++++
>> include/linux/usb/serial.h | 2 +
>> 6 files changed, 74 insertions(+), 29 deletions(-)
>>
>> From 21d71f1a2ca7a69dafe6366b6116e3abac4b8433 Mon Sep 17 00:00:00 2001
>> From: Tim Gardner <tim.gardner at canonical.com>
>> Date: Wed, 16 Feb 2011 10:39:22 -0700
>> 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>
>>
>> (backported from commit d281da7ff6f70efca0553c288bb883e8605b3862)
>>
>> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
>> ---
>> .../openvz/patchset/0001-2.6.24-ovz002.patch | 22 +++++++-----
>> drivers/char/tty_io.c | 18 +++++++++
>> drivers/serial/serial_core.c | 37 +++++++++-----------
>> drivers/usb/serial/usb-serial.c | 13 +++++++
>> include/linux/tty_driver.h | 11 ++++++
>> include/linux/usb/serial.h | 2 +
>> 6 files changed, 74 insertions(+), 29 deletions(-)
>>
>> diff --git a/debian/binary-custom.d/openvz/patchset/0001-2.6.24-ovz002.patch b/debian/binary-custom.d/openvz/patchset/0001-2.6.24-ovz002.patch
>> index f8443e0..729b278 100644
>> --- a/debian/binary-custom.d/openvz/patchset/0001-2.6.24-ovz002.patch
>> +++ b/debian/binary-custom.d/openvz/patchset/0001-2.6.24-ovz002.patch
>> @@ -4639,15 +4639,6 @@ Index: kernel/drivers/char/tty_io.c
>> ===================================================================
>> --- kernel.orig/drivers/char/tty_io.c 2008-11-18 01:19:43.000000000 +0100
>> +++ kernel/drivers/char/tty_io.c 2008-11-24 15:47:45.000000000 +0100
>> -@@ -94,6 +94,8 @@
>> - #include <linux/wait.h>
>> - #include <linux/bitops.h>
>> - #include <linux/delay.h>
>> -+#include <linux/nsproxy.h>
>> -+#include <linux/ve.h>
>> -
>> - #include <asm/uaccess.h>
>> - #include <asm/system.h>
>> @@ -104,6 +106,7 @@
>>
>> #include <linux/kmod.h>
>> @@ -91574,3 +91565,16 @@ Index: kernel/sound/core/info.c
>> }
>> return 0;
>> }
>> +diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
>> +index e85486b..8171d69 100644
>> +--- a/drivers/char/tty_io.c
>> ++++ b/drivers/char/tty_io.c
>> +@@ -94,6 +94,8 @@
>> + #include <linux/wait.h>
>> + #include <linux/bitops.h>
>> + #include <linux/delay.h>
>> ++#include <linux/nsproxy.h>
>> ++#include <linux/ve.h>
>> + #include <linux/serial.h>
>> +
>> + #include <asm/uaccess.h>
>> diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
>> index 23d7cfd..bbc03b2 100644
>> --- a/drivers/char/tty_io.c
>> +++ b/drivers/char/tty_io.c
>> @@ -94,6 +94,7 @@
>> #include <linux/wait.h>
>> #include <linux/bitops.h>
>> #include <linux/delay.h>
>> +#include <linux/serial.h>
>>
>> #include <asm/uaccess.h>
>> #include <asm/system.h>
>> @@ -3302,6 +3303,20 @@ static int tty_tiocmset(struct tty_struct *tty, struct file *file, unsigned int
>> return retval;
>> }
>>
>> +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->driver->get_icount)
>> + retval = tty->driver->get_icount(tty, &icount);
>> + if (retval != 0)
>> + return retval;
>> + if (copy_to_user(arg, &icount, sizeof(icount)))
>> + return -EFAULT;
>> + return 0;
>> +}
>> +
>> /*
>> * Split this up, as gcc can choke on it otherwise..
>> */
>> @@ -3435,6 +3450,8 @@ int tty_ioctl(struct inode * inode, struct file * file,
>> case TIOCMBIC:
>> case TIOCMBIS:
>> return tty_tiocmset(tty, file, cmd, p);
>> + case TIOCGICOUNT:
>> + return tty_tiocgicount(tty, p);
>
> The original patch seems to allow to fall-through here if -EINVAL is returned.
> It looks to be possible here too. Any reason it is not done?
>
>> case TCFLSH:
>> switch (arg) {
>> case TCIFLUSH:
>> @@ -3849,6 +3866,7 @@ void tty_set_operations(struct tty_driver *driver,
>> driver->write_proc = op->write_proc;
>> driver->tiocmget = op->tiocmget;
>> driver->tiocmset = op->tiocmset;
>> + driver->get_icount = op->get_icount;
>> }
>>
>>
>> diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
>> index 3bb5d24..472a098 100644
>> --- a/drivers/serial/serial_core.c
>> +++ b/drivers/serial/serial_core.c
>> @@ -1022,10 +1022,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 *port = state->port;
>>
>> @@ -1033,19 +1033,19 @@ static int uart_get_count(struct uart_state *state,
>> memcpy(&cnow, &port->icount, sizeof(struct uart_icount));
>> spin_unlock_irq(&port->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;
>> -
>> - return copy_to_user(icnt, &icount, sizeof(icount)) ? -EFAULT : 0;
>> + 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 0;
>> }
>>
>> /*
>> @@ -1098,10 +1098,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)
>> @@ -2197,6 +2193,7 @@ static const struct tty_operations uart_ops = {
>> #endif
>> .tiocmget = uart_tiocmget,
>> .tiocmset = uart_tiocmset,
>> + .get_icount = uart_get_icount,
>> };
>>
>> /**
>> diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
>> index d7fae72..11bb54e 100644
>> --- a/drivers/usb/serial/usb-serial.c
>> +++ b/drivers/usb/serial/usb-serial.c
>> @@ -540,6 +540,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
>> @@ -1145,6 +1157,7 @@ static const struct tty_operations serial_ops = {
>> .read_proc = serial_read_proc,
>> .tiocmget = serial_tiocmget,
>> .tiocmset = serial_tiocmset,
>> + .get_icount = serial_get_icount,
>> };
>>
>> struct tty_driver *usb_serial_tty_driver;
>> diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
>> index 85c95cd..3ddb342 100644
>> --- a/include/linux/tty_driver.h
>> +++ b/include/linux/tty_driver.h
>> @@ -118,11 +118,18 @@
>> *
>> * This routine is used to send a high-priority XON/XOFF
>> * character to the device.
>> + *
>> + * 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>
>> #include <linux/list.h>
>> #include <linux/cdev.h>
>> +#include <linux/serial.h> /* for serial_state and serial_icounter_struct */
>
> Different than in original patch but looks ok.
>
>>
>> struct tty_struct;
>>
>> @@ -157,6 +164,8 @@ struct tty_operations {
>> 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);
>> };
>>
>> struct tty_driver {
>> @@ -220,6 +229,8 @@ struct tty_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);
>>
>> struct list_head tty_drivers;
>> };
>> diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
>> index 488ce12..6f44a59 100644
>> --- a/include/linux/usb/serial.h
>> +++ b/include/linux/usb/serial.h
>> @@ -243,6 +243,8 @@ struct usb_serial_driver {
>> void (*unthrottle) (struct usb_serial_port *port);
>> int (*tiocmget) (struct usb_serial_port *port, struct file *file);
>> int (*tiocmset) (struct usb_serial_port *port, struct file *file, unsigned int set, unsigned int clear);
>> + int (*get_icount)(struct tty_struct *tty,
>> + struct serial_icounter_struct *icount);
>>
>> void (*read_int_callback)(struct urb *urb);
>> void (*write_int_callback)(struct urb *urb);
>
>
And since I just stumbled over the list, Tim, you probably want to add 2010-4077
to the same patch(es) (I don't think we want a second tracking bug in that case).
-Stefan
More information about the kernel-team
mailing list