2010-07-27 09:07:23

by Ville Tervo

[permalink] [raw]
Subject: L2CAP unknown options

Hi,

I found following problem with bluetooth-next-2.6.

Kernel disconnects if remote device replies with "unknown options"
result code. I guess in that case a new config req should be sent
without unknown option. I didn't check the spec yet.

The remote device is Nokia 9300.

2010-07-27 11:44:07.364827 < ACL data: handle 11 flags 0x02 dlen 10
L2CAP(s): Info req: type 2
2010-07-27 11:44:07.365337 < HCI Command: Remote Name Request
(0x01|0x0019) plen 10
bdaddr 00:12:37:F0:F1:77 mode 2 clkoffset 0x0000
2010-07-27 11:44:07.367793 > HCI Event: Command Status (0x0f) plen 4
Remote Name Request (0x01|0x0019) status 0x00 ncmd 1
2010-07-27 11:44:07.384797 > HCI Event: Max Slots Change (0x1b) plen 3
handle 11 slots 5
2010-07-27 11:44:07.406820 > ACL data: handle 11 flags 0x02 dlen 12
L2CAP(s): Info rsp: type 2 result 1
Not supported
2010-07-27 11:44:07.406849 < ACL data: handle 11 flags 0x02 dlen 12
L2CAP(s): Connect req: psm 1 scid 0x0040
2010-07-27 11:44:07.431790 > HCI Event: Remote Name Req Complete (0x07)
plen 255
status 0x00 bdaddr 00:12:37:F0:F1:77 name 'vt-m'
2010-07-27 11:44:07.433780 > HCI Event: Number of Completed Packets
(0x13) plen 5
handle 11 packets 2
2010-07-27 11:44:07.474801 > ACL data: handle 11 flags 0x02 dlen 16
L2CAP(s): Connect rsp: dcid 0x0042 scid 0x0040 result 1 status 2
Connection pending - Authorization pending
2010-07-27 11:44:07.668818 > ACL data: handle 11 flags 0x02 dlen 16
L2CAP(s): Connect rsp: dcid 0x0042 scid 0x0040 result 0 status 0
Connection successful
2010-07-27 11:44:07.668851 < ACL data: handle 11 flags 0x02 dlen 23
L2CAP(s): Config req: dcid 0x0042 flags 0x00 clen 11
RFC 0x00 (Basic)
2010-07-27 11:44:07.669814 > ACL data: handle 11 flags 0x02 dlen 12
L2CAP(s): Config req: dcid 0x0040 flags 0x00 clen 0
2010-07-27 11:44:07.669840 < ACL data: handle 11 flags 0x02 dlen 18
L2CAP(s): Config rsp: scid 0x0042 flags 0x00 result 0 clen 4
MTU 672
2010-07-27 11:44:07.675794 > HCI Event: Number of Completed Packets
(0x13) plen 5
handle 11 packets 2
2010-07-27 11:44:07.682822 > ACL data: handle 11 flags 0x02 dlen 19
L2CAP(s): Config rsp: scid 0x0040 flags 0x00 result 3 clen 5
Failure - unknown options
RFC QoS 0f 00 42
2010-07-27 11:44:07.682843 < ACL data: handle 11 flags 0x02 dlen 12
L2CAP(s): Disconn req: dcid 0x0042 scid 0x0040
2010-07-27 11:44:07.693823 > ACL data: handle 11 flags 0x02 dlen 12
L2CAP(s): Disconn rsp: dcid 0x0042 scid 0x0040


2010-07-31 22:41:45

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported

Hi Ville,

* Ville Tervo <[email protected]> [2010-07-30 16:13:00 +0300]:

> Hi,
>
> On 07/29/2010 09:00 PM, ext Gustavo F. Padovan wrote:
> >From: Gustavo F. Padovan<[email protected]>
> >
> >If the remote side doesn't support Enhanced Retransmission Mode neither
> >Streaming Mode, we shall not send the RFC option.
> >Some devices that only supports Basic Mode do not understanding the RFC
> >option. This patch fix the regression found with that devices.
>
>
> Yes this is better. After some research i found out that quite many
> old devices are not handling properly unknown options.

