[PATCH v2 0/9] Bluetooth: More generic L2CAP fixed channel handling

[PATCH v2 0/9] Bluetooth: More generic L2CAP fixed channel handling

[johan.hedberg]

Hi,

Here's v2 of my set with comments on patch 1/9 taken into account.

>From the original cover letter:

"
Here's a set of patches to make fixed L2CAP channel handling more
generic. The main special-casing that's removed here is for ATT
channels. The A2MP stuff is left for later since its semantics would
change from creating the context only when there's data on the channel
to as soon as we have a baseband link (which I'm not sure is desirable).

The main motivation for all of this work is to pave the way to convert
SMP to fully use the l2cap_chan infrastructure.
"

----------------------------------------------------------------
Johan Hedberg (9):
      Bluetooth: Fix reference counting of global L2CAP channels
      Bluetooth: Fix __l2cap_no_conn_pending() usage with all channels
      Bluetooth: Resume BT_CONNECTED state after LE security elevation
      Bluetooth: Remove special handling of ATT in l2cap_security_cfm()
      Bluetooth: Refactor l2cap_connect_cfm
      Bluetooth: Move L2CAP fixed channel creation into l2cap_conn_cfm
      Bluetooth: Improve fixed channel lookup based on link type
      Bluetooth: Remove special ATT data channel handling
      Bluetooth: Move parts of fixed channel initialization to l2cap_add_scid

 include/net/bluetooth/l2cap.h |   1 +
 net/bluetooth/l2cap_core.c    | 245 ++++++++++++++++++--------------------
 net/bluetooth/l2cap_sock.c    |  15 +--
 3 files changed, 122 insertions(+), 139 deletions(-)

[PATCH v2 1/9] Bluetooth: Fix reference counting of global L2CAP channels

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

When looking up entries from the global L2CAP channel list there needs
to be a guarantee that other code doesn't go and remove the entry after
a channel has been returned by the lookup function. This patch makes
sure that the channel reference is incremented before the read lock is
released in the global channel lookup functions. The patch also adds the
corresponding l2cap_chan_put() calls once the channels pointers are
no-longer needed.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/l2cap_core.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 46547b920f88..a47a14efd5aa 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1440,6 +1440,7 @@ static struct l2cap_chan *l2cap_global_chan_by_scid(int state, u16 cid,
 			src_match = !bacmp(&c->src, src);
 			dst_match = !bacmp(&c->dst, dst);
 			if (src_match && dst_match) {
+				l2cap_chan_hold(c);
 				read_unlock(&chan_list_lock);
 				return c;
 			}
@@ -1453,6 +1454,9 @@ static struct l2cap_chan *l2cap_global_chan_by_scid(int state, u16 cid,
 		}
 	}
 
+	if (c1)
+		l2cap_chan_hold(c1);
+
 	read_unlock(&chan_list_lock);
 
 	return c1;
@@ -1475,13 +1479,13 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 
 	/* Client ATT sockets should override the server one */
 	if (__l2cap_get_chan_by_dcid(conn, L2CAP_CID_ATT))
-		return;
+		goto put;
 
 	dst_type = bdaddr_type(hcon, hcon->dst_type);
 
 	/* If device is blocked, do not create a channel for it */
 	if (hci_bdaddr_list_lookup(&hdev->blacklist, &hcon->dst, dst_type))
-		return;
+		goto put;
 
 	/* For LE slave connections, make sure the connection interval
 	 * is in the range of the minium and maximum interval that has
@@ -1517,6 +1521,8 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 
 clean:
 	l2cap_chan_unlock(pchan);
+put:
+	l2cap_chan_put(pchan);
 }
 
 static void l2cap_conn_ready(struct l2cap_conn *conn)
@@ -1794,6 +1800,7 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm,
 			src_match = !bacmp(&c->src, src);
 			dst_match = !bacmp(&c->dst, dst);
 			if (src_match && dst_match) {
+				l2cap_chan_hold(c);
 				read_unlock(&chan_list_lock);
 				return c;
 			}
@@ -1807,6 +1814,9 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm,
 		}
 	}
 
+	if (c1)
+		l2cap_chan_hold(c1);
+
 	read_unlock(&chan_list_lock);
 
 	return c1;
@@ -3884,6 +3894,7 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
 response:
 	l2cap_chan_unlock(pchan);
 	mutex_unlock(&conn->chan_lock);
+	l2cap_chan_put(pchan);
 
 sendresp:
 	rsp.scid   = cpu_to_le16(scid);
@@ -5497,6 +5508,7 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
 response_unlock:
 	l2cap_chan_unlock(pchan);
 	mutex_unlock(&conn->chan_lock);
+	l2cap_chan_put(pchan);
 
 	if (result == L2CAP_CR_PEND)
 		return 0;
@@ -6845,12 +6857,12 @@ static void l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm,
 	struct l2cap_chan *chan;
 
 	if (hcon->type != ACL_LINK)
-		goto drop;
+		goto free_skb;
 
 	chan = l2cap_global_chan_by_psm(0, psm, &hcon->src, &hcon->dst,
 					ACL_LINK);
 	if (!chan)
-		goto drop;
+		goto free_skb;
 
 	BT_DBG("chan %p, len %d", chan, skb->len);
 
@@ -6864,10 +6876,14 @@ static void l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm,
 	bacpy(&bt_cb(skb)->bdaddr, &hcon->dst);
 	bt_cb(skb)->psm = psm;
 
-	if (!chan->ops->recv(chan, skb))
+	if (!chan->ops->recv(chan, skb)) {
+		l2cap_chan_put(chan);
 		return;
+	}
 
 drop:
+	l2cap_chan_put(chan);
+free_skb:
 	kfree_skb(skb);
 }
 
@@ -6878,22 +6894,26 @@ static void l2cap_att_channel(struct l2cap_conn *conn,
 	struct l2cap_chan *chan;
 
 	if (hcon->type != LE_LINK)
-		goto drop;
+		goto free_skb;
 
 	chan = l2cap_global_chan_by_scid(BT_CONNECTED, L2CAP_CID_ATT,
 					 &hcon->src, &hcon->dst);
 	if (!chan)
-		goto drop;
+		goto free_skb;
 
 	BT_DBG("chan %p, len %d", chan, skb->len);
 
 	if (chan->imtu < skb->len)
 		goto drop;
 
-	if (!chan->ops->recv(chan, skb))
+	if (!chan->ops->recv(chan, skb)) {
+		l2cap_chan_put(chan);
 		return;
+	}
 
 drop:
+	l2cap_chan_put(chan);
+free_skb:
 	kfree_skb(skb);
 }
 
-- 
1.9.3

[PATCH v2 2/9] Bluetooth: Fix __l2cap_no_conn_pending() usage with all channels

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

The __l2cap_no_conn_pending() function would previously only return a
meaningful value for connection oriented channels and was therefore not
useful for anything else. As preparation of making the L2CAP code more
generic allow the function to be called for other channel types as well
by returning a meaningful value for them.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/l2cap_core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index a47a14efd5aa..a0a910eb897b 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1082,6 +1082,9 @@ static void l2cap_send_rr_or_rnr(struct l2cap_chan *chan, bool poll)
 
 static inline int __l2cap_no_conn_pending(struct l2cap_chan *chan)
 {
+	if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED)
+		return true;
+
 	return !test_bit(CONF_CONNECT_PEND, &chan->conf_state);
 }
 
-- 
1.9.3

[PATCH v2 3/9] Bluetooth: Resume BT_CONNECTED state after LE security elevation

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

The LE ATT socket uses a special trick where it temporarily sets
BT_CONFIG state for the duration of a security level elevation. In order
to not require special hacks for going back to BT_CONNECTED state in the
l2cap_core.c code the most reasonable place to resume the state is the
resume callback. This patch adds a new flag to track the pending
security level change and ensures that the state is set back to
BT_CONNECTED in the resume callback in case the flag is set.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/l2cap.h | 1 +
 net/bluetooth/l2cap_sock.c    | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 8df15ad0d43f..4a51e7596608 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -708,6 +708,7 @@ enum {
 	FLAG_EFS_ENABLE,
 	FLAG_DEFER_SETUP,
 	FLAG_LE_CONN_REQ_SENT,
+	FLAG_PENDING_SECURITY,
 };
 
 enum {
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 1884f72083c2..5a42f6a818c0 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -790,6 +790,7 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
 		if (chan->scid == L2CAP_CID_ATT) {
 			if (smp_conn_security(conn->hcon, sec.level))
 				break;
+			set_bit(FLAG_PENDING_SECURITY, &chan->flags);
 			sk->sk_state = BT_CONFIG;
 			chan->state = BT_CONFIG;
 
@@ -1359,6 +1360,11 @@ static void l2cap_sock_resume_cb(struct l2cap_chan *chan)
 {
 	struct sock *sk = chan->data;
 
+	if (test_and_clear_bit(FLAG_PENDING_SECURITY, &chan->flags)) {
+		sk->sk_state = BT_CONNECTED;
+		chan->state = BT_CONNECTED;
+	}
+
 	clear_bit(BT_SK_SUSPEND, &bt_sk(sk)->flags);
 	sk->sk_state_change(sk);
 }
-- 
1.9.3

[PATCH v2 4/9] Bluetooth: Remove special handling of ATT in l2cap_security_cfm()

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

With the update to sk->resume() and __l2cap_no_conn_pending() we
no-longer need to have special handling of ATT channels in the
l2cap_security_cfm() function. The chan->sec_level update when
encryption has been enabled is safe to do for any kind of channel, and
the loop takes later care of calling chan->ready() or chan->resume() if
necessary.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/l2cap_core.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index a0a910eb897b..54cbfcd4da44 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -7341,15 +7341,8 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 			continue;
 		}
 
-		if (chan->scid == L2CAP_CID_ATT) {
-			if (!status && encrypt) {
-				chan->sec_level = hcon->sec_level;
-				l2cap_chan_ready(chan);
-			}
-
-			l2cap_chan_unlock(chan);
-			continue;
-		}
+		if (!status && encrypt)
+			chan->sec_level = hcon->sec_level;
 
 		if (!__l2cap_no_conn_pending(chan)) {
 			l2cap_chan_unlock(chan);
-- 
1.9.3

[PATCH v2 5/9] Bluetooth: Refactor l2cap_connect_cfm

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch is a simple refactoring of l2cap_connect_cfm to allow easier
extension of the function.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/l2cap_core.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 54cbfcd4da44..7a5cff89b792 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -7268,13 +7268,16 @@ void l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
 
 	BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status);
 
-	if (!status) {
-		conn = l2cap_conn_add(hcon);
-		if (conn)
-			l2cap_conn_ready(conn);
-	} else {
+	if (status) {
 		l2cap_conn_del(hcon, bt_to_errno(status));
+		return;
 	}
+
+	conn = l2cap_conn_add(hcon);
+	if (!conn)
+		return;
+
+	l2cap_conn_ready(conn);
 }
 
 int l2cap_disconn_ind(struct hci_conn *hcon)
-- 
1.9.3

[PATCH v2 6/9] Bluetooth: Move L2CAP fixed channel creation into l2cap_conn_cfm

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

In order to remove special handling of fixed L2CAP channels we need to
start creating them in a single place instead of having per-channel
exceptions. The most natural place is the l2cap_conn_cfm() function
which is called whenever there is a new baseband link.

The only really special case so far has been the ATT socket, so in order
not to break the code in between this patch removes the ATT special
handling at the same time as it adds the generic fixed channel handling
from l2cap_le_conn_ready() into the hci_conn_cfm() function. As a
related change the channel locking in l2cap_conn_ready() becomes simpler
and we can thereby move the smp_conn_security() call into the
l2cap_le_conn_ready() function.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/l2cap_core.c | 124 +++++++++++++++++++++++++++++----------------
 1 file changed, 80 insertions(+), 44 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 7a5cff89b792..8c4767f91a23 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1469,26 +1469,14 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 {
 	struct hci_conn *hcon = conn->hcon;
 	struct hci_dev *hdev = hcon->hdev;
-	struct l2cap_chan *chan, *pchan;
-	u8 dst_type;
 
-	BT_DBG("");
-
-	/* Check if we have socket listening on cid */
-	pchan = l2cap_global_chan_by_scid(BT_LISTEN, L2CAP_CID_ATT,
-					  &hcon->src, &hcon->dst);
-	if (!pchan)
-		return;
+	BT_DBG("%s conn %p", hdev->name, conn);
 
-	/* Client ATT sockets should override the server one */
-	if (__l2cap_get_chan_by_dcid(conn, L2CAP_CID_ATT))
-		goto put;
-
-	dst_type = bdaddr_type(hcon, hcon->dst_type);
-
-	/* If device is blocked, do not create a channel for it */
-	if (hci_bdaddr_list_lookup(&hdev->blacklist, &hcon->dst, dst_type))
-		goto put;
+	/* For outgoing pairing which doesn't necessarily have an
+	 * associated socket (e.g. mgmt_pair_device).
+	 */
+	if (hcon->out)
+		smp_conn_security(hcon, hcon->pending_sec_level);
 
 	/* For LE slave connections, make sure the connection interval
 	 * is in the range of the minium and maximum interval that has
@@ -1508,24 +1496,6 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 		l2cap_send_cmd(conn, l2cap_get_ident(conn),
 			       L2CAP_CONN_PARAM_UPDATE_REQ, sizeof(req), &req);
 	}
-
-	l2cap_chan_lock(pchan);
-
-	chan = pchan->ops->new_connection(pchan);
-	if (!chan)
-		goto clean;
-
-	bacpy(&chan->src, &hcon->src);
-	bacpy(&chan->dst, &hcon->dst);
-	chan->src_type = bdaddr_type(hcon, hcon->src_type);
-	chan->dst_type = dst_type;
-
-	__l2cap_chan_add(conn, chan);
-
-clean:
-	l2cap_chan_unlock(pchan);
-put:
-	l2cap_chan_put(pchan);
 }
 
 static void l2cap_conn_ready(struct l2cap_conn *conn)
@@ -1535,17 +1505,11 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
 
 	BT_DBG("conn %p", conn);
 
-	/* For outgoing pairing which doesn't necessarily have an
-	 * associated socket (e.g. mgmt_pair_device).
-	 */
-	if (hcon->out && hcon->type == LE_LINK)
-		smp_conn_security(hcon, hcon->pending_sec_level);
-
-	mutex_lock(&conn->chan_lock);
-
 	if (hcon->type == LE_LINK)
 		l2cap_le_conn_ready(conn);
 
+	mutex_lock(&conn->chan_lock);
+
 	list_for_each_entry(chan, &conn->chan_l, list) {
 
 		l2cap_chan_lock(chan);
@@ -7262,9 +7226,44 @@ int l2cap_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr)
 	return exact ? lm1 : lm2;
 }
 
