* [PATCH] work around for l2cap NULL dereference in l2cap_conn_start
[not found] ` <AANLkTi==Sh2yc2JjjY5t-NKxQexKPgPwNyWo4SMju92k@mail.gmail.com>
@ 2011-02-21 4:36 ` David Fries
2011-02-21 6:41 ` Liang Bao
2011-03-02 1:31 ` Andrei Warkentin
0 siblings, 2 replies; 10+ messages in thread
From: David Fries @ 2011-02-21 4:36 UTC (permalink / raw)
To: Andrei Warkentin; +Cc: Gustavo F. Padovan, linux-bluetooth, linux-kernel
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 <david@fries.net>
---
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
> <padovan@profusion.mobi> wrote:
> > Hi David,
> >
> > * David Fries <david@fries.net> [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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] work around for l2cap NULL dereference in l2cap_conn_start
2011-02-21 4:36 ` [PATCH] work around for l2cap NULL dereference in l2cap_conn_start David Fries
@ 2011-02-21 6:41 ` Liang Bao
2011-02-27 19:15 ` Gustavo F. Padovan
2011-03-02 1:31 ` Andrei Warkentin
1 sibling, 1 reply; 10+ messages in thread
From: Liang Bao @ 2011-02-21 6:41 UTC (permalink / raw)
To: David Fries
Cc: Andrei Warkentin, Gustavo F. Padovan, linux-bluetooth, linux-kernel
Hi, David, Andrew et al.
2011/2/21 David Fries <david@fries.net>:
> 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 <david@fries.net>
> ---
> 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
>> <padovan@profusion.mobi> wrote:
>> > Hi David,
>> >
>> > * David Fries <david@fries.net> [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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] work around for l2cap NULL dereference in l2cap_conn_start
2011-02-21 6:41 ` Liang Bao
@ 2011-02-27 19:15 ` Gustavo F. Padovan
2011-02-28 5:03 ` David Fries
0 siblings, 1 reply; 10+ messages in thread
From: Gustavo F. Padovan @ 2011-02-27 19:15 UTC (permalink / raw)
To: Liang Bao; +Cc: David Fries, Andrei Warkentin, linux-bluetooth, linux-kernel
Hi David and Liang,
* Liang Bao <tim.bao@gmail.com> [2011-02-21 14:41:29 +0800]:
> Hi, David, Andrew et al.
>
> 2011/2/21 David Fries <david@fries.net>:
> > 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 <david@fries.net>
> > ---
> > 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
> >> <padovan@profusion.mobi> wrote:
> >> > Hi David,
> >> >
> >> > * David Fries <david@fries.net> [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 <padovan@profusion.mobi>
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 <david@fries.net>
Reported-by: Liang Bao <tim.bao@gmail.com>
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] work around for l2cap NULL dereference in l2cap_conn_start
2011-02-27 19:15 ` Gustavo F. Padovan
@ 2011-02-28 5:03 ` David Fries
2011-02-28 17:30 ` Gustavo F. Padovan
2011-03-24 15:37 ` Andrei Emeltchenko
0 siblings, 2 replies; 10+ messages in thread
From: David Fries @ 2011-02-28 5:03 UTC (permalink / raw)
To: Liang Bao, Andrei Warkentin, linux-bluetooth, linux-kernel, Feng Tang
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 <david@fries.net>
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 <david@fries.net>
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 <padovan@profusion.mobi>
> 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 <david@fries.net>
> Reported-by: Liang Bao <tim.bao@gmail.com>
> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
>
> 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 <david@fries.net>
http://fries.net/~david/ (PGP encryption key available)
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] work around for l2cap NULL dereference in l2cap_conn_start
2011-02-28 5:03 ` David Fries
@ 2011-02-28 17:30 ` Gustavo F. Padovan
2011-03-02 6:19 ` David Fries
2011-03-24 15:37 ` Andrei Emeltchenko
1 sibling, 1 reply; 10+ messages in thread
From: Gustavo F. Padovan @ 2011-02-28 17:30 UTC (permalink / raw)
To: David Fries
Cc: Liang Bao, Andrei Warkentin, linux-bluetooth, linux-kernel, Feng Tang
Hi David,
* David Fries <david@fries.net> [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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] work around for l2cap NULL dereference in l2cap_conn_start
2011-02-21 4:36 ` [PATCH] work around for l2cap NULL dereference in l2cap_conn_start David Fries
2011-02-21 6:41 ` Liang Bao
@ 2011-03-02 1:31 ` Andrei Warkentin
1 sibling, 0 replies; 10+ messages in thread
From: Andrei Warkentin @ 2011-03-02 1:31 UTC (permalink / raw)
To: David Fries; +Cc: Gustavo F. Padovan, linux-bluetooth, linux-kernel
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 <david@fries.net> 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 <david@fries.net>
> ---
> 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
>> <padovan@profusion.mobi> wrote:
>> > Hi David,
>> >
>> > * David Fries <david@fries.net> [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
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] work around for l2cap NULL dereference in l2cap_conn_start
2011-02-28 17:30 ` Gustavo F. Padovan
@ 2011-03-02 6:19 ` David Fries
2011-03-05 2:12 ` Gustavo F. Padovan
0 siblings, 1 reply; 10+ messages in thread
From: David Fries @ 2011-03-02 6:19 UTC (permalink / raw)
To: Liang Bao, Andrei Warkentin, linux-bluetooth, linux-kernel, Feng Tang
On Mon, Feb 28, 2011 at 02:30:22PM -0300, Gustavo F. Padovan wrote:
> Hi David,
>
> * David Fries <david@fries.net> [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 <david@fries.net>
http://fries.net/~david/ (PGP encryption key available)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] work around for l2cap NULL dereference in l2cap_conn_start
2011-03-02 6:19 ` David Fries
@ 2011-03-05 2:12 ` Gustavo F. Padovan
2011-03-22 2:30 ` David Fries
0 siblings, 1 reply; 10+ messages in thread
From: Gustavo F. Padovan @ 2011-03-05 2:12 UTC (permalink / raw)
To: David Fries
Cc: Liang Bao, Andrei Warkentin, linux-bluetooth, linux-kernel, Feng Tang
Hi David,
* David Fries <david@fries.net> [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 <david@fries.net> [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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] work around for l2cap NULL dereference in l2cap_conn_start
2011-03-05 2:12 ` Gustavo F. Padovan
@ 2011-03-22 2:30 ` David Fries
0 siblings, 0 replies; 10+ messages in thread
From: David Fries @ 2011-03-22 2:30 UTC (permalink / raw)
To: Liang Bao, Andrei Warkentin, linux-bluetooth, linux-kernel, Feng Tang
On Fri, Mar 04, 2011 at 11:12:57PM -0300, Gustavo F. Padovan wrote:
> Hi David,
>
> * David Fries <david@fries.net> [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 <david@fries.net> [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 <david@fries.net>
http://fries.net/~david/ (PGP encryption key available)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] work around for l2cap NULL dereference in l2cap_conn_start
2011-02-28 5:03 ` David Fries
2011-02-28 17:30 ` Gustavo F. Padovan
@ 2011-03-24 15:37 ` Andrei Emeltchenko
1 sibling, 0 replies; 10+ messages in thread
From: Andrei Emeltchenko @ 2011-03-24 15:37 UTC (permalink / raw)
To: David Fries
Cc: Liang Bao, Andrei Warkentin, linux-bluetooth, linux-kernel, Feng Tang
Hi Gustavo,
On Mon, Feb 28, 2011 at 7:03 AM, David Fries <david@fries.net> 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 <david@fries.net>
> 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 <david@fries.net>
> 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 <padovan@profusion.mobi>
>> 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 <david@fries.net>
>> Reported-by: Liang Bao <tim.bao@gmail.com>
>> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
>>
>> 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 <david@fries.net>
> 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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-03-24 15:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20110211035309.GA22204@spacedout.fries.net>
[not found] ` <20110214145649.GE2597@joana>
[not found] ` <AANLkTi==Sh2yc2JjjY5t-NKxQexKPgPwNyWo4SMju92k@mail.gmail.com>
2011-02-21 4:36 ` [PATCH] work around for l2cap NULL dereference in l2cap_conn_start David Fries
2011-02-21 6:41 ` Liang Bao
2011-02-27 19:15 ` Gustavo F. Padovan
2011-02-28 5:03 ` David Fries
2011-02-28 17:30 ` Gustavo F. Padovan
2011-03-02 6:19 ` David Fries
2011-03-05 2:12 ` Gustavo F. Padovan
2011-03-22 2:30 ` David Fries
2011-03-24 15:37 ` Andrei Emeltchenko
2011-03-02 1:31 ` Andrei Warkentin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).