Nice, we can put this upstream, do you agree?

>
> However I found another regression. And this kind of patch is needed
> also. Otherwise the info rsp code is reading feat_mask from failed
> response (and does invalid memory access).
>
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 0f34e12..1e174a4 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -3348,6 +3348,13 @@ static inline int
> l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
>
> 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);
> + return 0;
> + }
> +
> if (type == L2CAP_IT_FEAT_MASK) {
> conn->feat_mask = get_unaligned_le32(rsp->data);
>

Ack. Send a proper GIT patch then we can push this fix.


--
Gustavo F. Padovan
http://padovan.org

2010-07-30 13:13:00

by Ville Tervo

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported

Hi,

On 07/29/2010 09:00 PM, ext Gustavo F. Padovan wrote:
> From: Gustavo F. Padovan<[email protected]>
>
> If the remote side doesn't support Enhanced Retransmission Mode neither
> Streaming Mode, we shall not send the RFC option.
> Some devices that only supports Basic Mode do not understanding the RFC
> option. This patch fix the regression found with that devices.


Yes this is better. After some research i found out that quite many old
devices are not handling properly unknown options.

However I found another regression. And this kind of patch is needed
also. Otherwise the info rsp code is reading feat_mask from failed
response (and does invalid memory access).


diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 0f34e12..1e174a4 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3348,6 +3348,13 @@ static inline int l2cap_information_rsp(struct
l2cap_conn *conn, struct l2cap_cm

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);
+ return 0;
+ }
+
if (type == L2CAP_IT_FEAT_MASK) {
conn->feat_mask = get_unaligned_le32(rsp->data);



--
Ville

2010-07-29 18:04:02

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported

Hi Ville,

* Gustavo F. Padovan <[email protected]> [2010-07-29 15:00:44 -0300]:

> From: Gustavo F. Padovan <[email protected]>
>
> If the remote side doesn't support Enhanced Retransmission Mode neither
> Streaming Mode, we shall not send the RFC option.
> Some devices that only supports Basic Mode do not understanding the RFC
> option. This patch fix the regression found with that devices.
>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> net/bluetooth/l2cap.c | 15 ++++++++++++---
> 1 files changed, 12 insertions(+), 3 deletions(-)


Could you please test this patch, I'm pretty sure that it fixes the
issue with Nokia 9300.

>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 9ba1e8e..0f34e12 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -2527,6 +2527,10 @@ done:
> if (pi->imtu != L2CAP_DEFAULT_MTU)
> l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->imtu);
>
> + if (!(pi->conn->feat_mask & L2CAP_FEAT_ERTM) &&
> + !(pi->conn->feat_mask & L2CAP_FEAT_STREAMING))
> + break;
> +
> rfc.mode = L2CAP_MODE_BASIC;
> rfc.txwin_size = 0;
> rfc.max_transmit = 0;
> @@ -2534,6 +2538,8 @@ done:
> rfc.monitor_timeout = 0;
> rfc.max_pdu_size = 0;
>
> + l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
> + (unsigned long) &rfc);
> break;
>
> case L2CAP_MODE_ERTM:
> @@ -2546,6 +2552,9 @@ done:
> if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->conn->mtu - 10)
> rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);
>
> + l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
> + (unsigned long) &rfc);
> +
> if (!(pi->conn->feat_mask & L2CAP_FEAT_FCS))
> break;
>
> @@ -2566,6 +2575,9 @@ done:
> if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->conn->mtu - 10)
> rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);
>
> + l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
> + (unsigned long) &rfc);
> +
> if (!(pi->conn->feat_mask & L2CAP_FEAT_FCS))
> break;
>
> @@ -2577,9 +2589,6 @@ done:
> break;
> }
>
> - l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
> - (unsigned long) &rfc);
> -
> /* FIXME: Need actual value of the flush timeout */
> //if (flush_to != L2CAP_DEFAULT_FLUSH_TO)
> // l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, 2, pi->flush_to);
> --
> 1.7.1.1
>

