| Subject | Re: [ 174/218] Bluetooth: Fix use-after-free bug in SMP | From | Ben Hutchings <> | Date | Sat, 29 Sep 2012 18:29:15 +0200 |
| |
On Fri, 2012-09-28 at 13:16 -0700, Greg Kroah-Hartman wrote: > 3.4-stable review patch. If anyone has any objections, please let me know. > > ------------------ > > From: Andre Guedes <andre.guedes@openbossa.org> > > commit 61a0cfb008f57ecf7eb28ee762952fb42dc15d15 upstream. > > If SMP fails, we should always cancel security_timer delayed work. > Otherwise, security_timer function may run after l2cap_conn object > has been freed. [...]
This particular bug doesn't appear to exist in earlier kernel versions, but it led me to find some related bugs in teardown that do.
I'm attaching two patches: - bluetooth-fix-l2cap_conn_del-locking.patch seems to be needed for both 3.0 and 3.2. It's very different from the upstream changes, and is compile-tested only. - bluetooth-fix-deadlock-and-crash-when-smp-pairing-times-out.patch (commit d06cc416f517a25713dedd9e2a9ccf4f3086c09a upstream) seems to be needed for 3.2 only.
What do you think?
Ben.
-- Ben Hutchings Usenet is essentially a HUGE group of people passing notes in class. - Rachel Kadel, `A Quick Guide to Newsgroup Etiquette' From: Ben Hutchings <ben@decadent.org.uk> Date: Sat, 29 Sep 2012 17:53:28 +0200 Subject: Bluetooth: Fix l2cap_conn_del() locking
This is covered by commit 3df91ea20e744344100b10ae69a17211fcf5b207 ('Bluetooth: Revert to mutexes from RCU list') upstream.
l2cap_conn_del() can race with various other functions that walk the channel list; in particular l2cap_conn_start() which is called from l2cap_info_timeout(). l2cap_chan_del() itself takes the chan_lock for writing, so instead of locking here we must move all channels onto a temporary list.
Signed-off-by: Ben Hutchings <ben@decadent.org.uk> --- --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -968,6 +968,7 @@ static void l2cap_info_timeout(unsigned static void l2cap_conn_del(struct hci_conn *hcon, int err) { struct l2cap_conn *conn = hcon->l2cap_data; + struct list_head chan_l; struct l2cap_chan *chan, *l; struct sock *sk; @@ -978,8 +979,13 @@ static void l2cap_conn_del(struct hci_co kfree_skb(conn->rx_skb); + INIT_LIST_HEAD(&chan_l); + write_lock_bh(&conn->chan_lock); + list_splice_init(&conn->chan_l, &chan_l); + write_unlock_bh(&conn->chan_lock); + /* Kill channels */ - list_for_each_entry_safe(chan, l, &conn->chan_l, list) { + list_for_each_entry_safe(chan, l, &chan_l, list) { sk = chan->sk; bh_lock_sock(sk); l2cap_chan_del(chan, err);From: Johan Hedberg <johan.hedberg@intel.com> Date: Wed, 6 Jun 2012 18:44:11 +0800 Subject: Bluetooth: Fix deadlock and crash when SMP pairing times out
commit d06cc416f517a25713dedd9e2a9ccf4f3086c09a upstream.
The l2cap_conn_del function tries to cancel_sync the security timer, but when it's called from the timeout function itself a deadlock occurs. Subsequently the "hcon->l2cap_data = NULL" that's supposed to protect multiple calls to l2cap_conn_del never gets cleared and when the connection finally drops we double free's etc which will crash the kernel.
This patch fixes the issue by using the HCI_CONN_LE_SMP_PEND for protecting against this. The same flag is also used for the same purpose in other places in the SMP code.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> [bwh: Backported to 3.2: pending flags are in hci_conn::pend] Signed-off-by: Ben Hutchings <ben@decadent.org.uk> --- net/bluetooth/l2cap_core.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
--- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -1009,7 +1009,12 @@ static void security_timeout(unsigned lo { struct l2cap_conn *conn = (void *) arg; - l2cap_conn_del(conn->hcon, ETIMEDOUT); + BT_DBG("conn %p", conn); + + if (test_and_clear_bit(HCI_CONN_LE_SMP_PEND, &conn->hcon->pend)) { + smp_chan_destroy(conn); + l2cap_conn_del(conn->hcon, ETIMEDOUT); + } } static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status)[unhandled content-type:application/pgp-signature] |