linux-kernel - Re: 2.6.31-rc5 regression: Oops when USB Serial disconnected while in use

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.44L0.0908251921390.3665-100000@netrider.rowland.org>
Date:	Tue, 25 Aug 2009 19:42:45 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Bruno Prémont <bonbons@...ux-vserver.org>
cc:	Alan Cox <alan@...rguk.ukuu.org.uk>, Greg KH <greg@...ah.com>,
	Kernel development list <linux-kernel@...r.kernel.org>,
	USB list <linux-usb@...r.kernel.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>
Subject: Re: 2.6.31-rc5 regression: Oops when USB Serial disconnected while
 in use

On Tue, 25 Aug 2009, Bruno [UTF-8] PrÃ©mont wrote:

> On Mon, 24 August 2009 Alan Stern <stern@...land.harvard.edu> wrote:
> > Apparently the tty isn't getting removed from the driver the way it
> > should when minicom closes it the first time.  And no wonder -- I just
> > realized that my new serial_shutdown() routine doesn't call
> > tty_shutdown(tty).  Try inserting such a call right before
> > tty->driver_data gets set to NULL.
> 
> Done that, it brings us to n_tty_set_termios() this time:

> [  200.248102] BUG: unable to handle kernel NULL pointer dereference at 0000000c
> [  200.248257] IP: [<c115ff36>] n_tty_set_termios+0x1a6/0x410

> My actions:
>   plug-in ftdi
>   run minicom
>   exit minicom
>   run minicom -> bang
> 
> Bang at:
> /tmp/linux-2.6.31-rc6-gregkh/drivers/char/n_tty.c:1456
> 
> 1449:	if (canon_change) {
> 1450:		memset(&tty->read_flags, 0, sizeof tty->read_flags);
> 1451:		tty->canon_head = tty->read_tail;
> 1452:		tty->canon_data = 0;
> 1453:		tty->erasing = 0;
> 1454:	}
> 1455:
> 1456:	if (canon_change && !L_ICANON(tty) && tty->read_cnt)
> 1457:		wake_up_interruptible(&tty->read_wait);
> 
> Not sure which part of the if() it is that explodes, I would guess
> it's !L_ICANON(tty):
> 
> 	#define ICANON      0000002
> 	#define _L_FLAG(tty, f) ((tty)->termios->c_lflag & (f))
> 	#define L_ICANON(tty)  _L_FLAG((tty), ICANON)
> =>
> 	if (canon_change && !((tty)->termios->c_lflag & (ICANON)) && tty->read_cnt)
> =>
> 	tty->termios is probably NULL

Unquestionably that is the problem.

It looks like something is wrong in serial_install().  We want to call 
tty_init_termios(tty) always, regardless of whether 
tty->driver->termios[idx] is NULL.  But after the call it definitely 
won't be NULL, so the test has to be made before the call is done.  
Something like this:

	struct ktermios *t = tty->driver->termios[idx];

	/* perform the standard setup */
	retval = tty_init_termios(tty);
	if (retval)
		return retval;

	/* If the specialized termios setup has yet to be done */
	if (!t) {
		...

Want to try that?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/