[RFCv1 0/3] Set of patches fixing kernel crash

[RFCv1 0/3] Set of patches fixing kernel crash

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

Kernel crash can happen in l2cap_conn_start as it was already
reported in:
http://www.spinics.net/lists/linux-bluetooth/msg11026.html
and:
http://www.spinics.net/lists/linux-bluetooth/msg10962.html

In my case crash happens when two devices connect to each other
at the same time and unbound L2CAP Information Requests mess up
req/rsp sequence. Patch makes sure that we do not process
out of the sequence packet. info_timer clean up hanging connections.

Andrei Emeltchenko (3):
  Bluetooth: check info_rsp ident and states
  Bluetooth: remove duplicated code
  Bluetooth: delete hanging L2CAP channel

 net/bluetooth/l2cap_core.c |   18 +++++++++++-------
 net/bluetooth/l2cap_sock.c |    5 +++--
 2 files changed, 14 insertions(+), 9 deletions(-)

[RFCv1 1/3] Bluetooth: check info_rsp ident and states

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

Information requests/responses are unbound to L2CAP channel. Patch
fixes issue arising when two devices connects at the same time to
each other. This way we do not process out of the context messages.
We are safe dropping info_rsp since info_timer is left running.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
 net/bluetooth/l2cap_core.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index ca27f3a..fd58b8f 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2462,6 +2462,16 @@ static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
 
 	BT_DBG("type 0x%4.4x result 0x%2.2x", type, result);
 
+	if (cmd->ident != conn->info_ident) {
+		BT_DBG("Collision receiving info response");
+		return 0;
+	}
+
+	if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE) {
+		BT_DBG("Information request already done");
+		return 0;
+	}
+
 	del_timer(&conn->info_timer);
 
 	if (result != L2CAP_IR_SUCCESS) {
-- 
1.7.1

Re: [RFCv1 1/3] Bluetooth: check info_rsp ident and states

[Gustavo F. Padovan]

Hi Andrei,

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2011-03-24 17:16:06 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> Information requests/responses are unbound to L2CAP channel. Patch
> fixes issue arising when two devices connects at the same time to
> each other. This way we do not process out of the context messages.
> We are safe dropping info_rsp since info_timer is left running.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  net/bluetooth/l2cap_core.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index ca27f3a..fd58b8f 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -2462,6 +2462,16 @@ static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
>  
>  	BT_DBG("type 0x%4.4x result 0x%2.2x", type, result);
>  
> +	if (cmd->ident != conn->info_ident) {
> +		BT_DBG("Collision receiving info response");
> +		return 0;
> +	}
> +
> +	if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE) {
> +		BT_DBG("Information request already done");
> +		return 0;
> +	}

This patch is good, but without the BT_DBG(). They are not needed.

-- 
Gustavo F. Padovan
http://profusion.mobi

[RFCv1 2/3] Bluetooth: remove duplicated code

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

info_timer takes care about removed code

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
 net/bluetooth/l2cap_core.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index fd58b8f..4255f00 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2472,16 +2472,10 @@ static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
 		return 0;
 	}
 