+/* Find the next fixed channel in BT_LISTEN state, continue iteration
+ * from an existing channel in the list or from the beginning of the
+ * global list (by passing NULL as first parameter).
+ */
+static struct l2cap_chan *l2cap_global_fixed_chan(struct l2cap_chan *c,
+						  bdaddr_t *src)
+{
+	read_lock(&chan_list_lock);
+
+	if (c)
+		c = list_next_entry(c, global_l);
+	else
+		c = list_entry(chan_list.next, typeof(*c), global_l);
+
+	list_for_each_entry_from(c, &chan_list, global_l) {
+		if (c->chan_type != L2CAP_CHAN_FIXED)
+			continue;
+		if (c->state != BT_LISTEN)
+			continue;
+		if (bacmp(&c->src, src) && bacmp(&c->src, BDADDR_ANY))
+			continue;
+
+		l2cap_chan_hold(c);
+		read_unlock(&chan_list_lock);
+		return c;
+	}
+
+	read_unlock(&chan_list_lock);
+
+	return NULL;
+}
+
 void l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
 {
+	struct hci_dev *hdev = hcon->hdev;
 	struct l2cap_conn *conn;
+	struct l2cap_chan *pchan;
+	u8 dst_type;
 
 	BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status);
 
@@ -7277,6 +7276,43 @@ void l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
 	if (!conn)
 		return;
 