--
Gustavo F. Padovan
http://padovan.org

2010-07-29 18:00:44

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported

From: Gustavo F. Padovan <[email protected]>

If the remote side doesn't support Enhanced Retransmission Mode neither
Streaming Mode, we shall not send the RFC option.
Some devices that only supports Basic Mode do not understanding the RFC
option. This patch fix the regression found with that devices.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap.c | 15 ++++++++++++---
1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 9ba1e8e..0f34e12 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2527,6 +2527,10 @@ done:
if (pi->imtu != L2CAP_DEFAULT_MTU)
l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->imtu);

+ if (!(pi->conn->feat_mask & L2CAP_FEAT_ERTM) &&
+ !(pi->conn->feat_mask & L2CAP_FEAT_STREAMING))
+ break;
+
rfc.mode = L2CAP_MODE_BASIC;
rfc.txwin_size = 0;
rfc.max_transmit = 0;
@@ -2534,6 +2538,8 @@ done:
rfc.monitor_timeout = 0;
rfc.max_pdu_size = 0;

+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+ (unsigned long) &rfc);
break;

case L2CAP_MODE_ERTM:
@@ -2546,6 +2552,9 @@ done:
if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->conn->mtu - 10)
rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);

+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+ (unsigned long) &rfc);
+
if (!(pi->conn->feat_mask & L2CAP_FEAT_FCS))
break;

@@ -2566,6 +2575,9 @@ done:
if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->conn->mtu - 10)
rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);

+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+ (unsigned long) &rfc);
+
if (!(pi->conn->feat_mask & L2CAP_FEAT_FCS))
break;

@@ -2577,9 +2589,6 @@ done:
break;
}

- l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
- (unsigned long) &rfc);
-
/* FIXME: Need actual value of the flush timeout */
//if (flush_to != L2CAP_DEFAULT_FLUSH_TO)
// l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, 2, pi->flush_to);
--
1.7.1.1


2010-07-28 12:21:43

by Ville Tervo

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Resend ConfigReq on unknown options failure

On 07/28/2010 10:16 AM, ext Gustavo F. Padovan wrote:
> Hi Ville,
>
> * Gustavo F. Padovan<[email protected]> [2010-07-28 04:11:20 -0300]:
>
>> From: Gustavo F. Padovan<[email protected]>
>>
>> If the remote device send a ConfigRsp with the unknown options failure we
>> should remove that option from the ConfigReq and send it again.
>> This patch only remove the RFC option in the case it is a unknown option.
>>
>> Signed-off-by: Gustavo F. Padovan<[email protected]>
>> ---
>> include/net/bluetooth/l2cap.h | 17 +++++++++--------
>> net/bluetooth/l2cap.c | 28 ++++++++++++++++++++++++----
>> 2 files changed, 33 insertions(+), 12 deletions(-)
>
> Can you please test thi patch? I think it fix the unknown option
> problem.

No it didn't help. I'll try to find out why it still fails.

--
Ville

2010-07-28 07:16:31

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Resend ConfigReq on unknown options failure

Hi Ville,

* Gustavo F. Padovan <[email protected]> [2010-07-28 04:11:20 -0300]:

> From: Gustavo F. Padovan <[email protected]>
>
> If the remote device send a ConfigRsp with the unknown options failure we
> should remove that option from the ConfigReq and send it again.
> This patch only remove the RFC option in the case it is a unknown option.
>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> include/net/bluetooth/l2cap.h | 17 +++++++++--------
> net/bluetooth/l2cap.c | 28 ++++++++++++++++++++++++----
> 2 files changed, 33 insertions(+), 12 deletions(-)

Can you please test thi patch? I think it fix the unknown option
problem.


--
Gustavo F. Padovan
http://padovan.org

2010-07-28 07:11:20

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH] Bluetooth: Resend ConfigReq on unknown options failure

From: Gustavo F. Padovan <[email protected]>

