public inbox for linux-serial@vger.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurentp@cse-semaphore.com>
To: Russell King <rmk@arm.linux.org.uk>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>, linux-serial@vger.kernel.org
Subject: Re: [PATCH] Fix oops in the 8250 serial driver when accessing a removed device
Date: Fri, 25 Jul 2008 13:40:13 +0200	[thread overview]
Message-ID: <200807251340.16716.laurentp@cse-semaphore.com> (raw)
In-Reply-To: <20080725095128.GC21452@flint.arm.linux.org.uk>

[-- Attachment #1: Type: text/plain, Size: 3669 bytes --]

On Friday 25 July 2008, Russell King wrote:
> On Tue, Jul 22, 2008 at 06:31:50PM +0200, Laurent Pinchart wrote:
> > Hi Russell,
> > 
> > On Tuesday 22 July 2008, Alan Cox wrote:
> > > > When a plug-and-play 8250 device is detected, the driver reuses one of
> > > > the dummy ports. If that device is later removed, the port goes pack
> > > > to the dummy ports pool. The capabilities field is not reset, which
> > > > causes a oops when trying to access the port.
> > > 
> > > Some day we need to allocate new dummy ports so there is always one
> > > free. 
> > > 
> > > > This patch resets the capabilities field when a 8250 device is
> > > > unregistered. 
> > > 
> > > Looks good to me. Would appreciate Russell's view though.
> > 
> > Could you please review the patch (available at
> > http://marc.info/?l=linux-serial&m=121674107306179&w=2) ? 
> > I'd like it to go into 2.6.27 if possible.
> 
> Is there a link to the oops as well?

I'm afraid not. I had no copy of the oops so I tried to regenerate it. It turned out the bug has been fixed in the meantime.

My patch can probably be dropped. I'm not sure if it's worth bisecting.

On the other hand, I ran into another issue caused by the WARN_ON in uart_flush_buffer@drivers/serial/serial_core.c.

------------[ cut here ]------------
Badness at c0141288 [verbose debug info unavailable]
NIP: c0141288 LR: c013e1f0 CTR: 00000000
REGS: c39f3cc0 TRAP: 0700   Not tainted  (2.6.26-00040-g7f43ec9-dirty)
MSR: 00029032 <EE,ME,IR,DR>  CR: 24242422  XER: 00000000
TASK = c398d900[375] 'hciattach' THREAD: c39f2000
GPR00: 00000000 c39f3d70 c398d900 c39a1c00 00000000 00000001 c39f3d90 00000000
GPR08: c386b000 c023f5b4 00009032 c39a1c00 84044422 1001d9bc 03ff7000 c3fb9000
GPR16: 00000000 00000000 00000000 00000000 c39a1d28 c39a1d20 00000128 00000120
GPR24: c02d0000 00000000 00000000 00000001 00000000 c39f2000 c39a1c00 c39a1c00
NIP [c0141288] uart_flush_buffer+0x2c/0xbc
LR [c013e1f0] tty_driver_flush_buffer+0x34/0x44
Call Trace:
[c39f3d70] [c01370e8] tty_ldisc_flush+0x18/0x5c (unreliable)
[c39f3d80] [c013e1f0] tty_driver_flush_buffer+0x34/0x44
[c39f3d90] [c0183480] hci_uart_flush+0x34/0xb0
[c39f3da0] [c018354c] hci_uart_close+0x50/0x70
[c39f3db0] [c0183704] hci_uart_tty_close+0x38/0xa4
[c39f3dc0] [c0138cdc] release_dev+0x640/0x680
[c39f3e70] [c0138d3c] tty_release+0x20/0x3c
[c39f3e90] [c00724f4] __fput+0x190/0x1b0
[c39f3eb0] [c0070358] filp_close+0x54/0xac
[c39f3ed0] [c001efa8] put_files_struct+0xa0/0xec
[c39f3ef0] [c001f860] do_exit+0x164/0x688
[c39f3f30] [c001fe60] do_group_exit+0xa0/0xdc
[c39f3f40] [c00103d0] ret_from_syscall+0x0/0x38
--- Exception: c01 at 0xff0e91c
    LR = 0xffb4b2c
Instruction dump:
4bffffe4 7c0802a6 9421fff0 93e1000c 7c7f1b78 90010014 81030144 2f880000
419e0010 80080010 2f800000 409e001c <0fe00000> 80010014 83e1000c 38210010

The hciattach process opens the serial port and sets the N_HCI line discipline. At some point the device is disconnected, and uart_remove_one_port is called by serial8250_unregister_port.

A comment in uart_remove_one_port states, right after calling tty_unregister_device,

"All users of this port should now be disconnected from this driver, and the port shut down.  We should be the only thread fiddling with this port from now on."

This seems not to be true, at least with the N_HCI line disclipline. I've tested the N_TTY line discipline by disconnecting the device while performing a 'cat /dev/ttyS0' and no crash occurs.

-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussee de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

  reply	other threads:[~2008-07-25 11:40 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-22 15:37 [PATCH] Fix oops in the 8250 serial driver when accessing a removed device Laurent Pinchart
2008-07-22 15:54 ` Alan Cox
2008-07-22 16:31   ` Laurent Pinchart
2008-07-25  9:51     ` Russell King
2008-07-25 11:40       ` Laurent Pinchart [this message]
2008-07-25 11:57         ` Russell King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200807251340.16716.laurentp@cse-semaphore.com \
    --to=laurentp@cse-semaphore.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=linux-serial@vger.kernel.org \
    --cc=rmk@arm.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox