Message ID | 20110217184543.D9AF4F89F8@sepang.rtg.net |
---|---|
State | Accepted |
Headers | show |
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@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@linux.intel.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> > > (cherry picked from commit d281da7ff6f70efca0553c288bb883e8605b3862) > > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> Acked-by: Stefan Bader <stefan.bader@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);
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@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@linux.intel.com> >> Signed-off-by: Greg Kroah-Hartman<gregkh@suse.de> >> >> (cherry picked from commit d281da7ff6f70efca0553c288bb883e8605b3862) >> >> Signed-off-by: Tim Gardner<tim.gardner@canonical.com> > Acked-by: Stefan Bader<stefan.bader@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@canonical.com>
applied