Here's a patch to avoid a very repeatable crash in the N900. If I
take a Motorola S305 bluetooth headset that was previously paried with
the N900, turn it on, and press the play button before the headphones
automatically pair with the cell phone, the N900 will crash (and
reboot) in pairing. If I wait until after they have paired there
isn't any problem. The patch is against the kernel-power
2.6.28-maemo46 by Thomas Tanner, the stock Nokia PR1.2 oops looked
the same, I just haven't gone back to that kernel.
All of the original oops dumps had the same backtrace functions,
(see original.txt for the full oops)
Backtrace:
l2cap_load+0x7e8/0xa94 [l2cap]
l2cap_sock_destruct+0x124/0x1484 [l2cap]
l2cap_recv_acldata+0x0/0x1f0 [l2cap]
hci_rx_task+0x0/0x298 [bluetooth]
tasklet_action+0x0/0xc0
__do_softirq+0x0/0xf4
irq_exit+0x0/0xa8
I first cherry picked the kernel patch by Andrei Emeltchenko
e501d0553a7580fcc6654d7f58a5f061d31d00af
"Bluetooth: Check L2CAP pending status before sending connect request"
It was still crashing, but the backtrace changed,
(see pending_protect.txt for the full oops)
Backtrace:
l2cap_conn_start+0x0/0x2ac [l2cap]
l2cap_recv_frame+0x0/0x12bc [l2cap]
l2cap_recv_acldata+0x0/0x1f0 [l2cap]
hci_rx_task+0x0/0x298 [bluetooth]
tasklet_action+0x0/0xc0
__do_softirq+0x0/0xf4
irq_exit+0x0/0xa8
Then I added the following patch to protected against a NULL parent,
and it no longer crashes. I'm not sure what to think about the above
"pending status" patch, I added a check and the
__l2cap_no_conn_pending function added by that patch never sees a
pending connection, but it did change the backtrace, so I'm confused.
But then again the original trace lists l2cap_load, which only
contains a return statement, and shouldn't be prone to making NULL
pointer dereferences anyawy.
I also attached two syslogs, one with out touching the headset while
it powered on and paired,
cut_no_press.txt
and another with pressing the button before it paired. This one
prints the debug message instead of the crash that would have
happened.
avoided crash in l2cap_conn_start parent 00000000 result 1 status 2
cut_with_press.txt
There's also a patch on android.git.kernel.org
"Bluetooth: Hack: Don't dereference null pointer."
http://android.git.kernel.org/?p=kernel/common.git;a=commit;h=5a2ad658a5efea775a00b162c1062ce33e8e3aaa
and a bug report on bugs.maemo.org
https://bugs.maemo.org/show_bug.cgi?id=10510
Subject: [PATCH] work around for l2cap NULL dereference in l2cap_conn_start
just skip the dereference, 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 ed83c1f..c5b4fbf 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -408,7 +408,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
Hi David,
* David Fries <[email protected]> [2011-02-27 23:03:40 -0600]:
> On Sun, Feb 27, 2011 at 04:15:45PM -0300, Gustavo F. Padovan wrote:
> > 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. I added back the
> l2cap_conn_start parent check and some debugging in af_bluetooth.c
> dmesg debug output and patches follow.
I want to see a test with this patch and a recent kernel. We added many fixes
to stack in the last two years. Can you test this scenario?
--
Gustavo F. Padovan
http://profusion.mobi
On Sun, Feb 27, 2011 at 04:15:45PM -0300, Gustavo F. Padovan wrote:
> 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. 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 <[email protected]>
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 <[email protected]>
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.3
> commit 743400e01a33779f93b79c84a1b0d1a2d27338c8
> Author: Gustavo F. Padovan <[email protected]>
> 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 <[email protected]>
> Reported-by: Liang Bao <[email protected]>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
>
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
David Fries <[email protected]>
http://fries.net/~david/ (PGP encryption key available)
Hi David and Liang,
* Liang Bao <[email protected]> [2011-02-21 14:41:29 +0800]:
> Hi, David, Andrew et al.
>
> 2011/2/21 David Fries <[email protected]>:
> > bt_sk(sk)->parent can be NULL in l2cap_conn_start in state BT_CONNECT2
> > at least when a headset device pairs and the play button was pressed
> > right before pairing.
> >
> > Signed-off-by: David Fries <[email protected]>
> > ---
> > I removed the printk, can this be merged to the bluetooth next tree?
> >
> > On Mon, Feb 14, 2011 at 03:40:46PM -0600, Andrei Warkentin wrote:
> >> FWIW still need it in 2.6.36.
> >
> > Andrei, I'm curious, what's your hardware hardware and bluetooth
> > device that's trigginer the crash?
> I once submitted an issue observed with Android+Motorola S305 stereo
> headset. It's still open in launchpad:
> https://bugs.launchpad.net/ubuntu/+source/bluez/+bug/513642. Hope this
> helps. Thanks.
> >
> >> On Mon, Feb 14, 2011 at 8:56 AM, Gustavo F. Padovan
> >> <[email protected]> wrote:
> >> > Hi David,
> >> >
> >> > * David Fries <[email protected]> [2011-02-10 21:53:09 -0600]:
> >> >
> >> >> Here's a patch to avoid a very repeatable crash in the N900. ?If I
> >> >> take a Motorola S305 bluetooth headset that was previously paried with
> >> >> the N900, turn it on, and press the play button before the headphones
> >> >> automatically pair with the cell phone, the N900 will crash (and
> >> >> reboot) in pairing. ?If I wait until after they have paired there
> >> >> isn't any problem. ?The patch is against the kernel-power
> >> >> 2.6.28-maemo46 by Thomas Tanner, the stock Nokia PR1.2 oops looked
> >> >> the same, I just haven't gone back to that kernel.
> >> >
> >> > This is a very old kernel. You need to check this issue against
> >> > bluetooth-next-2.6.
> >
> > ?net/bluetooth/l2cap.c | ? ?3 ++-
> > ?1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > index ed83c1f..a7aa4d9 100644
> > --- a/net/bluetooth/l2cap.c
> > +++ b/net/bluetooth/l2cap.c
> > @@ -408,7 +408,8 @@ 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)
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? parent->sk_data_ready(parent,0);
> >
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?} else {
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sk->sk_state = BT_CONFIG;
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:
commit 743400e01a33779f93b79c84a1b0d1a2d27338c8
Author: Gustavo F. Padovan <[email protected]>
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 <[email protected]>
Reported-by: Liang Bao <[email protected]>
Signed-off-by: Gustavo F. Padovan <[email protected]>
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
Hi, David, Andrew et al.
2011/2/21 David Fries <[email protected]>:
> bt_sk(sk)->parent can be NULL in l2cap_conn_start in state BT_CONNECT2
> at least when a headset device pairs and the play button was pressed
> right before pairing.
>
> Signed-off-by: David Fries <[email protected]>
> ---
> I removed the printk, can this be merged to the bluetooth next tree?
>
> On Mon, Feb 14, 2011 at 03:40:46PM -0600, Andrei Warkentin wrote:
>> FWIW still need it in 2.6.36.
>
> Andrei, I'm curious, what's your hardware hardware and bluetooth
> device that's trigginer the crash?
I once submitted an issue observed with Android+Motorola S305 stereo
headset. It's still open in launchpad:
https://bugs.launchpad.net/ubuntu/+source/bluez/+bug/513642. Hope this
helps. Thanks.
>
>> On Mon, Feb 14, 2011 at 8:56 AM, Gustavo F. Padovan
>> <[email protected]> wrote:
>> > Hi David,
>> >
>> > * David Fries <[email protected]> [2011-02-10 21:53:09 -0600]:
>> >
>> >> Here's a patch to avoid a very repeatable crash in the N900. ?If I
>> >> take a Motorola S305 bluetooth headset that was previously paried with
>> >> the N900, turn it on, and press the play button before the headphones
>> >> automatically pair with the cell phone, the N900 will crash (and
>> >> reboot) in pairing. ?If I wait until after they have paired there
>> >> isn't any problem. ?The patch is against the kernel-power
>> >> 2.6.28-maemo46 by Thomas Tanner, the stock Nokia PR1.2 oops looked
>> >> the same, I just haven't gone back to that kernel.
>> >
>> > This is a very old kernel. You need to check this issue against
>> > bluetooth-next-2.6.
>
> ?net/bluetooth/l2cap.c | ? ?3 ++-
> ?1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index ed83c1f..a7aa4d9 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -408,7 +408,8 @@ 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)
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? parent->sk_data_ready(parent,0);
>
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?} else {
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sk->sk_state = BT_CONFIG;
> --
> 1.7.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
bt_sk(sk)->parent can be NULL in l2cap_conn_start in state BT_CONNECT2
at least when a headset device pairs and the play button was pressed
right before pairing.
Signed-off-by: David Fries <[email protected]>
---
I removed the printk, can this be merged to the bluetooth next tree?
On Mon, Feb 14, 2011 at 03:40:46PM -0600, Andrei Warkentin wrote:
> FWIW still need it in 2.6.36.
Andrei, I'm curious, what's your hardware hardware and bluetooth
device that's trigginer the crash?
> On Mon, Feb 14, 2011 at 8:56 AM, Gustavo F. Padovan
> <[email protected]> wrote:
> > Hi David,
> >
> > * David Fries <[email protected]> [2011-02-10 21:53:09 -0600]:
> >
> >> Here's a patch to avoid a very repeatable crash in the N900. If I
> >> take a Motorola S305 bluetooth headset that was previously paried with
> >> the N900, turn it on, and press the play button before the headphones
> >> automatically pair with the cell phone, the N900 will crash (and
> >> reboot) in pairing. If I wait until after they have paired there
> >> isn't any problem. The patch is against the kernel-power
> >> 2.6.28-maemo46 by Thomas Tanner, the stock Nokia PR1.2 oops looked
> >> the same, I just haven't gone back to that kernel.
> >
> > This is a very old kernel. You need to check this issue against
> > bluetooth-next-2.6.
net/bluetooth/l2cap.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index ed83c1f..a7aa4d9 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -408,7 +408,8 @@ 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)
+ parent->sk_data_ready(parent,0);
} else {
sk->sk_state = BT_CONFIG;
--
1.7.2.3
FWIW still need it in 2.6.36.
On Mon, Feb 14, 2011 at 8:56 AM, Gustavo F. Padovan
<[email protected]> wrote:
> Hi David,
>
> * David Fries <[email protected]> [2011-02-10 21:53:09 -0600]:
>
>> Here's a patch to avoid a very repeatable crash in the N900. ?If I
>> take a Motorola S305 bluetooth headset that was previously paried with
>> the N900, turn it on, and press the play button before the headphones
>> automatically pair with the cell phone, the N900 will crash (and
>> reboot) in pairing. ?If I wait until after they have paired there
>> isn't any problem. ?The patch is against the kernel-power
>> 2.6.28-maemo46 by Thomas Tanner, the stock Nokia PR1.2 oops looked
>> the same, I just haven't gone back to that kernel.
>
> This is a very old kernel. You need to check this issue against
> bluetooth-next-2.6.
>
> --
> Gustavo F. Padovan
> http://profusion.mobi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
Hi David,
* David Fries <[email protected]> [2011-02-10 21:53:09 -0600]:
> Here's a patch to avoid a very repeatable crash in the N900. If I
> take a Motorola S305 bluetooth headset that was previously paried with
> the N900, turn it on, and press the play button before the headphones
> automatically pair with the cell phone, the N900 will crash (and
> reboot) in pairing. If I wait until after they have paired there
> isn't any problem. The patch is against the kernel-power
> 2.6.28-maemo46 by Thomas Tanner, the stock Nokia PR1.2 oops looked
> the same, I just haven't gone back to that kernel.
This is a very old kernel. You need to check this issue against
bluetooth-next-2.6.
--
Gustavo F. Padovan
http://profusion.mobi
Hi Gustavo,
On Mon, Feb 28, 2011 at 7:03 AM, David Fries <[email protected]> wrote:
> On Sun, Feb 27, 2011 at 04:15:45PM -0300, Gustavo F. Padovan wrote:
>> 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
> 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 <[email protected]>
> 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 <[email protected]>
> 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.3
>
>
>> commit 743400e01a33779f93b79c84a1b0d1a2d27338c8
>> Author: Gustavo F. Padovan <[email protected]>
>> 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 <[email protected]>
>> ? ? Reported-by: Liang Bao <[email protected]>
>> ? ? Signed-off-by: Gustavo F. Padovan <[email protected]>
>>
>> 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 [email protected]
>> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
> --
> David Fries <[email protected]>
> 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 [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
On Fri, Mar 04, 2011 at 11:12:57PM -0300, Gustavo F. Padovan wrote:
> Hi David,
>
> * David Fries <[email protected]> [2011-03-02 00:19:10 -0600]:
>
> > On Mon, Feb 28, 2011 at 02:30:22PM -0300, Gustavo F. Padovan wrote:
> > > Hi David,
> > >
> > > * David Fries <[email protected]> [2011-02-27 23:03:40 -0600]:
> > >
> > > > On Sun, Feb 27, 2011 at 04:15:45PM -0300, Gustavo F. Padovan wrote:
> > > > > 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. I added back the
> > > > l2cap_conn_start parent check and some debugging in af_bluetooth.c
> > > > dmesg debug output and patches follow.
> > >
> > > I want to see a test with this patch and a recent kernel. We added many fixes
> > > to stack in the last two years. Can you test this scenario?
> >
> > I'm sorry, but apparently not, at least this post says 2.6.37 isn't
> > going to happen for the N900 and Maemo.
> > http://forums.internettablettalk.com/showthread.php?t=70082
> >
> > I tried 2.6.37-n900 from
> > git://gitorious.org/nokia-n900-kernel/nokia-n900-kernel.git anyway,
> > but the display visibly degrades like it isn't being updated and
> > doesn't apparently get any further. I don't have anyway to debug it
> > further.
>
> I think you can test this in a desktop machine.
I've not been able to reproduce the bug on my desktop, and not for a
lack of trying.
2.6.28, l2cap_conn_start doesn't dereference parent (so it wouldn't
crash there anyway) N900 must have some backported patches.
2.6.30 first kernel with that code
2.6.30, 2.6.37+, 2.6.38-rc7+, with a debug patch to print
the sk and parent in l2cap_conn_start, only executes the BT_CONNECT2
path in l2cap_conn_start maybe only one in five or less times and I
have yet to see it (on the desktop) have a NULL parent.
This is with the following USB Bluetooth dongle,
Cambridge Silicon Radio, Ltd Bluetooth Dongle (HCI mode)
Looks like I'm not going to be any more help verifying it is or isn't
fixed with a newer bluetooth stack. Here's a post from Liang Bao.
On Tue, Mar 15, 2011 at 10:42:07PM +0800, Liang Bao wrote:
> Hi,
>
> Sorry for get back so late. I am really crazy busy with my project. I tested
> with 2.6.35-27 kernel + ubuntu 10.10 just now and seems the issue is really
> gone. Hcidump attached for your reference. It's more than one year so it
> might need some more time to figure out the difference of logs but as said,
> I am really hard to find out that time. Wondering if you would like to
> compare this with the one I attached into the mailing list a year ago.
--
David Fries <[email protected]>
http://fries.net/~david/ (PGP encryption key available)
Hi David,
* David Fries <[email protected]> [2011-03-02 00:19:10 -0600]:
> On Mon, Feb 28, 2011 at 02:30:22PM -0300, Gustavo F. Padovan wrote:
> > Hi David,
> >
> > * David Fries <[email protected]> [2011-02-27 23:03:40 -0600]:
> >
> > > On Sun, Feb 27, 2011 at 04:15:45PM -0300, Gustavo F. Padovan wrote:
> > > > 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. I added back the
> > > l2cap_conn_start parent check and some debugging in af_bluetooth.c
> > > dmesg debug output and patches follow.
> >
> > I want to see a test with this patch and a recent kernel. We added many fixes
> > to stack in the last two years. Can you test this scenario?
>
> I'm sorry, but apparently not, at least this post says 2.6.37 isn't
> going to happen for the N900 and Maemo.
> http://forums.internettablettalk.com/showthread.php?t=70082
>
> I tried 2.6.37-n900 from
> git://gitorious.org/nokia-n900-kernel/nokia-n900-kernel.git anyway,
> but the display visibly degrades like it isn't being updated and
> doesn't apparently get any further. I don't have anyway to debug it
> further.
I think you can test this in a desktop machine.
--
Gustavo F. Padovan
http://profusion.mobi
On Mon, Feb 28, 2011 at 02:30:22PM -0300, Gustavo F. Padovan wrote:
> Hi David,
>
> * David Fries <[email protected]> [2011-02-27 23:03:40 -0600]:
>
> > On Sun, Feb 27, 2011 at 04:15:45PM -0300, Gustavo F. Padovan wrote:
> > > 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. I added back the
> > l2cap_conn_start parent check and some debugging in af_bluetooth.c
> > dmesg debug output and patches follow.
>
> I want to see a test with this patch and a recent kernel. We added many fixes
> to stack in the last two years. Can you test this scenario?
I'm sorry, but apparently not, at least this post says 2.6.37 isn't
going to happen for the N900 and Maemo.
http://forums.internettablettalk.com/showthread.php?t=70082
I tried 2.6.37-n900 from
git://gitorious.org/nokia-n900-kernel/nokia-n900-kernel.git anyway,
but the display visibly degrades like it isn't being updated and
doesn't apparently get any further. I don't have anyway to debug it
further.
--
David Fries <[email protected]>
http://fries.net/~david/ (PGP encryption key available)
Hi all,
I don't have an S305 headset at the moment to play with this, but, our
tree (2.6.36) has
a fix like this for this issue.
if (bt_sk(sk)->defer_setup) {
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)
+ parent->sk_data_ready(parent, 0);
} else {
sk->sk_state = BT_CONFIG;
rsp.result = cpu_to_le16(L2CAP_CR_SUCCESS);
The comment is:
Bluetooth: Hack: Don't dereference null pointer.
This avoids the S305 panic during incoming connection.
S305 sends PSM 25 L2CAP connection request before the L2CAP info response.
When we receive that info response we crash on null pointer here.
Sorry for the wait,
A
On Sun, Feb 20, 2011 at 10:36 PM, David Fries <[email protected]> wrote:
> bt_sk(sk)->parent can be NULL in l2cap_conn_start in state BT_CONNECT2
> at least when a headset device pairs and the play button was pressed
> right before pairing.
>
> Signed-off-by: David Fries <[email protected]>
> ---
> I removed the printk, can this be merged to the bluetooth next tree?
>
> On Mon, Feb 14, 2011 at 03:40:46PM -0600, Andrei Warkentin wrote:
>> FWIW still need it in 2.6.36.
>
> Andrei, I'm curious, what's your hardware hardware and bluetooth
> device that's trigginer the crash?
>
>> On Mon, Feb 14, 2011 at 8:56 AM, Gustavo F. Padovan
>> <[email protected]> wrote:
>> > Hi David,
>> >
>> > * David Fries <[email protected]> [2011-02-10 21:53:09 -0600]:
>> >
>> >> Here's a patch to avoid a very repeatable crash in the N900. ?If I
>> >> take a Motorola S305 bluetooth headset that was previously paried with
>> >> the N900, turn it on, and press the play button before the headphones
>> >> automatically pair with the cell phone, the N900 will crash (and
>> >> reboot) in pairing. ?If I wait until after they have paired there
>> >> isn't any problem. ?The patch is against the kernel-power
>> >> 2.6.28-maemo46 by Thomas Tanner, the stock Nokia PR1.2 oops looked
>> >> the same, I just haven't gone back to that kernel.
>> >
>> > This is a very old kernel. You need to check this issue against
>> > bluetooth-next-2.6.
>
> ?net/bluetooth/l2cap.c | ? ?3 ++-
> ?1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index ed83c1f..a7aa4d9 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -408,7 +408,8 @@ 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)
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? parent->sk_data_ready(parent,0);
>
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?} else {
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sk->sk_state = BT_CONFIG;
> --
> 1.7.2.3
>
>