Pull request: bluetooth-2.6 2010-07-08

PATCH New Marcel Holtmann 2 participants 16 messages Jul 8, 2010

Subsystems (3)

Networking Network drivers Bluetooth

lore

Share & tools

lore

Marcel Holtmann

author maintainer

2010-07-08 20:00 UTC #0 lore raw

Hi Dave,

these patches fix a few bugs and crashes and also two security related issues with the authentication procedure.

This will also be my last pull request for you. From now on all future patches will be submitted towards John's wireless trees. That way we can coordindate the development of Bluetooth 3.0 (Bluetooth over WiFi) and avoid any kind of conflicts.

Regards

Marcel

Please pull from

git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-2.6.git master

This will update the following files:

 net/bluetooth/Kconfig     |   13 ----
 net/bluetooth/hci_conn.c  |    5 ++
 net/bluetooth/hci_event.c |    2 +
 net/bluetooth/l2cap.c     |  138 +++++++++++++++++++++++++++------------------
 4 files changed, 90 insertions(+), 68 deletions(-)

through these ChangeSets:

Andrei Emeltchenko (1):
Bluetooth: Check L2CAP pending status before sending connect request

Gustavo F. Padovan (8):
Bluetooth: Remove max_tx and tx_window module paramenters from L2CAP
Bluetooth: Remove L2CAP Extended Features from Kconfig
Bluetooth: Fix drop of packets with invalid req_seq/tx_seq
Bluetooth: Fix bug with ERTM vars increment
Bluetooth: Only check SAR bits if frame is an I-frame
Bluetooth: Fix bug in l2cap_ertm_send() behavior
Bluetooth: Proper shutdown ERTM when closing the channel
Bluetooth: Fix L2CAP control bit field corruption

Johan Hedberg (1):
Bluetooth: Reset the security level after an authentication failure

João Paulo Rechi Vita (1):
Bluetooth: Fix SREJ_QUEUE corruption in L2CAP

Nathan Holstein (1):
Bluetooth: Fix bug with ERTM minimum packet length

Ville Tervo (1):
Bluetooth: Update sec_level/auth_type for already existing connections

Marcel Holtmann

author maintainer

2010-07-08 20:00 UTC #1 lore raw

PATCH 1/13 Bluetooth: Remove max_tx and tx_window module paramenters from L2CAP

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

We don't need these parameters anymore since we have socket options for them.

Signed-off-by Gustavo F. Padovan <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 net/bluetooth/l2cap.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

net/bluetooth/l2cap.c +2 ~~-10~~

index 1b682a5..5c636b3 100644
@@ -60,8 +60,6 @@ static int enable_ertm = 1;
 #else
 static int enable_ertm = 0;
 #endif
-static int max_transmit = L2CAP_DEFAULT_MAX_TX;
-static int tx_window = L2CAP_DEFAULT_TX_WINDOW;
 
 static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
 static u8 l2cap_fixed_chan[8] = { 0x02, };

@@ -808,9 +806,9 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
 			pi->mode = L2CAP_MODE_ERTM;
 		else
 			pi->mode = L2CAP_MODE_BASIC;
-		pi->max_tx = max_transmit;
+		pi->max_tx = L2CAP_DEFAULT_MAX_TX;
 		pi->fcs  = L2CAP_FCS_CRC16;
-		pi->tx_win = tx_window;
+		pi->tx_win = L2CAP_DEFAULT_TX_WINDOW;
 		pi->sec_level = BT_SECURITY_LOW;
 		pi->role_switch = 0;
 		pi->force_reliable = 0;

@@ -4674,12 +4672,6 @@ module_exit(l2cap_exit);
 module_param(enable_ertm, bool, 0644);
 MODULE_PARM_DESC(enable_ertm, "Enable enhanced retransmission mode");
 
-module_param(max_transmit, uint, 0644);
-MODULE_PARM_DESC(max_transmit, "Max transmit value (default = 3)");
-
-module_param(tx_window, uint, 0644);
-MODULE_PARM_DESC(tx_window, "Transmission window size value (default = 63)");
-
 MODULE_AUTHOR("Marcel Holtmann <[email protected]>");
 MODULE_DESCRIPTION("Bluetooth L2CAP ver " VERSION);
 MODULE_VERSION(VERSION);

