diff options
author | David Newall <david@davidnewall.com> | 2008-02-11 21:41:30 -0800 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2008-02-12 17:54:16 -0800 |
commit | 3611f4d2a5e0f6135805f88bc5ecb63fa9ee5107 (patch) | |
tree | c8813d1ca4f750a00e9a88441cd8caa15f351ffc /drivers/bluetooth | |
parent | e848b583e03306f5f9b3a66a793c37e3649e04ca (diff) | |
download | lwn-3611f4d2a5e0f6135805f88bc5ecb63fa9ee5107.tar.gz lwn-3611f4d2a5e0f6135805f88bc5ecb63fa9ee5107.zip |
hci_ldisc: fix null pointer deref
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.
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'drivers/bluetooth')
-rw-r--r-- | drivers/bluetooth/hci_ldisc.c | 1 |
1 files changed, 1 insertions, 0 deletions
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c index e68821d074b0..7e31d5f1bc8a 100644 --- a/drivers/bluetooth/hci_ldisc.c +++ b/drivers/bluetooth/hci_ldisc.c @@ -208,6 +208,7 @@ static int hci_uart_close(struct hci_dev *hdev) return 0; hci_uart_flush(hdev); + hdev->flush = NULL; return 0; } |