+	dst_type = bdaddr_type(hcon, hcon->dst_type);
+
+	/* If device is blocked, do not create channels for it */
+	if (hci_bdaddr_list_lookup(&hdev->blacklist, &hcon->dst, dst_type))
+		return;
+
+	/* Find fixed channels and notify them of the new connection. We
+	 * use multiple individual lookups, continuing each time where
+	 * we left off, because the list lock would prevent calling the
+	 * potentially sleeping l2cap_chan_lock() function.
+	 */
+	pchan = l2cap_global_fixed_chan(NULL, &hdev->bdaddr);
+	while (pchan) {
+		struct l2cap_chan *chan, *next;
+
+		/* Client fixed channels should override server ones */
+		if (__l2cap_get_chan_by_dcid(conn, pchan->scid))
+			goto next;
+
+		l2cap_chan_lock(pchan);
+		chan = pchan->ops->new_connection(pchan);
+		if (chan) {
+			bacpy(&chan->src, &hcon->src);
+			bacpy(&chan->dst, &hcon->dst);
+			chan->src_type = bdaddr_type(hcon, hcon->src_type);
+			chan->dst_type = dst_type;
+
+			__l2cap_chan_add(conn, chan);
+		}
+
+		l2cap_chan_unlock(pchan);
+next:
+		next = l2cap_global_fixed_chan(pchan, &hdev->bdaddr);
+		l2cap_chan_put(pchan);
+		pchan = next;
+	}
+
 	l2cap_conn_ready(conn);
 }
 