Signature · 1 line

1.7.1.1

Marcel Holtmann

author maintainer

2010-07-08 20:00 UTC #2 lore raw

PATCH 2/13 Bluetooth: Remove L2CAP Extended Features from Kconfig

In reply to Marcel Holtmann

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

This reverts commit 84fb0a6334af0ccad3544f6972c055d90fbb9fbe which adds the L2CAP Extended Features to the Kconfig, that is actually not needed. One can use other mechanisms to enable L2CAP Extended Features.

Signed-off-by Gustavo F. Padovan <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 net/bluetooth/Kconfig |   13 -------------
 net/bluetooth/l2cap.c |    4 ----
 2 files changed, 0 insertions(+), 17 deletions(-)

net/bluetooth/Kconfig +0 ~~-13~~

index ee3b304..ed37168 100644
@@ -43,19 +43,6 @@ config BT_L2CAP
 	  Say Y here to compile L2CAP support into the kernel or say M to
 	  compile it as module (l2cap).
 
-config BT_L2CAP_EXT_FEATURES
-	bool "L2CAP Extended Features support (EXPERIMENTAL)"
-	depends on BT_L2CAP && EXPERIMENTAL
-	help
-	  This option enables the L2CAP Extended Features support. These
-	  new features include the Enhanced Retransmission and Streaming
-	  Modes, the Frame Check Sequence (FCS), and Segmentation and
-	  Reassembly (SAR) for L2CAP packets. They are a required for the
-	  new Alternate MAC/PHY and the Bluetooth Medical Profile.
-
-	  You should say N unless you know what you are doing. Note that
-	  this is in an experimental state yet.
-
 config BT_SCO
 	tristate "SCO links support"
 	depends on BT

net/bluetooth/l2cap.c +0 -4

index 5c636b3..e036419 100644
@@ -55,11 +55,7 @@
 
 #define VERSION "2.14"
 
-#ifdef CONFIG_BT_L2CAP_EXT_FEATURES
-static int enable_ertm = 1;
-#else
 static int enable_ertm = 0;
-#endif
 
 static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
 static u8 l2cap_fixed_chan[8] = { 0x02, };

Signature · 1 line

1.7.1.1

Marcel Holtmann

author maintainer

2010-07-08 20:00 UTC #3 lore raw

PATCH 3/13 Bluetooth: Fix drop of packets with invalid req_seq/tx_seq

In reply to Marcel Holtmann

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

We shall not use an unsigned var since we are expecting negatives value there. Using unsigned causes ERTM connection to close due to invalid ReqSeq numbers.

Signed-off-by Gustavo F. Padovan <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 net/bluetooth/l2cap.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

net/bluetooth/l2cap.c +3 -2

index e036419..b08731d 100644
@@ -3789,7 +3789,7 @@ static inline int l2cap_data_channel_iframe(struct sock *sk, u16 rx_control, str
 	u8 tx_seq = __get_txseq(rx_control);
 	u8 req_seq = __get_reqseq(rx_control);
 	u8 sar = rx_control >> L2CAP_CTRL_SAR_SHIFT;
-	u8 tx_seq_offset, expected_tx_seq_offset;
+	int tx_seq_offset, expected_tx_seq_offset;
 	int num_to_ack = (pi->tx_win/6) + 1;
 	int err = 0;

@@ -4074,7 +4074,8 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
 	struct sock *sk;
 	struct l2cap_pinfo *pi;
 	u16 control, len;
-	u8 tx_seq, req_seq, next_tx_seq_offset, req_seq_offset;
+	u8 tx_seq, req_seq;
+	int next_tx_seq_offset, req_seq_offset;
 
 	sk = l2cap_get_chan_by_scid(&conn->chan_list, cid);
 	if (!sk) {

Signature · 1 line

1.7.1.1

Marcel Holtmann

author maintainer

2010-07-08 20:00 UTC #4 lore raw

PATCH 4/13 Bluetooth: Fix bug with ERTM vars increment

In reply to Marcel Holtmann

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

All ERTM operations regarding the txWindow should be modulo 64, otherwise we confuse the ERTM logic and connections will break.

Signed-off-by Gustavo F. Padovan <[email protected]>

Reviewed-by João Paulo Rechi Vita <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 net/bluetooth/l2cap.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

net/bluetooth/l2cap.c +4 -3

index b08731d..bca8c41 100644
@@ -3741,7 +3741,7 @@ static void l2cap_check_srej_gap(struct sock *sk, u8 tx_seq)
 		l2cap_ertm_reassembly_sdu(sk, skb, control);
 		l2cap_pi(sk)->buffer_seq_srej =
 			(l2cap_pi(sk)->buffer_seq_srej + 1) % 64;
-		tx_seq++;
+		tx_seq = (tx_seq + 1) % 64;
 	}
 }

