FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Ubuntu > Ubuntu Kernel Team

 
 
LinkBack Thread Tools
 
Old 02-17-2011, 04:34 PM
 
Default Hardy CVE: tty: Make tiocgicount a handler, CVE-2010-4076

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(-)
 
Old 02-18-2011, 12:28 PM
Stefan Bader
 
Default Hardy CVE: tty: Make tiocgicount a handler, CVE-2010-4076

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@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@linux.intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
>
> (backported from commit d281da7ff6f70efca0553c288bb883e8605b3862)
>
> Signed-off-by: Tim Gardner <tim.gardner@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);


--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 02-18-2011, 12:37 PM
Stefan Bader
 
Default Hardy CVE: tty: Make tiocgicount a handler, CVE-2010-4076

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@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@linux.intel.com>
>> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
>>
>> (backported from commit d281da7ff6f70efca0553c288bb883e8605b3862)
>>
>> Signed-off-by: Tim Gardner <tim.gardner@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

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 02-18-2011, 01:01 PM
Tim Gardner
 
Default Hardy CVE: tty: Make tiocgicount a handler, CVE-2010-4076

On 02/18/2011 06:37 AM, Stefan Bader wrote:

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@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@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman<gregkh@suse.de>

(backported from commit d281da7ff6f70efca0553c288bb883e8605b3862)

Signed-off-by: Tim Gardner<tim.gardner@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



Cool, a two for one.

I'm also reconsidering the Dapper/Hardy patches where you commented:

>>> + case TIOCGICOUNT:
>>> + return tty_tiocgicount(tty, p);

I had thought that the fall-thru cleanup code was different in the older
kernels, but upon closer inspection it appears to be sufficient, e.g.,
it allows the IOCTL code to fall through and call the underlying driver
directly. I'll resubmit.


rtg
--
Tim Gardner tim.gardner@canonical.com

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 02-18-2011, 01:19 PM
Tim Gardner
 
Default Hardy CVE: tty: Make tiocgicount a handler, CVE-2010-4076

This is a multi-part message in MIME format.
Added CVE-2010-4077 and restored 'case TIOCGICOUNT' to its original code.

--
Tim Gardner tim.gardner@canonical.com
 
Old 02-18-2011, 01:23 PM
Stefan Bader
 
Default Hardy CVE: tty: Make tiocgicount a handler, CVE-2010-4076

On 02/18/2011 03:19 PM, Tim Gardner wrote:
> Added CVE-2010-4077 and restored 'case TIOCGICOUNT' to its original code.
>
ACK (includes all later releases with just the cve number added. I am too lazy)

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 02-22-2011, 03:27 PM
Brad Figg
 
Default Hardy CVE: tty: Make tiocgicount a handler, CVE-2010-4076

On 02/18/2011 06:19 AM, Tim Gardner wrote:

Added CVE-2010-4077 and restored 'case TIOCGICOUNT' to its original code.



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

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

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 02-22-2011, 04:14 PM
Tim Gardner
 
Default Hardy CVE: tty: Make tiocgicount a handler, CVE-2010-4076

applied

--
Tim Gardner tim.gardner@canonical.com

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 

Thread Tools




All times are GMT. The time now is 11:11 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org