If the remote device send a ConfigRsp with the unknown options failure we
should remove that option from the ConfigReq and send it again.
This patch only remove the RFC option in the case it is a unknown option.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
include/net/bluetooth/l2cap.h | 17 +++++++++--------
net/bluetooth/l2cap.c | 28 ++++++++++++++++++++++++----
2 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 636724b..407af2b 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -373,14 +373,15 @@ struct l2cap_pinfo {
struct sock *prev_c;
};

-#define L2CAP_CONF_REQ_SENT 0x01
-#define L2CAP_CONF_INPUT_DONE 0x02
-#define L2CAP_CONF_OUTPUT_DONE 0x04
-#define L2CAP_CONF_MTU_DONE 0x08
-#define L2CAP_CONF_MODE_DONE 0x10
-#define L2CAP_CONF_CONNECT_PEND 0x20
-#define L2CAP_CONF_NO_FCS_RECV 0x40
-#define L2CAP_CONF_STATE2_DEVICE 0x80
+#define L2CAP_CONF_REQ_SENT 0x0001
+#define L2CAP_CONF_INPUT_DONE 0x0002
+#define L2CAP_CONF_OUTPUT_DONE 0x0004
+#define L2CAP_CONF_MTU_DONE 0x0008
+#define L2CAP_CONF_MODE_DONE 0x0010
+#define L2CAP_CONF_CONNECT_PEND 0x0020
+#define L2CAP_CONF_NO_FCS_RECV 0x0040
+#define L2CAP_CONF_STATE2_DEVICE 0x0080
+#define L2CAP_CONF_RFC_UNKNOWN 0x0200

#define L2CAP_CONF_MAX_CONF_REQ 2
#define L2CAP_CONF_MAX_CONF_RSP 2
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 9ba1e8e..dc435a3 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2577,8 +2577,9 @@ done:
break;
}

- l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
- (unsigned long) &rfc);
+ if (!(pi->conf_state & L2CAP_CONF_RFC_UNKNOWN))
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+ (unsigned long) &rfc);

