[patch 6/7] hci_ldisc: fix null pointer deref

[patch 6/7] hci_ldisc: fix null pointer deref

[akpm]

From: David Newall <david@davidnewall.com>

Arjan:

  With the help of kerneloops.org I've spotted a nice little interaction
  between the TTY layer and the bluetooth code, however the tty layer is not
  something I'm all too familiar with so I rather ask than brute-force fix the
  code incorrectly.

  The raw details are at:
  http://www.kerneloops.org/search.php?search=uart_flush_buffer

  What happens is that, on closing the bluetooth tty, the tty layer goes
  into the release_dev() function, which first does a bunch of stuff, then
  sets the file->private_data to NULL, does some more stuff and then calls the
  ldisc close function.  Which in this case, is hci_uart_tty_close().

  Now, hci_uart_tty_close() calls hci_uart_close() which clears some
  internal bit, and then calls hci_uart_flush()...  which calls back to the
  tty layers' uart_flush_buffer() function.  (in drivers/bluetooth/hci_tty.c
  around line 194) Which then WARN_ON()'s because that's not allowed/supposed
  to be called this late in the shutdown of the port....

  Should the bluetooth driver even call this flush function at all??

David:

  This seems to be what happens: Hci_uart_close() flushes using
  hci_uart_flush().  Subsequently, in hci_dev_do_close(), (one step in
  hci_unregister_dev()), hci_uart_flush() is called again.  The comment in
  uart_flush_buffer(), relating to the WARN_ON(), indicates you can't flush
  after the port is closed; which sounds reasonable.  I think hci_uart_close()
  should set hdev->flush to NULL before returning.  Hci_dev_do_close() does
  check for this.  The code path is rather involved and I'm not entirely clear
  of all steps, but I think that's what should be done.

akpm:

  No idea.  trollmerge.

Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/bluetooth/hci_ldisc.c |    1 +
 1 file changed, 1 insertion(+)

diff -puN drivers/bluetooth/hci_ldisc.c~hci_ldisc-fix-null-pointer-deref drivers/bluetooth/hci_ldisc.c
--- a/drivers/bluetooth/hci_ldisc.c~hci_ldisc-fix-null-pointer-deref
+++ a/drivers/bluetooth/hci_ldisc.c
@@ -208,6 +208,7 @@ static int hci_uart_close(struct hci_dev
 		return 0;
 
 	hci_uart_flush(hdev);
+	hdev->flush = NULL;
 	return 0;
 }
 
_

Re: [patch 6/7] hci_ldisc: fix null pointer deref

[David Miller]

From: akpm@linux-foundation.org
Date: Mon, 04 Feb 2008 23:48:18 -0800

> akpm:
> 
>   No idea.  trollmerge.
> 
> Cc: Arjan van de Ven <arjan@linux.intel.com>
> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Cc: Marcel Holtmann <marcel@holtmann.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

I'll let Marcel and/or Alan take a look at this one.

Re: [patch 6/7] hci_ldisc: fix null pointer deref

[Marcel Holtmann]

Hi David,

> > akpm:
> > 
> >   No idea.  trollmerge.
> > 
> > Cc: Arjan van de Ven <arjan@linux.intel.com>
> > Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> > Cc: Marcel Holtmann <marcel@holtmann.org>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> 
> I'll let Marcel and/or Alan take a look at this one.

we might have to re-work the whole hci_uart driver at some point, but
for now I think this fix is feasible.

Alan, if you think it is wrong please step up. I am not an expert when
it comes to TTY line disciplines.

Regards

Marcel

Re: [patch 6/7] hci_ldisc: fix null pointer deref

[David Miller]

From: Marcel Holtmann <marcel@holtmann.org>
Date: Tue, 05 Feb 2008 17:51:07 +0100

> Hi David,
> 
> > > akpm:
> > > 
> > >   No idea.  trollmerge.
> > > 
> > > Cc: Arjan van de Ven <arjan@linux.intel.com>
> > > Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> > > Cc: Marcel Holtmann <marcel@holtmann.org>
> > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > 
> > I'll let Marcel and/or Alan take a look at this one.
> 
> we might have to re-work the whole hci_uart driver at some point, but
> for now I think this fix is feasible.

Ok, I've applied this patch.