-- 
1.9.3

[PATCH v2 7/9] Bluetooth: Improve fixed channel lookup based on link type

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

When notifying global fixed channels of new connections it doesn't make
sense to consider channels meant for a different link type than the one
available. This patch adds an extra parameter to the
l2cap_global_fixed_chan() lookup function and ensures that only channels
matching the current hci_conn type are looked up.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/l2cap_core.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 8c4767f91a23..191b58f48245 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -7231,7 +7231,7 @@ int l2cap_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr)
  * global list (by passing NULL as first parameter).
  */
 static struct l2cap_chan *l2cap_global_fixed_chan(struct l2cap_chan *c,
-						  bdaddr_t *src)
+						  bdaddr_t *src, u8 link_type)
 {
 	read_lock(&chan_list_lock);
 
@@ -7247,6 +7247,10 @@ static struct l2cap_chan *l2cap_global_fixed_chan(struct l2cap_chan *c,
 			continue;
 		if (bacmp(&c->src, src) && bacmp(&c->src, BDADDR_ANY))
 			continue;
+		if (link_type == ACL_LINK && c->src_type != BDADDR_BREDR)
+			continue;
+		if (link_type == LE_LINK && c->src_type == BDADDR_BREDR)
+			continue;
 
 		l2cap_chan_hold(c);
 		read_unlock(&chan_list_lock);
@@ -7287,7 +7291,7 @@ void l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
 	 * we left off, because the list lock would prevent calling the
 	 * potentially sleeping l2cap_chan_lock() function.
 	 */
-	pchan = l2cap_global_fixed_chan(NULL, &hdev->bdaddr);
+	pchan = l2cap_global_fixed_chan(NULL, &hdev->bdaddr, hcon->type);
 	while (pchan) {
 		struct l2cap_chan *chan, *next;
 
@@ -7308,7 +7312,8 @@ void l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
 
 		l2cap_chan_unlock(pchan);
 next:
-		next = l2cap_global_fixed_chan(pchan, &hdev->bdaddr);
+		next = l2cap_global_fixed_chan(pchan, &hdev->bdaddr,
+					       hcon->type);
 		l2cap_chan_put(pchan);
 		pchan = next;
 	}
-- 
1.9.3

[PATCH v2 8/9] Bluetooth: Remove special ATT data channel handling

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

Now that we've got the fixed channel infrastructure cleaned up in a
generic way there's no longer a need to have a dedicated function for
handling data on the ATT channel. Instead the generic
l2cap_data_channel() handler will be able to do the exact same thing.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/l2cap_core.c | 79 ----------------------------------------------
 1 file changed, 79 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 191b58f48245..09eb4e3e8ab2 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1420,51 +1420,6 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
 	mutex_unlock(&conn->chan_lock);
 }
 
-/* Find socket with cid and source/destination bdaddr.
- * Returns closest match, locked.
- */
-static struct l2cap_chan *l2cap_global_chan_by_scid(int state, u16 cid,
-						    bdaddr_t *src,
-						    bdaddr_t *dst)
-{
-	struct l2cap_chan *c, *c1 = NULL;
-
-	read_lock(&chan_list_lock);
-
-	list_for_each_entry(c, &chan_list, global_l) {
-		if (state && c->state != state)
-			continue;
-
-		if (c->scid == cid) {
-			int src_match, dst_match;
-			int src_any, dst_any;
-
-			/* Exact match. */
-			src_match = !bacmp(&c->src, src);
-			dst_match = !bacmp(&c->dst, dst);
-			if (src_match && dst_match) {
-				l2cap_chan_hold(c);
-				read_unlock(&chan_list_lock);
-				return c;
-			}
-
-			/* Closest match */
-			src_any = !bacmp(&c->src, BDADDR_ANY);
-			dst_any = !bacmp(&c->dst, BDADDR_ANY);
-			if ((src_match && dst_any) || (src_any && dst_match) ||
-			    (src_any && dst_any))
-				c1 = c;
-		}
-	}
-
-	if (c1)
-		l2cap_chan_hold(c1);
-
-	read_unlock(&chan_list_lock);
-
-	return c1;
-}
-
 static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 {
 	struct hci_conn *hcon = conn->hcon;
@@ -6854,36 +6809,6 @@ free_skb:
 	kfree_skb(skb);
 }
 
-static void l2cap_att_channel(struct l2cap_conn *conn,
-			      struct sk_buff *skb)
-{
-	struct hci_conn *hcon = conn->hcon;
-	struct l2cap_chan *chan;
-
-	if (hcon->type != LE_LINK)
-		goto free_skb;
-
-	chan = l2cap_global_chan_by_scid(BT_CONNECTED, L2CAP_CID_ATT,
-					 &hcon->src, &hcon->dst);
-	if (!chan)
-		goto free_skb;
-
-	BT_DBG("chan %p, len %d", chan, skb->len);
-
-	if (chan->imtu < skb->len)
-		goto drop;
-
-	if (!chan->ops->recv(chan, skb)) {
-		l2cap_chan_put(chan);
-		return;
-	}
-
-drop:
-	l2cap_chan_put(chan);
-free_skb:
-	kfree_skb(skb);
-}
-
 static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb)
 {
 	struct l2cap_hdr *lh = (void *) skb->data;
@@ -6929,10 +6854,6 @@ static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb)
 		l2cap_conless_channel(conn, psm, skb);
 		break;
 