/* FIXME: Need actual value of the flush timeout */
//if (flush_to != L2CAP_DEFAULT_FLUSH_TO)
@@ -2748,12 +2749,15 @@ static int l2cap_parse_conf_rsp(struct sock *sk, void *rsp, int len, void *data,
struct l2cap_pinfo *pi = l2cap_pi(sk);
struct l2cap_conf_req *req = data;
void *ptr = req->data;
- int type, olen;
+ int type, olen, unknown = 0;
unsigned long val;
struct l2cap_conf_rfc rfc;

BT_DBG("sk %p, rsp %p, len %d, req %p", sk, rsp, len, data);

+ if (*result == L2CAP_CONF_UNKNOWN)
+ unknown = 1;
+
while (len >= L2CAP_CONF_OPT_SIZE) {
len -= l2cap_get_conf_opt(&rsp, &type, &olen, &val);

@@ -2774,6 +2778,11 @@ static int l2cap_parse_conf_rsp(struct sock *sk, void *rsp, int len, void *data,
break;

case L2CAP_CONF_RFC:
+ if (unknown) {
+ pi->conf_state |= L2CAP_CONF_RFC_UNKNOWN;
+ break;
+ }
+
if (olen == sizeof(rfc))
memcpy(&rfc, (void *)val, olen);

@@ -3161,6 +3170,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
u16 scid, flags, result;
struct sock *sk;
int len = cmd->len - sizeof(*rsp);
+ char req[64];

scid = __le16_to_cpu(rsp->scid);
flags = __le16_to_cpu(rsp->flags);
@@ -3180,7 +3190,6 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr

case L2CAP_CONF_UNACCEPT:
if (l2cap_pi(sk)->num_conf_rsp <= L2CAP_CONF_MAX_CONF_RSP) {
- char req[64];

if (len > sizeof(req) - sizeof(struct l2cap_conf_req)) {
l2cap_send_disconn_req(conn, sk, ECONNRESET);
@@ -3203,6 +3212,17 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
goto done;
break;
}
+ case L2CAP_CONF_UNKNOWN:
+ len = l2cap_parse_conf_rsp(sk, rsp->data,
+ len, req, &result);
+ if (len < 0) {
+ l2cap_send_disconn_req(conn, sk, ECONNRESET);
+ goto done;
+ }
+
+ l2cap_send_cmd(conn, l2cap_get_ident(conn), L2CAP_CONF_REQ,
+ l2cap_build_conf_req(sk, req), req);
+ goto done;

default:
sk->sk_err = ECONNRESET;
--
1.7.1.1


2010-07-28 04:16:14

by Gustavo Padovan

[permalink] [raw]
Subject: Re: L2CAP unknown options

Hi Ville,

* Ville Tervo <[email protected]> [2010-07-27 12:07:23 +0300]:

> Hi,
>
> I found following problem with bluetooth-next-2.6.
>
> Kernel disconnects if remote device replies with "unknown options"
> result code. I guess in that case a new config req should be sent
> without unknown option. I didn't check the spec yet.
>
> The remote device is Nokia 9300.
>
> 2010-07-27 11:44:07.364827 < ACL data: handle 11 flags 0x02 dlen 10
> L2CAP(s): Info req: type 2
> 2010-07-27 11:44:07.365337 < HCI Command: Remote Name Request
> (0x01|0x0019) plen 10
> bdaddr 00:12:37:F0:F1:77 mode 2 clkoffset 0x0000
> 2010-07-27 11:44:07.367793 > HCI Event: Command Status (0x0f) plen 4
> Remote Name Request (0x01|0x0019) status 0x00 ncmd 1
> 2010-07-27 11:44:07.384797 > HCI Event: Max Slots Change (0x1b) plen 3
> handle 11 slots 5
> 2010-07-27 11:44:07.406820 > ACL data: handle 11 flags 0x02 dlen 12
> L2CAP(s): Info rsp: type 2 result 1
> Not supported
> 2010-07-27 11:44:07.406849 < ACL data: handle 11 flags 0x02 dlen 12
> L2CAP(s): Connect req: psm 1 scid 0x0040
> 2010-07-27 11:44:07.431790 > HCI Event: Remote Name Req Complete
> (0x07) plen 255
> status 0x00 bdaddr 00:12:37:F0:F1:77 name 'vt-m'
> 2010-07-27 11:44:07.433780 > HCI Event: Number of Completed Packets
> (0x13) plen 5
> handle 11 packets 2
> 2010-07-27 11:44:07.474801 > ACL data: handle 11 flags 0x02 dlen 16
> L2CAP(s): Connect rsp: dcid 0x0042 scid 0x0040 result 1 status 2
> Connection pending - Authorization pending
> 2010-07-27 11:44:07.668818 > ACL data: handle 11 flags 0x02 dlen 16
> L2CAP(s): Connect rsp: dcid 0x0042 scid 0x0040 result 0 status 0
> Connection successful
> 2010-07-27 11:44:07.668851 < ACL data: handle 11 flags 0x02 dlen 23
> L2CAP(s): Config req: dcid 0x0042 flags 0x00 clen 11
> RFC 0x00 (Basic)
> 2010-07-27 11:44:07.669814 > ACL data: handle 11 flags 0x02 dlen 12
> L2CAP(s): Config req: dcid 0x0040 flags 0x00 clen 0
> 2010-07-27 11:44:07.669840 < ACL data: handle 11 flags 0x02 dlen 18
> L2CAP(s): Config rsp: scid 0x0042 flags 0x00 result 0 clen 4
> MTU 672
> 2010-07-27 11:44:07.675794 > HCI Event: Number of Completed Packets
> (0x13) plen 5
> handle 11 packets 2
> 2010-07-27 11:44:07.682822 > ACL data: handle 11 flags 0x02 dlen 19
> L2CAP(s): Config rsp: scid 0x0040 flags 0x00 result 3 clen 5
> Failure - unknown options
> RFC QoS 0f 00 42

Actually the 9300 stack is broken here, it should only send the unknown
options that were in the ConfiqReq, i.e, only the RFC. But that doesn't
change the fact that we have a regression.

> 2010-07-27 11:44:07.682843 < ACL data: handle 11 flags 0x02 dlen 12
> L2CAP(s): Disconn req: dcid 0x0042 scid 0x0040
> 2010-07-27 11:44:07.693823 > ACL data: handle 11 flags 0x02 dlen 12
> L2CAP(s): Disconn rsp: dcid 0x0042 scid 0x0040

--
Gustavo F. Padovan
http://padovan.org

2010-07-28 03:32:25

by Gustavo Padovan

[permalink] [raw]
Subject: Re: L2CAP unknown options

Hi Ville,

* Ville Tervo <[email protected]> [2010-07-27 12:07:23 +0300]:

> Hi,
>
> I found following problem with bluetooth-next-2.6.
>
> Kernel disconnects if remote device replies with "unknown options"
> result code. I guess in that case a new config req should be sent
> without unknown option. I didn't check the spec yet.

It's a regression introduced by this commit
625477523b4e656fbcc5ec2a8ca7a1beb39b1caf

It adds the RFC when requesting Basic Mode, this is a PTS requirement.
A possible fix should be send a new Config Req without RFC or do not
send the RFC in the first request when the other side doesn't support
Info Req.

>
> The remote device is Nokia 9300.
>
> 2010-07-27 11:44:07.364827 < ACL data: handle 11 flags 0x02 dlen 10
> L2CAP(s): Info req: type 2
> 2010-07-27 11:44:07.365337 < HCI Command: Remote Name Request
> (0x01|0x0019) plen 10
> bdaddr 00:12:37:F0:F1:77 mode 2 clkoffset 0x0000
> 2010-07-27 11:44:07.367793 > HCI Event: Command Status (0x0f) plen 4
> Remote Name Request (0x01|0x0019) status 0x00 ncmd 1
> 2010-07-27 11:44:07.384797 > HCI Event: Max Slots Change (0x1b) plen 3
> handle 11 slots 5
> 2010-07-27 11:44:07.406820 > ACL data: handle 11 flags 0x02 dlen 12
> L2CAP(s): Info rsp: type 2 result 1
> Not supported
> 2010-07-27 11:44:07.406849 < ACL data: handle 11 flags 0x02 dlen 12
> L2CAP(s): Connect req: psm 1 scid 0x0040
> 2010-07-27 11:44:07.431790 > HCI Event: Remote Name Req Complete
> (0x07) plen 255
> status 0x00 bdaddr 00:12:37:F0:F1:77 name 'vt-m'
> 2010-07-27 11:44:07.433780 > HCI Event: Number of Completed Packets
> (0x13) plen 5
> handle 11 packets 2
> 2010-07-27 11:44:07.474801 > ACL data: handle 11 flags 0x02 dlen 16
> L2CAP(s): Connect rsp: dcid 0x0042 scid 0x0040 result 1 status 2
> Connection pending - Authorization pending
> 2010-07-27 11:44:07.668818 > ACL data: handle 11 flags 0x02 dlen 16
> L2CAP(s): Connect rsp: dcid 0x0042 scid 0x0040 result 0 status 0
> Connection successful
> 2010-07-27 11:44:07.668851 < ACL data: handle 11 flags 0x02 dlen 23
> L2CAP(s): Config req: dcid 0x0042 flags 0x00 clen 11
> RFC 0x00 (Basic)
> 2010-07-27 11:44:07.669814 > ACL data: handle 11 flags 0x02 dlen 12
> L2CAP(s): Config req: dcid 0x0040 flags 0x00 clen 0
> 2010-07-27 11:44:07.669840 < ACL data: handle 11 flags 0x02 dlen 18
> L2CAP(s): Config rsp: scid 0x0042 flags 0x00 result 0 clen 4
> MTU 672
> 2010-07-27 11:44:07.675794 > HCI Event: Number of Completed Packets
> (0x13) plen 5
> handle 11 packets 2
> 2010-07-27 11:44:07.682822 > ACL data: handle 11 flags 0x02 dlen 19
> L2CAP(s): Config rsp: scid 0x0040 flags 0x00 result 3 clen 5
> Failure - unknown options
> RFC QoS 0f 00 42
> 2010-07-27 11:44:07.682843 < ACL data: handle 11 flags 0x02 dlen 12
> L2CAP(s): Disconn req: dcid 0x0042 scid 0x0040
> 2010-07-27 11:44:07.693823 > ACL data: handle 11 flags 0x02 dlen 12
> L2CAP(s): Disconn rsp: dcid 0x0042 scid 0x0040

--
Gustavo F. Padovan
http://padovan.org

2010-08-04 14:27:24

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported

Hi Gustavo,

> If the remote side doesn't support Enhanced Retransmission Mode neither
> Streaming Mode, we shall not send the RFC option.
> Some devices that only supports Basic Mode do not understanding the RFC
> option. This patch fix the regression found with that devices.
>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> net/bluetooth/l2cap.c | 15 ++++++++++++---
> 1 files changed, 12 insertions(+), 3 deletions(-)

patch has been applied. Thanks.

Regards

Marcel



2010-08-04 06:18:12

by Ville Tervo

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported

On Wed, Aug 04, 2010 at 03:56:31AM +0200, ext Marcel Holtmann wrote:
> Hi Ville,
>
> > > On 07/29/2010 09:00 PM, ext Gustavo F. Padovan wrote:
> > > >From: Gustavo F. Padovan<[email protected]>
> > > >
> > > >If the remote side doesn't support Enhanced Retransmission Mode neither
> > > >Streaming Mode, we shall not send the RFC option.
> > > >Some devices that only supports Basic Mode do not understanding the RFC
> > > >option. This patch fix the regression found with that devices.
> > >
> > >
> > > Yes this is better. After some research i found out that quite many
> > > old devices are not handling properly unknown options.
> >
> > Nice, we can put this upstream, do you agree?
> >
> > >
> > > However I found another regression. And this kind of patch is needed
> > > also. Otherwise the info rsp code is reading feat_mask from failed
> > > response (and does invalid memory access).
> > >
> > >
> > > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > > index 0f34e12..1e174a4 100644
> > > --- a/net/bluetooth/l2cap.c
> > > +++ b/net/bluetooth/l2cap.c
> > > @@ -3348,6 +3348,13 @@ static inline int
> > > l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
> > >
> > > 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);
> > > + return 0;
> > > + }
> > > +
> > > if (type == L2CAP_IT_FEAT_MASK) {
> > > conn->feat_mask = get_unaligned_le32(rsp->data);
> > >
> >
> > Ack. Send a proper GIT patch then we can push this fix.
>
> am I getting this proper patch or not?

Yes.

--
Ville

2010-08-04 01:56:31

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported

Hi Ville,

> > On 07/29/2010 09:00 PM, ext Gustavo F. Padovan wrote:
> > >From: Gustavo F. Padovan<[email protected]>
> > >
> > >If the remote side doesn't support Enhanced Retransmission Mode neither
> > >Streaming Mode, we shall not send the RFC option.
> > >Some devices that only supports Basic Mode do not understanding the RFC
> > >option. This patch fix the regression found with that devices.
> >
> >
> > Yes this is better. After some research i found out that quite many
> > old devices are not handling properly unknown options.
>
> Nice, we can put this upstream, do you agree?
>
> >
> > However I found another regression. And this kind of patch is needed
> > also. Otherwise the info rsp code is reading feat_mask from failed
> > response (and does invalid memory access).
> >
> >
> > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > index 0f34e12..1e174a4 100644
> > --- a/net/bluetooth/l2cap.c
> > +++ b/net/bluetooth/l2cap.c
> > @@ -3348,6 +3348,13 @@ static inline int
> > l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
> >
> > 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);
> > + return 0;
> > + }
> > +
> > if (type == L2CAP_IT_FEAT_MASK) {
> > conn->feat_mask = get_unaligned_le32(rsp->data);
> >
>
> Ack. Send a proper GIT patch then we can push this fix.

am I getting this proper patch or not?

Regards

Marcel