@@ -3777,10 +3777,11 @@ static void l2cap_send_srejframe(struct sock *sk, u8 tx_seq)
 		l2cap_send_sframe(pi, control);
 
 		new = kzalloc(sizeof(struct srej_list), GFP_ATOMIC);
-		new->tx_seq = pi->expected_tx_seq++;
+		new->tx_seq = pi->expected_tx_seq;
+		pi->expected_tx_seq = (pi->expected_tx_seq + 1) % 64;
 		list_add_tail(&new->list, SREJ_LIST(sk));
 	}
-	pi->expected_tx_seq++;
+	pi->expected_tx_seq = (pi->expected_tx_seq + 1) % 64;
 }
 
 static inline int l2cap_data_channel_iframe(struct sock *sk, u16 rx_control, struct sk_buff *skb)

Signature · 1 line

1.7.1.1

Marcel Holtmann

author maintainer

2010-07-08 20:00 UTC #5 lore raw

PATCH 5/13 Bluetooth: Only check SAR bits if frame is an I-frame

In reply to Marcel Holtmann

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

The SAR bits doesn't make sense for an S-frame. It doesn't use SAR.

Checking SAR for a S-frames can lead to L2CAP errors, it could close the channel with an invalid packet length, since we was removing the 2 of the of any frame that match SAR start bits, without check if it is an I-frame.

Signed-off-by Gustavo F. Padovan <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 net/bluetooth/l2cap.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

net/bluetooth/l2cap.c +1 -1

index bca8c41..fa842cc 100644
@@ -4110,7 +4110,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
 		skb_pull(skb, 2);
 		len = skb->len;
 
-		if (__is_sar_start(control))
+		if (__is_sar_start(control) && __is_iframe(control))
 			len -= 2;
 
 		if (pi->fcs == L2CAP_FCS_CRC16)

Signature · 1 line

1.7.1.1

Marcel Holtmann

author maintainer

2010-07-08 20:00 UTC #6 lore raw

PATCH 6/13 Bluetooth: Fix bug in l2cap_ertm_send() behavior

In reply to Marcel Holtmann

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

This patch makes l2cap_ertm_send() similar to the Send-Data action of the ERTM spec. We shall not check for RemoteBusy or WAIT_F state inside l2cap_ertm_send().

Such checks were causing a bug in the retransmission logic of ERTM and making ERTM stalls until the ACL is dropped.

Signed-off-by Gustavo F. Padovan <[email protected]>

Reviewed-by João Paulo Rechi Vita <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 net/bluetooth/l2cap.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

net/bluetooth/l2cap.c +6 -6

index fa842cc..4df5473 100644
@@ -1407,11 +1407,8 @@ static int l2cap_ertm_send(struct sock *sk)
 	u16 control, fcs;
 	int nsent = 0;
 
-	if (pi->conn_state & L2CAP_CONN_WAIT_F)
-		return 0;
 