-	case L2CAP_CID_ATT:
-		l2cap_att_channel(conn, skb);
-		break;
-
 	case L2CAP_CID_LE_SIGNALING:
 		l2cap_le_sig_channel(conn, skb);
 		break;
-- 
1.9.3

[PATCH v2 9/9] Bluetooth: Move parts of fixed channel initialization to l2cap_add_scid

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

The l2cap_add_scid function is used for registering a fixed L2CAP
channel. Instead of having separate initialization of the channel type
and outgoing MTU in l2cap_sock.c it's more intuitive to do these things
in the l2cap_add_scid function itself (and thereby make the
functionality available to other users besides l2cap_sock.c).

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/l2cap_core.c | 4 ++++
 net/bluetooth/l2cap_sock.c | 9 ---------
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 09eb4e3e8ab2..344a29c53227 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -210,6 +210,10 @@ int l2cap_add_scid(struct l2cap_chan *chan,  __u16 scid)
 {
 	write_lock(&chan_list_lock);
 
+	/* Override the defaults (which are for conn-oriented) */
+	chan->omtu = L2CAP_DEFAULT_MTU;
+	chan->chan_type = L2CAP_CHAN_FIXED;
+
 	chan->scid = scid;
 
 	write_unlock(&chan_list_lock);
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 5a42f6a818c0..ed06f88e6f10 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -99,15 +99,6 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
 	if (!bdaddr_type_is_valid(la.l2_bdaddr_type))
 		return -EINVAL;
 
-	if (la.l2_cid) {
-		/* When the socket gets created it defaults to
-		 * CHAN_CONN_ORIENTED, so we need to overwrite the
-		 * default here.
-		 */
-		chan->chan_type = L2CAP_CHAN_FIXED;
-		chan->omtu = L2CAP_DEFAULT_MTU;
-	}
-
 	if (bdaddr_type_is_le(la.l2_bdaddr_type)) {
 		/* We only allow ATT user space socket */
 		if (la.l2_cid &&
-- 
1.9.3

Re: [PATCH v2 0/9] Bluetooth: More generic L2CAP fixed channel handling

[Marcel Holtmann]

Hi Johan,

> Here's v2 of my set with comments on patch 1/9 taken into account.
> 
> From the original cover letter:
> 
> "
> Here's a set of patches to make fixed L2CAP channel handling more
> generic. The main special-casing that's removed here is for ATT
> channels. The A2MP stuff is left for later since its semantics would
> change from creating the context only when there's data on the channel
> to as soon as we have a baseband link (which I'm not sure is desirable).
> 
> The main motivation for all of this work is to pave the way to convert
> SMP to fully use the l2cap_chan infrastructure.
> "
> 
> ----------------------------------------------------------------
> Johan Hedberg (9):
>      Bluetooth: Fix reference counting of global L2CAP channels
>      Bluetooth: Fix __l2cap_no_conn_pending() usage with all channels
>      Bluetooth: Resume BT_CONNECTED state after LE security elevation
>      Bluetooth: Remove special handling of ATT in l2cap_security_cfm()
>      Bluetooth: Refactor l2cap_connect_cfm
>      Bluetooth: Move L2CAP fixed channel creation into l2cap_conn_cfm
>      Bluetooth: Improve fixed channel lookup based on link type
>      Bluetooth: Remove special ATT data channel handling
>      Bluetooth: Move parts of fixed channel initialization to l2cap_add_scid
> 
> include/net/bluetooth/l2cap.h |   1 +
> net/bluetooth/l2cap_core.c    | 245 ++++++++++++++++++--------------------
> net/bluetooth/l2cap_sock.c    |  15 +--
> 3 files changed, 122 insertions(+), 139 deletions(-)

all 9 patches have been applied to bluetooth-next tree.

Regards

Marcel