L2CAP unknown options

L2CAP unknown options

[Ville Tervo]

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

Re: L2CAP unknown options

[Gustavo F. Padovan]

Hi Ville,

* Ville Tervo <ville.tervo@nokia.com> [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

[PATCH] Bluetooth: Resend ConfigReq on unknown options failure

[Gustavo F. Padovan]

From: Gustavo F. Padovan <padovan@profusion.mobi>

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 <padovan@profusion.mobi>
---
 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

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

[Gustavo F. Padovan]

Hi Ville,

* Gustavo F. Padovan <gustavo@padovan.org> [2010-07-28 04:11:20 -0300]:

> From: Gustavo F. Padovan <padovan@profusion.mobi>
> 
> 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 <padovan@profusion.mobi>
> ---
>  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

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

[Ville Tervo]

On 07/28/2010 10:16 AM, ext Gustavo F. Padovan wrote:
> Hi Ville,
>
> * Gustavo F. Padovan<gustavo@padovan.org>  [2010-07-28 04:11:20 -0300]:
>
>> From: Gustavo F. Padovan<padovan@profusion.mobi>
>>
>> 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<padovan@profusion.mobi>
>> ---
>>   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

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

[Gustavo F. Padovan]

From: Gustavo F. Padovan <padovan@profusion.mobi>

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 <padovan@profusion.mobi>
---
 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

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

[Gustavo F. Padovan]

Hi Ville,

* Gustavo F. Padovan <gustavo@padovan.org> [2010-07-29 15:00:44 -0300]:

> From: Gustavo F. Padovan <padovan@profusion.mobi>
> 
> 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 <padovan@profusion.mobi>
> ---
>  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

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

[Ville Tervo]

Hi,

On 07/29/2010 09:00 PM, ext Gustavo F. Padovan wrote:
> From: Gustavo F. Padovan<padovan@profusion.mobi>
>
> 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

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

[Gustavo F. Padovan]

Hi Ville,

* Ville Tervo <ville.tervo@nokia.com> [2010-07-30 16:13:00 +0300]:

> Hi,
> 
> On 07/29/2010 09:00 PM, ext Gustavo F. Padovan wrote:
> >From: Gustavo F. Padovan<padovan@profusion.mobi>
> >
> >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

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

[Marcel Holtmann]

Hi Ville,

> > On 07/29/2010 09:00 PM, ext Gustavo F. Padovan wrote:
> > >From: Gustavo F. Padovan<padovan@profusion.mobi>
> > >
> > >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

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

[Ville Tervo]

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<padovan@profusion.mobi>
> > > >
> > > >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

Re: L2CAP unknown options

[Gustavo F. Padovan]

Hi Ville,

* Ville Tervo <ville.tervo@nokia.com> [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

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

[Gustavo F. Padovan]

From: Gustavo F. Padovan <padovan@profusion.mobi>

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 <padovan@profusion.mobi>
---
 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

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

[Marcel Holtmann]

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 <padovan@profusion.mobi>
> ---
>  net/bluetooth/l2cap.c |   15 ++++++++++++---
>  1 files changed, 12 insertions(+), 3 deletions(-)

patch has been applied. Thanks.

Regards

Marcel