Re: [PATCH] work around for l2cap NULL dereference in l2cap_conn_start
From: Andrei Emeltchenko <hidden>
Date: 2011-03-24 15:37:33
Also in:
lkml
Hi Gustavo, On Mon, Feb 28, 2011 at 7:03 AM, David Fries [off-list ref] wrote:
On Sun, Feb 27, 2011 at 04:15:45PM -0300, Gustavo F. Padovan wrote:quoted
I pushed the following patch to bluetooth-2.6 tree. It should fix the problem by avoiding connections to be accepted before a L2CAP info response comes:Is git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-2.6.git the bluetooth-2.6 tree you mentioned? I don't see your patch there. As a side note, the inline patch in your e-mail has the tabs replaced by spaces, once I changed them, it applied cleanly. I first reverted to the base N900 kernel-power-2.6.28 46 (none of my changes or debugging), it crashed as expected. I then applied your patch 743400e0, and it still crashed.
the same for me. Your patch with adding BT_CONNECT2 check seems have no effect since sk_state == BT_CONNECT2 for my case. I've posted series of patches which fixes the issue but I believe it is better to keep check for parent. Search my patches by "[RFCv1 0/3] Set of patches fixing kernel crash" Regards, Andrei
quoted hunk ↗ jump to hunk
I added back the l2cap_conn_start parent check and some debugging in af_bluetooth.c dmesg debug output and patches follow. I haven't at all looked into the bluetooth protocol, but what connect sequence difference does it make if I power on the bluetooth headset and press play on the headset before it automatically pairs with the N900, vs power on bluetooth headset, wait for it to pair then press play? I ask this partly because I'm curiouse, but mostly how I trigger the bug. This is with pulse audio running, but no applications playing audio or responding to a play event from the headset. [ 443.424560] bt_accept_dequeue, parent cd54ba00 newsock c81f0180, defer_setup && BT_CONNECT2 [ 443.427368] avoided crash in l2cap_conn_start sk c6d3f600 result 1 status 2 [ 443.518463] bt_accept_dequeue, parent cdee9c00 newsock c81f0000, BT_CONNECTED [ 443.729736] bt_accept_dequeue, parent cd54be00 newsock c81f0000, BT_CONNECTED [ 443.813537] bt_accept_dequeue, parent cd54b600 newsock c81f0180, defer_setup && BT_CONNECT2 From 5bc80fafac43b6698e271f1246cb24e596bf2ef1 Mon Sep 17 00:00:00 2001 From: David Fries <redacted> Date: Sun, 6 Feb 2011 14:34:49 -0600 Subject: [PATCH 1/2] work around for l2cap NULL dereference in l2cap_conn_start print sk --- net/bluetooth/l2cap.c | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-)diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index fda7741..ff05f51 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c@@ -400,7 +400,16 @@ static void l2cap_conn_start(struct l2cap_conn *conn)struct sock *parent = bt_sk(sk)->parent; rsp.result = cpu_to_le16(L2CAP_CR_PEND); rsp.status = cpu_to_le16(L2CAP_CS_AUTHOR_PEND); - parent->sk_data_ready(parent, 0); + if(!parent) { + printk(KERN_DEBUG "avoided " + "crash in %s sk %p " + "result %d status %d\n", + __func__, sk, + rsp.result, rsp.status); + } else { + parent->sk_data_ready(parent, + 0); + } } else { sk->sk_state = BT_CONFIG; -- 1.7.2.3 From 42b9a6ef68a1cd0ef025b826afcfb0ef23342fe5 Mon Sep 17 00:00:00 2001 From: David Fries <redacted> Date: Sun, 27 Feb 2011 21:50:14 -0600 Subject: [PATCH 2/2] af_bluetooth.c debug --- net/bluetooth/af_bluetooth.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c index 8e910f1..57cd360 100644 --- a/net/bluetooth/af_bluetooth.c +++ b/net/bluetooth/af_bluetooth.c@@ -211,6 +211,18 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)continue; } + if (bt_sk(parent)->defer_setup && sk->sk_state == BT_CONNECT2) + printk("%s, parent %p newsock %p, " + "defer_setup && BT_CONNECT2\n", __func__, + parent, newsock); + if (sk->sk_state == BT_CONNECTED) + printk("%s, parent %p newsock %p, " + "BT_CONNECTED\n", __func__, + parent, newsock); + if (!newsock) + printk("%s, parent %p newsock %p, " + "!newsock\n", __func__, + parent, newsock); if ((bt_sk(parent)->defer_setup && sk->sk_state == BT_CONNECT2) || sk->sk_state == BT_CONNECTED || !newsock) { bt_accept_unlink(sk); -- 1.7.2.3quoted
commit 743400e01a33779f93b79c84a1b0d1a2d27338c8 Author: Gustavo F. Padovan [off-list ref] Date: Sun Feb 27 16:05:07 2011 -0300 Bluetooth: Don't accept l2cap connection before info_rsp When using defer_setup accepting a connection before receive the L2CAP Info Response for the connection lead us to a crash in l2cap_conn_start(. Reported-by: David Fries [off-list ref] Reported-by: Liang Bao [off-list ref] Signed-off-by: Gustavo F. Padovan [off-list ref]diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c index c4cf3f5..a8ca42b 100644 --- a/net/bluetooth/af_bluetooth.c +++ b/net/bluetooth/af_bluetooth.c@@ -211,8 +211,8 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)continue; } - if (sk->sk_state == BT_CONNECTED || !newsock || - bt_sk(parent)->defer_setup) { + if ((bt_sk(parent)->defer_setup && sk->sk_state == BT_CONNECT2) + || sk->sk_state == BT_CONNECTED || !newsock) { bt_accept_unlink(sk); if (newsock) sock_graft(sk, newsock); -- Gustavo F. Padovan http://profusion.mobi -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html-- David Fries [off-list ref] http://fries.net/~david/ (PGP encryption key available) -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html