-	del_timer(&conn->info_timer);
-
-	if (result != L2CAP_IR_SUCCESS) {
-		conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
-		conn->info_ident = 0;
-
-		l2cap_conn_start(conn);
-
+	if (result != L2CAP_IR_SUCCESS)
 		return 0;
-	}
+
+	del_timer(&conn->info_timer);
 
 	if (type == L2CAP_IT_FEAT_MASK) {
 		conn->feat_mask = get_unaligned_le32(rsp->data);
-- 
1.7.1

Re: [RFCv1 2/3] Bluetooth: remove duplicated code

[Gustavo F. Padovan]

Hi Andrei,

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2011-03-24 17:16:07 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> info_timer takes care about removed code
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  net/bluetooth/l2cap_core.c |   12 +++---------
>  1 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index fd58b8f..4255f00 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -2472,16 +2472,10 @@ static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
>  		return 0;
>  	}
>  
> -	del_timer(&conn->info_timer);
> -
> -	if (result != L2CAP_IR_SUCCESS) {
> -		conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
> -		conn->info_ident = 0;
> -
> -		l2cap_conn_start(conn);

Are you sure? Does remove this code won't create a delay to call
l2cap_conn_start()?

-- 
Gustavo F. Padovan
http://profusion.mobi

Re: [RFCv1 2/3] Bluetooth: remove duplicated code

[Andrei Emeltchenko]

Hi Gustavo,

On Thu, Mar 24, 2011 at 8:57 PM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
> Hi Andrei,
>
> * Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2011-03-24 17:16:07 +0200]:
>
>> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>>
>> info_timer takes care about removed code
>>
>> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>> ---
>>  net/bluetooth/l2cap_core.c |   12 +++---------
>>  1 files changed, 3 insertions(+), 9 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index fd58b8f..4255f00 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -2472,16 +2472,10 @@ static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
>>               return 0;
>>       }
>>
>> -     del_timer(&conn->info_timer);
>> -
>> -     if (result != L2CAP_IR_SUCCESS) {
>> -             conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
>> -             conn->info_ident = 0;
>> -
>> -             l2cap_conn_start(conn);
>
> Are you sure? Does remove this code won't create a delay to call
> l2cap_conn_start()?

Yes, but the difference is only with delay. I believe that this way it
will be cleaner.

Do you think that it could cause problems?

Regards,
Andrei

[RFCv1 3/3] Bluetooth: delete hanging L2CAP channel

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

Sometimes L2CAP connection remains hanging. Make sure that
L2CAP channel is deleted.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
 net/bluetooth/l2cap_sock.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index fc85e7a..f77308e 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -923,8 +923,9 @@ void __l2cap_sock_close(struct sock *sk, int reason)
 			rsp.status = cpu_to_le16(L2CAP_CS_NO_INFO);
 			l2cap_send_cmd(conn, l2cap_pi(sk)->ident,
 					L2CAP_CONN_RSP, sizeof(rsp), &rsp);
-		} else
-			l2cap_chan_del(sk, reason);
+		}
+
+		l2cap_chan_del(sk, reason);
 		break;
 
 	case BT_CONNECT:
-- 
1.7.1

Re: [RFCv1 3/3] Bluetooth: delete hanging L2CAP channel

[Gustavo F. Padovan]

Hi Andrei,

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2011-03-24 17:16:08 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> Sometimes L2CAP connection remains hanging. Make sure that
> L2CAP channel is deleted.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  net/bluetooth/l2cap_sock.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)

This one is applied. Thanks.

-- 
Gustavo F. Padovan
http://profusion.mobi

Re: [RFCv1 3/3] Bluetooth: delete hanging L2CAP channel

[Andrei Emeltchenko]

Hi Gustavo,

On Thu, Mar 24, 2011 at 9:12 PM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
> Hi Andrei,
>
> * Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2011-03-24 17:16:08 +0200]:
>
>> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>>
>> Sometimes L2CAP connection remains hanging. Make sure that
>> L2CAP channel is deleted.
>>
>> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>> ---
>>  net/bluetooth/l2cap_sock.c |    5 +++--
>>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> This one is applied. Thanks.

Where it is applied? Cannot find it here:

http://git.kernel.org/?p=linux/kernel/git/padovan/bluetooth-next-2.6.git;a=shortlog;h=refs/heads/master


Regards,
Andrei

Re: [RFCv1 3/3] Bluetooth: delete hanging L2CAP channel

[Gustavo F. Padovan]

Hi Andrei,

* Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com> [2011-03-25 10:57:52 +0200]:

> Hi Gustavo,
> 
> On Thu, Mar 24, 2011 at 9:12 PM, Gustavo F. Padovan
> <padovan@profusion.mobi> wrote:
> > Hi Andrei,
> >
> > * Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2011-03-24 17:16:08 +0200]:
> >
> >> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> >>
> >> Sometimes L2CAP connection remains hanging. Make sure that
> >> L2CAP channel is deleted.
> >>
> >> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> >> ---
> >>  net/bluetooth/l2cap_sock.c |    5 +++--
> >>  1 files changed, 3 insertions(+), 2 deletions(-)
> >
> > This one is applied. Thanks.
> 
> Where it is applied? Cannot find it here:

I applied it to bluetooth-2.6 as it is a bug fix.

-- 
Gustavo F. Padovan
http://profusion.mobi