2009-09-05 21:09:48

by Jason Wessel

[permalink] [raw]
Subject: [PATCH 0/2] usb console: 2.6.31 regression fixes

There are two regressions in the 2.6.31 for the usb serial console.

1) A critical crash from user space

* Boot with the kernel argument console=ttyUSB0,9600
* Run: stty -a < /dev/ttyUSB0
* Immediately you get an oops warning, which later leads to a hard
kernel crash

WARNING: at drivers/usb/serial/usb-serial.c:414 serial_write_room+0x75/0x80()
Hardware name:
Modules linked in:
Pid: 6, comm: events/0 Not tainted 2.6.31-rc8-00038-g37d0892-dirty #737
Call Trace:
[<c0416fd5>] ? serial_write_room+0x75/0x80
[<c022acbc>] warn_slowpath_common+0x7c/0xa0
[<c0416fd5>] ? serial_write_room+0x75/0x80
[<c022acf5>] warn_slowpath_null+0x15/0x20
[<c0416fd5>] serial_write_room+0x75/0x80
[<c02245fb>] ? default_wake_function+0xb/0x10
[<c021e7a8>] ? __wake_up_common+0x48/0x70
[<c038fcd8>] tty_write_room+0x18/0x20
[<c038d572>] process_echoes+0x42/0x2c0
[<c038d29b>] ? echo_char_raw+0x3b/0x70
[<c038f3a1>] n_tty_receive_buf+0x1141/0x1250
[<c022056f>] ? hrtick_update+0x3f/0x50
[<c02277c1>] ? dequeue_task_fair+0xa1/0xb0
[<c02017d4>] ? __switch_to+0x24/0x1b0
[<c050ec50>] ? _spin_unlock_irq+0x10/0x30
[<c0223479>] ? finish_task_switch+0x99/0xc0
[<c050cae2>] ? schedule+0x242/0x7c0
[<c050e86c>] ? _spin_lock_irqsave+0x1c/0x40
[<c050ec21>] ? _spin_unlock_irqrestore+0x11/0x30
[<c0390eab>] ? tty_ldisc_try+0x3b/0x50
[<c0391c90>] flush_to_ldisc+0xe0/0x1a0
[<c038e260>] ? n_tty_receive_buf+0x0/0x1250
[<c023aa3f>] worker_thread+0x10f/0x1f0
[<c0391bb0>] ? flush_to_ldisc+0x0/0x1a0
[<c023db60>] ? autoremove_wake_function+0x0/0x50
[<c023a930>] ? worker_thread+0x0/0x1f0
[<c023da5a>] kthread+0x7a/0x90
[<c023d9e0>] ? kthread+0x0/0x90
[<c020385f>] kernel_thread_helper+0x7/0x38
---[ end trace 5552e7699618b972 ]---


2) Original console baud is not passed to first post boot open of the
tty -> hw.

This fix is not critical, and should be reviewed to see if there is
any other preferred way the usb maintainer would prefer to see this
operate.

The first open of the usb serial HW has the termios initialized to
9600 baud, and this will override what ever was setup via the original
console initialization. The solution is to save the console baud rate
and re-use it later on the first open.


---

The following changes since commit 37d0892c5a94e208cf863e3b7bac014edee4346d:
Ian Kent (1):
autofs4 - fix missed case when changing to use struct path

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/linux-2.6-kgdb.git for_linus

Jason Wessel (2):
usb console: fix kernel crash on stty -a < /dev/ttyUSB0
usb console: pass initial console baud on to first tty open

drivers/usb/serial/console.c | 1 +
drivers/usb/serial/usb-serial.c | 19 ++++++++++++-------
include/linux/usb/serial.h | 1 +
3 files changed, 14 insertions(+), 7 deletions(-)


2009-09-05 21:09:59

by Jason Wessel

[permalink] [raw]
Subject: [PATCH 1/2] usb console: fix kernel crash on stty -a < /dev/ttyUSB0

* Boot with the kernel argument console=ttyUSB0,9600
* Run: stty -a < /dev/ttyUSB0
* Immediately you get an oops warning, which later leads to a hard
kernel crash

The commit 335f8514f200e63d689113d29cb7253a5c282967 created the
original regression and commit
6e4061210150d1d6d388c5fba05f6b49a306a27e only fixed part of the
problem.

