Hardy CVE: tty: Make tiocgicount a handler, CVE-2010-4076
Stefan Bader
stefan.bader at canonical.com
Fri Feb 18 13:28:56 UTC 2011
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);
More information about the kernel-team
mailing list