-	while ((skb = sk->sk_send_head) && (!l2cap_tx_window_full(sk)) &&
-			!(pi->conn_state & L2CAP_CONN_REMOTE_BUSY)) {
+	while ((skb = sk->sk_send_head) && (!l2cap_tx_window_full(sk))) {
 
 		if (pi->remote_max_tx &&
 				bt_cb(skb)->retries == pi->remote_max_tx) {

@@ -1784,6 +1781,11 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms
 		if (pi->mode == L2CAP_MODE_STREAMING) {
 			err = l2cap_streaming_send(sk);
 		} else {
+			if (pi->conn_state & L2CAP_CONN_REMOTE_BUSY &&
+					pi->conn_state && L2CAP_CONN_WAIT_F) {
+				err = len;
+				break;
+			}
 			spin_lock_bh(&pi->send_lock);
 			err = l2cap_ertm_send(sk);
 			spin_unlock_bh(&pi->send_lock);

@@ -3371,8 +3373,6 @@ static inline void l2cap_send_i_or_rr_or_rnr(struct sock *sk)
 	if (pi->conn_state & L2CAP_CONN_REMOTE_BUSY && pi->unacked_frames > 0)
 		__mod_retrans_timer();
 
-	pi->conn_state &= ~L2CAP_CONN_REMOTE_BUSY;
-
 	spin_lock_bh(&pi->send_lock);
 	l2cap_ertm_send(sk);
 	spin_unlock_bh(&pi->send_lock);

Signature · 1 line

1.7.1.1

Marcel Holtmann

author maintainer

2010-07-08 20:00 UTC #7 lore raw

PATCH 7/13 Bluetooth: Fix SREJ_QUEUE corruption in L2CAP

In reply to Marcel Holtmann

From: João Paulo Rechi Vita <[email protected]>

Since all TxSeq values are modulo, we shall not compare them directly. We have to compare their offset inside the TxWindow instead.

Signed-off-by João Paulo Rechi Vita <[email protected]>

Acked-by Gustavo F. Padovan <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 net/bluetooth/l2cap.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

net/bluetooth/l2cap.c +12 -1

index 4df5473..df21cdc 100644
@@ -3387,6 +3387,8 @@ static inline void l2cap_send_i_or_rr_or_rnr(struct sock *sk)
 static int l2cap_add_to_srej_queue(struct sock *sk, struct sk_buff *skb, u8 tx_seq, u8 sar)
 {
 	struct sk_buff *next_skb;
+	struct l2cap_pinfo *pi = l2cap_pi(sk);
+	int tx_seq_offset, next_tx_seq_offset;
 
 	bt_cb(skb)->tx_seq = tx_seq;
 	bt_cb(skb)->sar = sar;

@@ -3397,11 +3399,20 @@ static int l2cap_add_to_srej_queue(struct sock *sk, struct sk_buff *skb, u8 tx_s
 		return 0;
 	}
 
+	tx_seq_offset = (tx_seq - pi->buffer_seq) % 64;
+	if (tx_seq_offset < 0)
+		tx_seq_offset += 64;
+
 	do {
 		if (bt_cb(next_skb)->tx_seq == tx_seq)
 			return -EINVAL;
 
-		if (bt_cb(next_skb)->tx_seq > tx_seq) {
+		next_tx_seq_offset = (bt_cb(next_skb)->tx_seq -
+						pi->buffer_seq) % 64;
+		if (next_tx_seq_offset < 0)
+			next_tx_seq_offset += 64;
+
+		if (next_tx_seq_offset > tx_seq_offset) {
 			__skb_queue_before(SREJ_QUEUE(sk), next_skb, skb);
 			return 0;
 		}

Signature · 1 line

1.7.1.1

Marcel Holtmann

author maintainer

2010-07-08 20:00 UTC #8 lore raw

PATCH 8/13 Bluetooth: Fix bug with ERTM minimum packet length

In reply to Marcel Holtmann

From: Nathan Holstein <[email protected]>

ERTM and streaming mode L2CAP sockets have no minimum packet length. Only basic mode connections have minimum length.

Instead, validate the packet containing all necessary control, FCS, and SAR fields.

The patch fixes the drop of valid packets with length lower than 4.

Signed-off-by Nathan Holstein <[email protected]>

Signed-off-by Gustavo F. Padovan <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 net/bluetooth/l2cap.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

net/bluetooth/l2cap.c +4 -4

index df21cdc..63b0a7d 100644
@@ -4085,9 +4085,9 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
 {
 	struct sock *sk;
 	struct l2cap_pinfo *pi;
-	u16 control, len;
+	u16 control;
 	u8 tx_seq, req_seq;
-	int next_tx_seq_offset, req_seq_offset;
+	int len, next_tx_seq_offset, req_seq_offset;
 
 	sk = l2cap_get_chan_by_scid(&conn->chan_list, cid);
 	if (!sk) {

@@ -4157,7 +4157,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
 		}
 
 		if (__is_iframe(control)) {
-			if (len < 4) {
+			if (len < 0) {
 				l2cap_send_disconn_req(pi->conn, sk);
 				goto drop;
 			}

@@ -4185,7 +4185,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
 		if (pi->fcs == L2CAP_FCS_CRC16)
 			len -= 2;
 
-		if (len > pi->mps || len < 4 || __is_sframe(control))
+		if (len > pi->mps || len < 0 || __is_sframe(control))
 			goto drop;
 
 		if (l2cap_check_fcs(pi, skb))

Signature · 1 line

1.7.1.1

Marcel Holtmann

author maintainer

2010-07-08 20:00 UTC #9 lore raw

PATCH 9/13 Bluetooth: Proper shutdown ERTM when closing the channel

In reply to Marcel Holtmann

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

Fix a crash regarding the Monitor Timeout, it was running even after the shutdown of the ACL connection, which doesn't make sense.

The same code also fixes another issue, before this patch L2CAP was sending many Disconnections Requests while we have to send only one.

The issues are related to each other, a expired Monitor Timeout can trigger a Disconnection Request and then we may have a crash if the link was already deleted.

Signed-off-by Gustavo F. Padovan <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 net/bluetooth/l2cap.c |   59 ++++++++++++++++++++++++++++++------------------
 1 files changed, 37 insertions(+), 22 deletions(-)

net/bluetooth/l2cap.c +37 ~~-22~~

index 63b0a7d..41c9887 100644
@@ -272,6 +272,24 @@ static void l2cap_chan_del(struct sock *sk, int err)
 		parent->sk_data_ready(parent, 0);
 	} else
 		sk->sk_state_change(sk);
+
+	skb_queue_purge(TX_QUEUE(sk));
+
+	if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) {
+		struct srej_list *l, *tmp;
+
+		del_timer(&l2cap_pi(sk)->retrans_timer);
+		del_timer(&l2cap_pi(sk)->monitor_timer);
+		del_timer(&l2cap_pi(sk)->ack_timer);
+
+		skb_queue_purge(SREJ_QUEUE(sk));
+		skb_queue_purge(BUSY_QUEUE(sk));
+
+		list_for_each_entry_safe(l, tmp, SREJ_LIST(sk), list) {
+			list_del(&l->list);
+			kfree(l);
+		}
+	}
 }
 
 /* Service level security */

@@ -345,8 +363,12 @@ static inline void l2cap_send_sframe(struct l2cap_pinfo *pi, u16 control)
 	struct sk_buff *skb;
 	struct l2cap_hdr *lh;
 	struct l2cap_conn *conn = pi->conn;
+	struct sock *sk = (struct sock *)pi;
 	int count, hlen = L2CAP_HDR_SIZE + 2;
 
+	if (sk->sk_state != BT_CONNECTED)
+		return;
+
 	if (pi->fcs == L2CAP_FCS_CRC16)
 		hlen += 2;

@@ -432,10 +454,23 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn, struct sock *sk)
 {
 	struct l2cap_disconn_req req;
 
+	if (!conn)
+		return;
+
+	skb_queue_purge(TX_QUEUE(sk));
+
+	if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) {
+		del_timer(&l2cap_pi(sk)->retrans_timer);
+		del_timer(&l2cap_pi(sk)->monitor_timer);
+		del_timer(&l2cap_pi(sk)->ack_timer);
+	}
+
 	req.dcid = cpu_to_le16(l2cap_pi(sk)->dcid);
 	req.scid = cpu_to_le16(l2cap_pi(sk)->scid);
 	l2cap_send_cmd(conn, l2cap_get_ident(conn),
 			L2CAP_DISCONN_REQ, sizeof(req), &req);
+
+	sk->sk_state = BT_DISCONN;
 }
 
 /* ---- L2CAP connections ---- */

@@ -726,7 +761,6 @@ static void __l2cap_sock_close(struct sock *sk, int reason)
 				sk->sk_type == SOCK_STREAM) {
 			struct l2cap_conn *conn = l2cap_pi(sk)->conn;
 
-			sk->sk_state = BT_DISCONN;
 			l2cap_sock_set_timer(sk, sk->sk_sndtimeo);
 			l2cap_send_disconn_req(conn, sk);
 		} else

@@ -1407,6 +1441,8 @@ static int l2cap_ertm_send(struct sock *sk)
 	u16 control, fcs;
 	int nsent = 0;
 
+	if (sk->sk_state != BT_CONNECTED)
+		return -ENOTCONN;
 
 	while ((skb = sk->sk_send_head) && (!l2cap_tx_window_full(sk))) {

@@ -3065,7 +3101,6 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
 		}
 
 	default:
-		sk->sk_state = BT_DISCONN;
 		sk->sk_err = ECONNRESET;
 		l2cap_sock_set_timer(sk, HZ * 5);
 		l2cap_send_disconn_req(conn, sk);

@@ -3119,16 +3154,6 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd
 
 	sk->sk_shutdown = SHUTDOWN_MASK;
 
-	skb_queue_purge(TX_QUEUE(sk));
-
-	if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) {
-		skb_queue_purge(SREJ_QUEUE(sk));
-		skb_queue_purge(BUSY_QUEUE(sk));
-		del_timer(&l2cap_pi(sk)->retrans_timer);
-		del_timer(&l2cap_pi(sk)->monitor_timer);
-		del_timer(&l2cap_pi(sk)->ack_timer);
-	}
-
 	l2cap_chan_del(sk, ECONNRESET);
 	bh_unlock_sock(sk);

@@ -3151,16 +3176,6 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd
 	if (!sk)
 		return 0;
 
-	skb_queue_purge(TX_QUEUE(sk));
-
-	if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) {
-		skb_queue_purge(SREJ_QUEUE(sk));
-		skb_queue_purge(BUSY_QUEUE(sk));
-		del_timer(&l2cap_pi(sk)->retrans_timer);
-		del_timer(&l2cap_pi(sk)->monitor_timer);
-		del_timer(&l2cap_pi(sk)->ack_timer);
-	}
-
 	l2cap_chan_del(sk, 0);
 	bh_unlock_sock(sk);

Signature · 1 line

1.7.1.1

Marcel Holtmann

author maintainer

2010-07-08 20:00 UTC #10 lore raw

PATCH 10/13 Bluetooth: Fix L2CAP control bit field corruption

In reply to Marcel Holtmann

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

When resending an I-frame, ERTM was reusing the control bits from the last time it was sent, that was causing a corruption in the new control field due to it dirty fields.

This patches extracts only the SAR bits from the old field and reuse it to resend the packet, the others bits should be reset and receive the updated value.

Signed-off-by Gustavo F. Padovan <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 net/bluetooth/l2cap.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

net/bluetooth/l2cap.c +4 -0

index 41c9887..6785e52 100644
@@ -1422,6 +1422,8 @@ static void l2cap_retransmit_one_frame(struct sock *sk, u8 tx_seq)
 	tx_skb = skb_clone(skb, GFP_ATOMIC);
 	bt_cb(skb)->retries++;
 	control = get_unaligned_le16(tx_skb->data + L2CAP_HDR_SIZE);
+	control &= L2CAP_CTRL_SAR;
+
 	control |= (pi->buffer_seq << L2CAP_CTRL_REQSEQ_SHIFT)
 			| (tx_seq << L2CAP_CTRL_TXSEQ_SHIFT);
 	put_unaligned_le16(control, tx_skb->data + L2CAP_HDR_SIZE);

@@ -1457,6 +1459,8 @@ static int l2cap_ertm_send(struct sock *sk)
 		bt_cb(skb)->retries++;
 
 		control = get_unaligned_le16(tx_skb->data + L2CAP_HDR_SIZE);
+		control &= L2CAP_CTRL_SAR;
+
 		if (pi->conn_state & L2CAP_CONN_SEND_FBIT) {
 			control |= L2CAP_CTRL_FINAL;
 			pi->conn_state &= ~L2CAP_CONN_SEND_FBIT;

Signature · 1 line

1.7.1.1

Marcel Holtmann

author maintainer

2010-07-08 20:00 UTC #11 lore raw

PATCH 11/13 Bluetooth: Check L2CAP pending status before sending connect request

In reply to Marcel Holtmann

From: Andrei Emeltchenko <[email protected]>

Due to race condition in L2CAP state machine L2CAP Connection Request may be sent twice for SDP with the same source channel id. Problems reported connecting to Apple products, some carkit, Blackberry phones.

...

2010-06-07 21:18:03.651031 < ACL data: handle 1 flags 0x02 dlen 12
    L2CAP(s): Connect req: psm 1 scid 0x0040
2010-06-07 21:18:03.653473 > HCI Event: Number of Completed Packets (0x13) plen 5
    handle 1 packets 1
2010-06-07 21:18:03.653808 > HCI Event: Auth Complete (0x06) plen 3
    status 0x00 handle 1
2010-06-07 21:18:03.653869 < ACL data: handle 1 flags 0x02 dlen 12
    L2CAP(s): Connect req: psm 1 scid 0x0040

...

Patch uses L2CAP_CONF_CONNECT_PEND flag to mark that L2CAP Connection Request has been sent already.

Modified version of patch from Ville Tervo.

Signed-off-by Andrei Emeltchenko <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 net/bluetooth/l2cap.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

net/bluetooth/l2cap.c +11 -3

index 6785e52..c2fb26d 100644
@@ -417,6 +417,11 @@ static inline void l2cap_send_rr_or_rnr(struct l2cap_pinfo *pi, u16 control)
 	l2cap_send_sframe(pi, control);
 }
 
+static inline int __l2cap_no_conn_pending(struct sock *sk)
+{
+	return !(l2cap_pi(sk)->conf_state & L2CAP_CONF_CONNECT_PEND);
+}
+
 static void l2cap_do_start(struct sock *sk)
 {
 	struct l2cap_conn *conn = l2cap_pi(sk)->conn;

@@ -425,12 +430,13 @@ static void l2cap_do_start(struct sock *sk)
 		if (!(conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE))
 			return;
 
-		if (l2cap_check_security(sk)) {
+		if (l2cap_check_security(sk) && __l2cap_no_conn_pending(sk)) {
 			struct l2cap_conn_req req;
 			req.scid = cpu_to_le16(l2cap_pi(sk)->scid);
 			req.psm  = l2cap_pi(sk)->psm;
 
 			l2cap_pi(sk)->ident = l2cap_get_ident(conn);
+			l2cap_pi(sk)->conf_state |= L2CAP_CONF_CONNECT_PEND;
 
 			l2cap_send_cmd(conn, l2cap_pi(sk)->ident,
 					L2CAP_CONN_REQ, sizeof(req), &req);

@@ -493,12 +499,14 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
 		}
 
 		if (sk->sk_state == BT_CONNECT) {
-			if (l2cap_check_security(sk)) {
+			if (l2cap_check_security(sk) &&
+					__l2cap_no_conn_pending(sk)) {
 				struct l2cap_conn_req req;
 				req.scid = cpu_to_le16(l2cap_pi(sk)->scid);
 				req.psm  = l2cap_pi(sk)->psm;
 
 				l2cap_pi(sk)->ident = l2cap_get_ident(conn);
+				l2cap_pi(sk)->conf_state |= L2CAP_CONF_CONNECT_PEND;
 
 				l2cap_send_cmd(conn, l2cap_pi(sk)->ident,
 					L2CAP_CONN_REQ, sizeof(req), &req);

@@ -2948,7 +2956,6 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd
 		l2cap_pi(sk)->ident = 0;
 		l2cap_pi(sk)->dcid = dcid;
 		l2cap_pi(sk)->conf_state |= L2CAP_CONF_REQ_SENT;
-
 		l2cap_pi(sk)->conf_state &= ~L2CAP_CONF_CONNECT_PEND;
 
 		l2cap_send_cmd(conn, l2cap_get_ident(conn), L2CAP_CONF_REQ,

@@ -4430,6 +4437,7 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 				req.psm  = l2cap_pi(sk)->psm;
 
 				l2cap_pi(sk)->ident = l2cap_get_ident(conn);
+				l2cap_pi(sk)->conf_state |= L2CAP_CONF_CONNECT_PEND;
 
 				l2cap_send_cmd(conn, l2cap_pi(sk)->ident,
 					L2CAP_CONN_REQ, sizeof(req), &req);

Signature · 1 line

1.7.1.1

Marcel Holtmann

author maintainer

2010-07-08 20:00 UTC #12 lore raw

PATCH 12/13 Bluetooth: Reset the security level after an authentication failure

In reply to Marcel Holtmann

From: Johan Hedberg <[email protected]>

When authentication fails for a connection the assumed security level should be set back to BT_SECURITY_LOW so that subsequent connect attempts over the same link don't falsely assume that security is adequate enough.

Signed-off-by Johan Hedberg <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 net/bluetooth/hci_event.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

net/bluetooth/hci_event.c +2 -0

index 6c57fc7..786b5de 100644
@@ -1049,6 +1049,8 @@ static inline void hci_auth_complete_evt(struct hci_dev *hdev, struct sk_buff *s
 	if (conn) {
 		if (!ev->status)
 			conn->link_mode |= HCI_LM_AUTH;
+		else
+			conn->sec_level = BT_SECURITY_LOW;
 
 		clear_bit(HCI_CONN_AUTH_PEND, &conn->pend);

Signature · 1 line

1.7.1.1

Marcel Holtmann

author maintainer

2010-07-08 20:00 UTC #13 lore raw

PATCH 13/13 Bluetooth: Update sec_level/auth_type for already existing connections

In reply to Marcel Holtmann

From: Ville Tervo <[email protected]>

Update auth level for already existing connections if it is lower than required by new connection.

Signed-off-by Ville Tervo <[email protected]>

Reviewed-by Emeltchenko Andrei <[email protected]>

Signed-off-by Luciano Coelho <[email protected]>

Signed-off-by Andrei Emeltchenko <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 net/bluetooth/hci_conn.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

net/bluetooth/hci_conn.c +5 -0

index b10e3cd..800b6b9 100644
@@ -358,6 +358,11 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
 		acl->sec_level = sec_level;
 		acl->auth_type = auth_type;
 		hci_acl_connect(acl);
+	} else {
+		if (acl->sec_level < sec_level)
+			acl->sec_level = sec_level;
+		if (acl->auth_type < auth_type)
+			acl->auth_type = auth_type;
 	}
 
 	if (type == ACL_LINK)

Signature · 1 line

1.7.1.1

David Miller

maintainer

2010-07-08 22:46 UTC #14 lore raw

In reply to Marcel Holtmann

From: Marcel Holtmann <[email protected]>
Date: Thu, 8 Jul 2010 16:59:49 -0300

Quoted 2 lines

these patches fix a few bugs and crashes and also two security related issues with the authentication procedure.

13 changes is too much this late in the -RC series. Fixes need to trickle in, in small quantities, and therefore it's critical that maintainers submit fixes often and as soon as they are ready.

Please pick a small number of the most critical fixes, say 3 or 4. An easy way to roughly quantify which ones shoule be included is:

1) Is there an OOPS or crash regression reported by real users and listed in the official lkml regression list which is caused by this problem?

2) Is there an exploitable security concern fixed by this change?

Else, it's only net-next-2.6 material.

For example:

Bluetooth: Remove max_tx and tx_window module paramenters from L2CAP

Things like that change are absolutely not appropriate at this stage in the post merge-window development environment.

Thanks.

Marcel Holtmann

author maintainer

2010-07-08 23:29 UTC #15 lore raw

Hi Dave,

Quoted 16 lines

Quoted 2 lines
these patches fix a few bugs and crashes and also two security related issues with the authentication procedure.
13 changes is too much this late in the -RC series. Fixes need to trickle in, in small quantities, and therefore it's critical that maintainers submit fixes often and as soon as they are ready.
Please pick a small number of the most critical fixes, say 3 or 4. An easy way to roughly quantify which ones shoule be included is:
1) Is there an OOPS or crash regression reported by real users and listed in the official lkml regression list which is caused by this problem?
2) Is there an exploitable security concern fixed by this change?

Quoted 8 lines

Else, it's only net-next-2.6 material.
For example:
Bluetooth: Remove max_tx and tx_window module paramenters from L2CAP
Things like that change are absolutely not appropriate at this stage in the post merge-window development environment.

I can take these out and leave them for -next. That is fine with me, but you asked Gustavo to remove these. And so I left them in.

Regards

Marcel