L2CAP unknown options

PATCH In review Ville Tervo 3 participants 12 messages Jul 27, 2010 – Aug 4, 2010

Subsystems (2)

Bluetooth drivers Bluetooth

lore

Share & tools

lore

Ville Tervo

author

2010-07-27 09:07 UTC #0 lore raw

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

Gustavo F. Padovan

2010-07-28 03:32 UTC #1 lore raw

Hi Ville,

Quoted 7 lines · 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.

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.

Quoted 48 lines

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

Signature · 2 lines

Gustavo F. Padovan
http://padovan.org

Gustavo F. Padovan

2010-07-28 07:11 UTC #2 lore raw

PATCH 1/1 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(-)

include/net/bluetooth/l2cap.h +9 -8

index 636724b..407af2b 100644
@@ -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

net/bluetooth/l2cap.c +24 -4

index 9ba1e8e..dc435a3 100644
@@ -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;

Signature · 1 line

1.7.1.1

Gustavo F. Padovan

2010-07-28 07:16 UTC #3 lore raw

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

Hi Ville,

Quoted 11 lines · Gustavo F. Padovan

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.

Signature · 2 lines

Gustavo F. Padovan
http://padovan.org

Ville Tervo

author

2010-07-28 12:21 UTC #4 lore raw

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

Quoted 10 lines · ext Gustavo F. Padovan

Hi Ville,
Quoted 6 lines · Gustavo F. Padovan
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.

Quoted 8 lines

Quoted 5 lines
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.

Signature · 1 line

Ville

Gustavo F. Padovan

2010-07-29 18:00 UTC #5 lore raw

PATCH 1/1 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(-)

net/bluetooth/l2cap.c +12 -3

index 9ba1e8e..0f34e12 100644
@@ -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);

Signature · 1 line

1.7.1.1

Gustavo F. Padovan

2010-07-29 18:04 UTC #6 lore raw

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

Hi Ville,

Quoted 11 lines · Gustavo F. Padovan

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.

Quoted 58 lines

net/bluetooth/l2cap.c +12 -3

index 9ba1e8e..0f34e12 100644
@@ -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);

Signature · 1 line

1.7.1.1

Signature · 2 lines

Gustavo F. Padovan
http://padovan.org

Ville Tervo

author

2010-07-30 13:13 UTC #7 lore raw

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

In reply to Gustavo F. Padovan

Hi,

Quoted 6 lines · ext Gustavo F. Padovan

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).

net/bluetooth/l2cap.c +7 -0

index 0f34e12..1e174a4 100644
@@ -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);

Signature · 1 line

Ville

Gustavo F. Padovan

2010-07-31 22:41 UTC #8 lore raw

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

Hi Ville,

Quoted 5 lines · Ville Tervo

Hi,
Quoted 2 lines · ext Gustavo F. Padovan
From: Gustavo F. Padovan<[email protected]>

Quoted 8 lines

Quoted 4 lines
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?

Quoted 16 lines

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).
net/bluetooth/l2cap.c +0 -0
index 0f34e12..1e174a4 100644
@@ -3348,6 +3348,13 @@ static inline int
l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
        del_timer(&conn->info_timer);

net/bluetooth/l2cap.c static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm · line 3359

Quoted 9 lines

net/bluetooth/l2cap.c +7 -0

+       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.

Signature · 2 lines

Gustavo F. Padovan
http://padovan.org

Marcel Holtmann

maintainer

2010-08-04 01:56 UTC #9 lore raw

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

Hi Ville,

Quoted 29 lines

Quoted 11 lines · ext Gustavo F. Padovan
Quoted 6 lines
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?
Quoted 15 lines
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).
net/bluetooth/l2cap.c +0 -0
index 0f34e12..1e174a4 100644
@@ -3348,6 +3348,13 @@ static inline int
l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
        del_timer(&conn->info_timer);

Quoted 12 lines

net/bluetooth/l2cap.c static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm · line 3359

Quoted 10 lines

net/bluetooth/l2cap.c +7 -0

+       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

Ville Tervo

author

2010-08-04 06:18 UTC #10 lore raw

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

Quoted 44 lines · ext Marcel Holtmann

Hi Ville,
Quoted 29 lines
Quoted 11 lines · ext Gustavo F. Padovan
Quoted 6 lines
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?
Quoted 15 lines
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).
net/bluetooth/l2cap.c +0 -0
index 0f34e12..1e174a4 100644
@@ -3348,6 +3348,13 @@ static inline int
l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
        del_timer(&conn->info_timer);
Quoted 12 lines
net/bluetooth/l2cap.c static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm · line 3359
Quoted 10 lines
net/bluetooth/l2cap.c +7 -0
+       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.

Quoted 1 line

am I getting this proper patch or not?

Yes.

Signature · 1 line

Ville

Gustavo F. Padovan

2010-07-28 04:16 UTC #11 lore raw

In reply to Ville Tervo

Hi Ville,

Quoted 43 lines · 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

Quoted 8 lines

      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.

Quoted 4 lines

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

Signature · 2 lines

Gustavo F. Padovan
http://padovan.org