Only protect the serial->type->open() from getting executed when the
device is used as a console.

The wider scope of the console protection added in
6e4061210150d1d6d388c5fba05f6b49a306a27e causes the logic in
serial_open() to fall through and zero out the port->port.count with
the stty sys call. Once the port.count is zeroed the HW will get
closed while other drivers still have call backs to a non-initialized
device which crashes the kernel.

Signed-off-by: Jason Wessel <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Alan Stern <[email protected]>

---
drivers/usb/serial/usb-serial.c | 17 ++++++++++-------
1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 99188c9..7ca4ced 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -223,8 +223,7 @@ static int serial_open (struct tty_struct *tty, struct file *filp)
tty->driver_data = port;
tty_port_tty_set(&port->port, tty);

- /* If the console is attached, the device is already open */
- if (port->port.count == 1 && !port->console) {
+ if (port->port.count == 1) {
first = 1;
/* lock this module before we call it
* this may fail, which means we must bail out,
@@ -242,11 +241,15 @@ static int serial_open (struct tty_struct *tty, struct file *filp)
if (retval)
goto bailout_module_put;

- /* only call the device specific open if this
- * is the first time the port is opened */
- retval = serial->type->open(tty, port, filp);
- if (retval)
- goto bailout_interface_put;
+ /* only call the device specific open if this is the
+ * first time the port is opened and it is not a
+ * console port where the HW has already been
+ * initialized */
+ if (!port->console) {
+ retval = serial->type->open(tty, port, filp);
+ if (retval)
+ goto bailout_interface_put;
+ }
mutex_unlock(&serial->disc_mutex);
set_bit(ASYNCB_INITIALIZED, &port->port.flags);
}
--
1.6.3.1.9.g95405b

2009-09-05 21:09:58

by Jason Wessel

[permalink] [raw]
Subject: [PATCH 2/2] usb console: pass initial console baud on to first tty open

The first open of the usb serial HW has the termios initialized to the
default of 9600 baud, and this will override what ever was setup via
the original console initialization.

The solution is to save the console baud rate and re-use it later on
the first open, so as to allow the use of baud rates other than 9600
for the usb serial console.

Signed-off-by: Jason Wessel <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Alan Stern <[email protected]>

---
drivers/usb/serial/console.c | 1 +
drivers/usb/serial/usb-serial.c | 4 +++-
include/linux/usb/serial.h | 1 +
3 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/serial/console.c b/drivers/usb/serial/console.c
index 0e4f2e4..35ba616 100644
--- a/drivers/usb/serial/console.c
+++ b/drivers/usb/serial/console.c
@@ -175,6 +175,7 @@ static int usb_console_setup(struct console *co, char *options)
/* The console is special in terms of closing the device so
* indicate this port is now acting as a system console. */
port->console = 1;
+ port->console_init_baud = baud;
retval = 0;

out:
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 7ca4ced..7ec5e0c 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -245,7 +245,9 @@ static int serial_open (struct tty_struct *tty, struct file *filp)
* first time the port is opened and it is not a
* console port where the HW has already been
* initialized */
- if (!port->console) {
+ if (port->console) {
+ tty_encode_baud_rate(tty, port->console_init_baud, port->console_init_baud);
+ } else {
retval = serial->type->open(tty, port, filp);
if (retval)
goto bailout_interface_put;
diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index 0ec50ba..d5e586f 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -107,6 +107,7 @@ struct usb_serial_port {
char throttled;
char throttle_req;
char console;
+ int console_init_baud;
unsigned long sysrq; /* sysrq timeout */
struct device dev;
enum port_dev_state dev_state;
--
1.6.3.1.9.g95405b

2009-09-15 22:57:07

by Greg KH

[permalink] [raw]
Subject: Re: USB: console: fix kernel crash on stty -a < /dev/ttyUSB0

On Sat, Sep 05, 2009 at 04:08:37PM -0500, Jason Wessel wrote:
>
> * Boot with the kernel argument console=ttyUSB0,9600
> * Run: stty -a < /dev/ttyUSB0
> * Immediately you get an oops warning, which later leads to a hard
> kernel crash

<snip>

I've had to drop this and your second patch due to conflicts with Alan
Stern's fixes. Care to respin them on the next linux-next tree if they
are still needed? I think Alan's fixes might have solved some of these
problems.

thanks,

greg k-h