mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/4] Bluetooth: 6lowpan: reset link-local header on ipv6 recv path
@ 2025-11-01 12:09 Pauli Virtanen
  2025-11-01 12:09 ` [PATCH 2/4] Bluetooth: 6lowpan: fix BDADDR_LE vs ADDR_LE_DEV address type confusion Pauli Virtanen
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Pauli Virtanen @ 2025-11-01 12:09 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: Pauli Virtanen, marcel, johan.hedberg, luiz.dentz,
	jukka.rissanen, linux-kernel

Bluetooth 6lowpan.c netdev has header_ops, so it must set link-local
header for RX skb, otherwise things crash, eg. with AF_PACKET SOCK_RAW

Add missing skb_reset_mac_header() for uncompressed ipv6 RX path.

For the compressed one, it is done in lowpan_header_decompress().

Log: (BlueZ 6lowpan-tester Client Recv Raw - Success)
------
kernel BUG at net/core/skbuff.c:212!
Call Trace:
<IRQ>
...
packet_rcv (net/packet/af_packet.c:2152)
...
<TASK>
__local_bh_enable_ip (kernel/softirq.c:407)
netif_rx (net/core/dev.c:5648)
chan_recv_cb (net/bluetooth/6lowpan.c:294 net/bluetooth/6lowpan.c:359)
------

Fixes: 18722c247023 ("Bluetooth: Enable 6LoWPAN support for BT LE devices")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/6lowpan.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index f0c862091bff..f1d29fa4b411 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -289,6 +289,7 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
 		local_skb->pkt_type = PACKET_HOST;
 		local_skb->dev = dev;
 
+		skb_reset_mac_header(local_skb);
 		skb_set_transport_header(local_skb, sizeof(struct ipv6hdr));
 
 		if (give_skb_to_upper(local_skb, dev) != NET_RX_SUCCESS) {
-- 
2.51.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/4] Bluetooth: 6lowpan: fix BDADDR_LE vs ADDR_LE_DEV address type confusion
  2025-11-01 12:09 [PATCH 1/4] Bluetooth: 6lowpan: reset link-local header on ipv6 recv path Pauli Virtanen
@ 2025-11-01 12:09 ` Pauli Virtanen
  2025-11-01 16:24   ` Paul Menzel
  2025-11-01 12:09 ` [PATCH 3/4] Bluetooth: 6lowpan: Don't hold spin lock over sleeping functions Pauli Virtanen
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Pauli Virtanen @ 2025-11-01 12:09 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: Pauli Virtanen, marcel, johan.hedberg, luiz.dentz,
	jukka.rissanen, linux-kernel

Bluetooth 6lowpan.c confuses BDADDR_LE and ADDR_LE_DEV address types,
e.g. debugfs "connect" command takes the former, and "disconnect" and
"connect" to already connected device take the latter.  This is due to
using same value both for l2cap_chan_connect and hci_conn_hash_lookup_le
which take different dst_type values.

Fix address type passed to hci_conn_hash_lookup_le().

Retain the debugfs API difference between "connect" and "disconnect"
commands since it's been like this since 2015 and nobody apparently
complained.

Fixes: f5ad4ffceba0 ("Bluetooth: 6lowpan: Use hci_conn_hash_lookup_le() when possible")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/6lowpan.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index f1d29fa4b411..0d8c2e2e9a6c 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -957,10 +957,11 @@ static struct l2cap_chan *bt_6lowpan_listen(void)
 }
 
 static int get_l2cap_conn(char *buf, bdaddr_t *addr, u8 *addr_type,
-			  struct l2cap_conn **conn)
+			  struct l2cap_conn **conn, bool disconnect)
 {
 	struct hci_conn *hcon;
 	struct hci_dev *hdev;
+	int le_addr_type;
 	int n;
 
 	n = sscanf(buf, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx %hhu",
@@ -971,13 +972,32 @@ static int get_l2cap_conn(char *buf, bdaddr_t *addr, u8 *addr_type,
 	if (n < 7)
 		return -EINVAL;
 
+	if (disconnect) {
+		/* The "disconnect" debugfs command has used different address
+		 * type constants than "connect" since 2015. Let's retain that
+		 * for now even though it's obviously buggy...
+		 */
+		*addr_type += 1;
+	}
+
+	switch (*addr_type) {
+	case BDADDR_LE_PUBLIC:
+		le_addr_type = ADDR_LE_DEV_PUBLIC;
+		break;
+	case BDADDR_LE_RANDOM:
+		le_addr_type = ADDR_LE_DEV_RANDOM;
+		break;
+	default:
+		return -EINVAL;
+	}
+
 	/* The LE_PUBLIC address type is ignored because of BDADDR_ANY */
 	hdev = hci_get_route(addr, BDADDR_ANY, BDADDR_LE_PUBLIC);
 	if (!hdev)
 		return -ENOENT;
 
 	hci_dev_lock(hdev);
-	hcon = hci_conn_hash_lookup_le(hdev, addr, *addr_type);
+	hcon = hci_conn_hash_lookup_le(hdev, addr, le_addr_type);
 	hci_dev_unlock(hdev);
 	hci_dev_put(hdev);
 
@@ -1104,7 +1124,7 @@ static ssize_t lowpan_control_write(struct file *fp,
 	buf[buf_size] = '\0';
 
 	if (memcmp(buf, "connect ", 8) == 0) {
-		ret = get_l2cap_conn(&buf[8], &addr, &addr_type, &conn);
+		ret = get_l2cap_conn(&buf[8], &addr, &addr_type, &conn, false);
 		if (ret == -EINVAL)
 			return ret;
 
@@ -1141,7 +1161,7 @@ static ssize_t lowpan_control_write(struct file *fp,
 	}
 
 	if (memcmp(buf, "disconnect ", 11) == 0) {
-		ret = get_l2cap_conn(&buf[11], &addr, &addr_type, &conn);
+		ret = get_l2cap_conn(&buf[11], &addr, &addr_type, &conn, true);
 		if (ret < 0)
 			return ret;
 
-- 
2.51.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/4] Bluetooth: 6lowpan: Don't hold spin lock over sleeping functions
  2025-11-01 12:09 [PATCH 1/4] Bluetooth: 6lowpan: reset link-local header on ipv6 recv path Pauli Virtanen
  2025-11-01 12:09 ` [PATCH 2/4] Bluetooth: 6lowpan: fix BDADDR_LE vs ADDR_LE_DEV address type confusion Pauli Virtanen
@ 2025-11-01 12:09 ` Pauli Virtanen
  2025-11-03  5:24   ` kernel test robot
  2025-11-01 12:09 ` [PATCH 4/4] Bluetooth: 6lowpan: add missing l2cap_chan_lock() Pauli Virtanen
  2025-11-01 15:52 ` [PATCH 1/4] Bluetooth: 6lowpan: reset link-local header on ipv6 recv path Paul Menzel
  3 siblings, 1 reply; 9+ messages in thread
From: Pauli Virtanen @ 2025-11-01 12:09 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: Pauli Virtanen, marcel, johan.hedberg, luiz.dentz,
	jukka.rissanen, linux-kernel

disconnect_all_peers() calls sleeping function (l2cap_chan_close) under
spinlock.  Holding the lock doesn't actually do any good -- we work on a
local copy of the list, and the lock doesn't protect against peer->chan
having already been freed.

Fix by taking refcounts of peer->chan instead.  Clean up the code and
old comments a bit.

Also take l2cap_chan_lock() which is required for l2cap_chan_close().

Log: (bluez 6lowpan-tester Client Connect - Disable)
------
BUG: sleeping function called from invalid context at kernel/locking/mutex.c:575
...
<TASK>
...
l2cap_send_disconn_req (net/bluetooth/l2cap_core.c:938 net/bluetooth/l2cap_core.c:1495)
...
? __pfx_l2cap_chan_close (net/bluetooth/l2cap_core.c:809)
do_enable_set (net/bluetooth/6lowpan.c:1048 net/bluetooth/6lowpan.c:1068)
------

Fixes: 90305829635d ("Bluetooth: 6lowpan: Converting rwlocks to use RCU")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/6lowpan.c | 78 ++++++++++++++++++++++++++---------------
 1 file changed, 50 insertions(+), 28 deletions(-)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 0d8c2e2e9a6c..f64bc4dc2b54 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -53,6 +53,11 @@ static bool enable_6lowpan;
 static struct l2cap_chan *listen_chan;
 static DEFINE_MUTEX(set_lock);
 
+enum {
+	LOWPAN_PEER_CLOSING,
+	LOWPAN_PEER_MAXBITS
+};
+
 struct lowpan_peer {
 	struct list_head list;
 	struct rcu_head rcu;
@@ -61,6 +66,8 @@ struct lowpan_peer {
 	/* peer addresses in various formats */
 	unsigned char lladdr[ETH_ALEN];
 	struct in6_addr peer_addr;
+
+	DECLARE_BITMAP(flags, LOWPAN_PEER_MAXBITS);
 };
 
 struct lowpan_btle_dev {
@@ -1014,41 +1021,56 @@ static int get_l2cap_conn(char *buf, bdaddr_t *addr, u8 *addr_type,
 static void disconnect_all_peers(void)
 {
 	struct lowpan_btle_dev *entry;
-	struct lowpan_peer *peer, *tmp_peer, *new_peer;
-	struct list_head peers;
+	struct lowpan_peer *peer;
+	int nchans;
 
-	INIT_LIST_HEAD(&peers);
-
-	/* We make a separate list of peers as the close_cb() will
-	 * modify the device peers list so it is better not to mess
-	 * with the same list at the same time.
+	/* l2cap_chan_close() cannot be called from RCU, and lock ordering
+	 * chan->lock > devices_lock prevents taking write side lock, so copy
+	 * then close.
 	 */
 
 	rcu_read_lock();
-
-	list_for_each_entry_rcu(entry, &bt_6lowpan_devices, list) {
-		list_for_each_entry_rcu(peer, &entry->peers, list) {
-			new_peer = kmalloc(sizeof(*new_peer), GFP_ATOMIC);
-			if (!new_peer)
-				break;
-
-			new_peer->chan = peer->chan;
-			INIT_LIST_HEAD(&new_peer->list);
-
-			list_add(&new_peer->list, &peers);
-		}
-	}
-
+	list_for_each_entry_rcu(entry, &bt_6lowpan_devices, list)
+		list_for_each_entry_rcu(peer, &entry->peers, list)
+			clear_bit(LOWPAN_PEER_CLOSING, peer->flags);
 	rcu_read_unlock();
 
-	spin_lock(&devices_lock);
-	list_for_each_entry_safe(peer, tmp_peer, &peers, list) {
-		l2cap_chan_close(peer->chan, ENOENT);
+	do {
+		struct l2cap_chan *chans[64];
+		int i;
 
-		list_del_rcu(&peer->list);
-		kfree_rcu(peer, rcu);
-	}
-	spin_unlock(&devices_lock);
+		nchans = 0;
+
+		rcu_read_lock();
+
+		list_for_each_entry_rcu(entry, &bt_6lowpan_devices, list) {
+			list_for_each_entry_rcu(peer, &entry->peers, list) {
+				struct l2cap_chan *chan;
+
+				if (test_and_set_bit(LOWPAN_PEER_CLOSING,
+						     peer->flags))
+					continue;
+
+				chan = l2cap_chan_hold_unless_zero(peer->chan);
+				if (!chan)
+					continue;
+
+				chans[nchans++] = chan;
+				if (nchans >= ARRAY_SIZE(chans))
+					goto done;
+			}
+		}
+
+done:
+		rcu_read_unlock();
+
+		for (i = 0; i < nchans; ++i) {
+			l2cap_chan_lock(chans[i]);
+			l2cap_chan_close(chans[i], ENOENT);
+			l2cap_chan_unlock(chans[i]);
+			l2cap_chan_put(chans[i]);
+		}
+	} while (nchans);
 }
 
 struct set_enable {
-- 
2.51.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/4] Bluetooth: 6lowpan: add missing l2cap_chan_lock()
  2025-11-01 12:09 [PATCH 1/4] Bluetooth: 6lowpan: reset link-local header on ipv6 recv path Pauli Virtanen
  2025-11-01 12:09 ` [PATCH 2/4] Bluetooth: 6lowpan: fix BDADDR_LE vs ADDR_LE_DEV address type confusion Pauli Virtanen
  2025-11-01 12:09 ` [PATCH 3/4] Bluetooth: 6lowpan: Don't hold spin lock over sleeping functions Pauli Virtanen
@ 2025-11-01 12:09 ` Pauli Virtanen
  2025-11-01 15:52 ` [PATCH 1/4] Bluetooth: 6lowpan: reset link-local header on ipv6 recv path Paul Menzel
  3 siblings, 0 replies; 9+ messages in thread
From: Pauli Virtanen @ 2025-11-01 12:09 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: Pauli Virtanen, marcel, johan.hedberg, luiz.dentz,
	jukka.rissanen, linux-kernel

l2cap_chan_close() needs to be called in l2cap_chan_lock(), otherwise
l2cap_le_sig_cmd() etc. may run concurrently.

Add missing locks around l2cap_chan_close().

Fixes: 6b8d4a6a0314 ("Bluetooth: 6LoWPAN: Use connected oriented channel instead of fixed one")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---

Notes:
    l2cap_chan_send() has same issue, but harder to fix so leave for later

 net/bluetooth/6lowpan.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index f64bc4dc2b54..cdd83f54677a 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -927,7 +927,9 @@ static int bt_6lowpan_disconnect(struct l2cap_conn *conn, u8 dst_type)
 
 	BT_DBG("peer %p chan %p", peer, peer->chan);
 
+	l2cap_chan_lock(peer->chan);
 	l2cap_chan_close(peer->chan, ENOENT);
+	l2cap_chan_unlock(peer->chan);
 
 	return 0;
 }
@@ -1093,7 +1095,9 @@ static void do_enable_set(struct work_struct *work)
 
 	mutex_lock(&set_lock);
 	if (listen_chan) {
+		l2cap_chan_lock(listen_chan);
 		l2cap_chan_close(listen_chan, 0);
+		l2cap_chan_unlock(listen_chan);
 		l2cap_chan_put(listen_chan);
 	}
 
@@ -1152,7 +1156,9 @@ static ssize_t lowpan_control_write(struct file *fp,
 
 		mutex_lock(&set_lock);
 		if (listen_chan) {
+			l2cap_chan_lock(listen_chan);
 			l2cap_chan_close(listen_chan, 0);
+			l2cap_chan_unlock(listen_chan);
 			l2cap_chan_put(listen_chan);
 			listen_chan = NULL;
 		}
@@ -1314,7 +1320,9 @@ static void __exit bt_6lowpan_exit(void)
 	debugfs_remove(lowpan_control_debugfs);
 
 	if (listen_chan) {
+		l2cap_chan_lock(listen_chan);
 		l2cap_chan_close(listen_chan, 0);
+		l2cap_chan_unlock(listen_chan);
 		l2cap_chan_put(listen_chan);
 	}
 
-- 
2.51.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] Bluetooth: 6lowpan: reset link-local header on ipv6 recv path
  2025-11-01 12:09 [PATCH 1/4] Bluetooth: 6lowpan: reset link-local header on ipv6 recv path Pauli Virtanen
                   ` (2 preceding siblings ...)
  2025-11-01 12:09 ` [PATCH 4/4] Bluetooth: 6lowpan: add missing l2cap_chan_lock() Pauli Virtanen
@ 2025-11-01 15:52 ` Paul Menzel
  2025-11-01 16:46   ` Paul Menzel
  3 siblings, 1 reply; 9+ messages in thread
From: Paul Menzel @ 2025-11-01 15:52 UTC (permalink / raw)
  To: Pauli Virtanen
  Cc: linux-bluetooth, marcel, johan.hedberg, luiz.dentz,
	jukka.rissanen, linux-kernel

Dear Pauli,


Thank you for your patch.

Am 01.11.25 um 13:09 schrieb Pauli Virtanen:
> Bluetooth 6lowpan.c netdev has header_ops, so it must set link-local
> header for RX skb, otherwise things crash, eg. with AF_PACKET SOCK_RAW
> 
> Add missing skb_reset_mac_header() for uncompressed ipv6 RX path.
> 
> For the compressed one, it is done in lowpan_header_decompress().
> 
> Log: (BlueZ 6lowpan-tester Client Recv Raw - Success)
> ------
> kernel BUG at net/core/skbuff.c:212!
> Call Trace:
> <IRQ>
> ...
> packet_rcv (net/packet/af_packet.c:2152)
> ...
> <TASK>
> __local_bh_enable_ip (kernel/softirq.c:407)
> netif_rx (net/core/dev.c:5648)
> chan_recv_cb (net/bluetooth/6lowpan.c:294 net/bluetooth/6lowpan.c:359)
> ------
> 
> Fixes: 18722c247023 ("Bluetooth: Enable 6LoWPAN support for BT LE devices")
> Signed-off-by: Pauli Virtanen <pav@iki.fi>
> ---
>   net/bluetooth/6lowpan.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> index f0c862091bff..f1d29fa4b411 100644
> --- a/net/bluetooth/6lowpan.c
> +++ b/net/bluetooth/6lowpan.c
> @@ -289,6 +289,7 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
>   		local_skb->pkt_type = PACKET_HOST;
>   		local_skb->dev = dev;
>   
> +		skb_reset_mac_header(local_skb);
>   		skb_set_transport_header(local_skb, sizeof(struct ipv6hdr));
>   
>   		if (give_skb_to_upper(local_skb, dev) != NET_RX_SUCCESS) {

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] Bluetooth: 6lowpan: fix BDADDR_LE vs ADDR_LE_DEV address type confusion
  2025-11-01 12:09 ` [PATCH 2/4] Bluetooth: 6lowpan: fix BDADDR_LE vs ADDR_LE_DEV address type confusion Pauli Virtanen
@ 2025-11-01 16:24   ` Paul Menzel
  2025-11-01 16:45     ` Paul Menzel
  0 siblings, 1 reply; 9+ messages in thread
From: Paul Menzel @ 2025-11-01 16:24 UTC (permalink / raw)
  To: Pauli Virtanen
  Cc: linux-bluetooth, marcel, johan.hedberg, luiz.dentz,
	jukka.rissanen, linux-kernel

Dear Pauli,


Thank you for your patch.

Am 01.11.25 um 13:09 schrieb Pauli Virtanen:
> Bluetooth 6lowpan.c confuses BDADDR_LE and ADDR_LE_DEV address types,
> e.g. debugfs "connect" command takes the former, and "disconnect" and
> "connect" to already connected device take the latter.  This is due to
> using same value both for l2cap_chan_connect and hci_conn_hash_lookup_le
> which take different dst_type values.
> 
> Fix address type passed to hci_conn_hash_lookup_le().
> 
> Retain the debugfs API difference between "connect" and "disconnect"
> commands since it's been like this since 2015 and nobody apparently
> complained.
> 
> Fixes: f5ad4ffceba0 ("Bluetooth: 6lowpan: Use hci_conn_hash_lookup_le() when possible")
> Signed-off-by: Pauli Virtanen <pav@iki.fi>
> ---
>   net/bluetooth/6lowpan.c | 28 ++++++++++++++++++++++++----
>   1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> index f1d29fa4b411..0d8c2e2e9a6c 100644
> --- a/net/bluetooth/6lowpan.c
> +++ b/net/bluetooth/6lowpan.c
> @@ -957,10 +957,11 @@ static struct l2cap_chan *bt_6lowpan_listen(void)
>   }
>   
>   static int get_l2cap_conn(char *buf, bdaddr_t *addr, u8 *addr_type,
> -			  struct l2cap_conn **conn)
> +			  struct l2cap_conn **conn, bool disconnect)
>   {
>   	struct hci_conn *hcon;
>   	struct hci_dev *hdev;
> +	int le_addr_type;
>   	int n;
>   
>   	n = sscanf(buf, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx %hhu",
> @@ -971,13 +972,32 @@ static int get_l2cap_conn(char *buf, bdaddr_t *addr, u8 *addr_type,
>   	if (n < 7)
>   		return -EINVAL;
>   
> +	if (disconnect) {
> +		/* The "disconnect" debugfs command has used different address
> +		 * type constants than "connect" since 2015. Let's retain that
> +		 * for now even though it's obviously buggy...
> +		 */
> +		*addr_type += 1;
> +	}
> +
> +	switch (*addr_type) {
> +	case BDADDR_LE_PUBLIC:
> +		le_addr_type = ADDR_LE_DEV_PUBLIC;
> +		break;
> +	case BDADDR_LE_RANDOM:
> +		le_addr_type = ADDR_LE_DEV_RANDOM;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
>   	/* The LE_PUBLIC address type is ignored because of BDADDR_ANY */
>   	hdev = hci_get_route(addr, BDADDR_ANY, BDADDR_LE_PUBLIC);
>   	if (!hdev)
>   		return -ENOENT;
>   
>   	hci_dev_lock(hdev);
> -	hcon = hci_conn_hash_lookup_le(hdev, addr, *addr_type);
> +	hcon = hci_conn_hash_lookup_le(hdev, addr, le_addr_type);
>   	hci_dev_unlock(hdev);
>   	hci_dev_put(hdev);
>   
> @@ -1104,7 +1124,7 @@ static ssize_t lowpan_control_write(struct file *fp,
>   	buf[buf_size] = '\0';
>   
>   	if (memcmp(buf, "connect ", 8) == 0) {
> -		ret = get_l2cap_conn(&buf[8], &addr, &addr_type, &conn);
> +		ret = get_l2cap_conn(&buf[8], &addr, &addr_type, &conn, false);
>   		if (ret == -EINVAL)
>   			return ret;
>   
> @@ -1141,7 +1161,7 @@ static ssize_t lowpan_control_write(struct file *fp,
>   	}
>   
>   	if (memcmp(buf, "disconnect ", 11) == 0) {
> -		ret = get_l2cap_conn(&buf[11], &addr, &addr_type, &conn);
> +		ret = get_l2cap_conn(&buf[11], &addr, &addr_type, &conn, true);
>   		if (ret < 0)
>   			return ret;
>   

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] Bluetooth: 6lowpan: fix BDADDR_LE vs ADDR_LE_DEV address type confusion
  2025-11-01 16:24   ` Paul Menzel
@ 2025-11-01 16:45     ` Paul Menzel
  0 siblings, 0 replies; 9+ messages in thread
From: Paul Menzel @ 2025-11-01 16:45 UTC (permalink / raw)
  To: Pauli Virtanen
  Cc: linux-bluetooth, marcel, johan.hedberg, luiz.dentz, linux-kernel

[Remove bouncing jukka.rissanen@linux.intel.com]

Am 01.11.25 um 17:24 schrieb Paul Menzel:
> Dear Pauli,
> 
> 
> Thank you for your patch.
> 
> Am 01.11.25 um 13:09 schrieb Pauli Virtanen:
>> Bluetooth 6lowpan.c confuses BDADDR_LE and ADDR_LE_DEV address types,
>> e.g. debugfs "connect" command takes the former, and "disconnect" and
>> "connect" to already connected device take the latter.  This is due to
>> using same value both for l2cap_chan_connect and hci_conn_hash_lookup_le
>> which take different dst_type values.
>>
>> Fix address type passed to hci_conn_hash_lookup_le().
>>
>> Retain the debugfs API difference between "connect" and "disconnect"
>> commands since it's been like this since 2015 and nobody apparently
>> complained.
>>
>> Fixes: f5ad4ffceba0 ("Bluetooth: 6lowpan: Use hci_conn_hash_lookup_le() when possible")
>> Signed-off-by: Pauli Virtanen <pav@iki.fi>
>> ---
>>   net/bluetooth/6lowpan.c | 28 ++++++++++++++++++++++++----
>>   1 file changed, 24 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
>> index f1d29fa4b411..0d8c2e2e9a6c 100644
>> --- a/net/bluetooth/6lowpan.c
>> +++ b/net/bluetooth/6lowpan.c
>> @@ -957,10 +957,11 @@ static struct l2cap_chan *bt_6lowpan_listen(void)
>>   }
>>   static int get_l2cap_conn(char *buf, bdaddr_t *addr, u8 *addr_type,
>> -              struct l2cap_conn **conn)
>> +              struct l2cap_conn **conn, bool disconnect)
>>   {
>>       struct hci_conn *hcon;
>>       struct hci_dev *hdev;
>> +    int le_addr_type;
>>       int n;
>>       n = sscanf(buf, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx %hhu",
>> @@ -971,13 +972,32 @@ static int get_l2cap_conn(char *buf, bdaddr_t *addr, u8 *addr_type,
>>       if (n < 7)
>>           return -EINVAL;
>> +    if (disconnect) {
>> +        /* The "disconnect" debugfs command has used different address
>> +         * type constants than "connect" since 2015. Let's retain that
>> +         * for now even though it's obviously buggy...
>> +         */
>> +        *addr_type += 1;
>> +    }
>> +
>> +    switch (*addr_type) {
>> +    case BDADDR_LE_PUBLIC:
>> +        le_addr_type = ADDR_LE_DEV_PUBLIC;
>> +        break;
>> +    case BDADDR_LE_RANDOM:
>> +        le_addr_type = ADDR_LE_DEV_RANDOM;
>> +        break;
>> +    default:
>> +        return -EINVAL;
>> +    }
>> +
>>       /* The LE_PUBLIC address type is ignored because of BDADDR_ANY */
>>       hdev = hci_get_route(addr, BDADDR_ANY, BDADDR_LE_PUBLIC);
>>       if (!hdev)
>>           return -ENOENT;
>>       hci_dev_lock(hdev);
>> -    hcon = hci_conn_hash_lookup_le(hdev, addr, *addr_type);
>> +    hcon = hci_conn_hash_lookup_le(hdev, addr, le_addr_type);
>>       hci_dev_unlock(hdev);
>>       hci_dev_put(hdev);
>> @@ -1104,7 +1124,7 @@ static ssize_t lowpan_control_write(struct file *fp,
>>       buf[buf_size] = '\0';
>>       if (memcmp(buf, "connect ", 8) == 0) {
>> -        ret = get_l2cap_conn(&buf[8], &addr, &addr_type, &conn);
>> +        ret = get_l2cap_conn(&buf[8], &addr, &addr_type, &conn, false);
>>           if (ret == -EINVAL)
>>               return ret;
>> @@ -1141,7 +1161,7 @@ static ssize_t lowpan_control_write(struct file 
>> *fp,
>>       }
>>       if (memcmp(buf, "disconnect ", 11) == 0) {
>> -        ret = get_l2cap_conn(&buf[11], &addr, &addr_type, &conn);
>> +        ret = get_l2cap_conn(&buf[11], &addr, &addr_type, &conn, true);
>>           if (ret < 0)
>>               return ret;
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> 
> 
> Kind regards,
> 
> Paul


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] Bluetooth: 6lowpan: reset link-local header on ipv6 recv path
  2025-11-01 15:52 ` [PATCH 1/4] Bluetooth: 6lowpan: reset link-local header on ipv6 recv path Paul Menzel
@ 2025-11-01 16:46   ` Paul Menzel
  0 siblings, 0 replies; 9+ messages in thread
From: Paul Menzel @ 2025-11-01 16:46 UTC (permalink / raw)
  To: Pauli Virtanen
  Cc: linux-bluetooth, marcel, johan.hedberg, luiz.dentz, linux-kernel

[Remove bouncing jukka.rissanen@linux.intel.com]

Am 01.11.25 um 16:52 schrieb Paul Menzel:
> Dear Pauli,
> 
> 
> Thank you for your patch.
> 
> Am 01.11.25 um 13:09 schrieb Pauli Virtanen:
>> Bluetooth 6lowpan.c netdev has header_ops, so it must set link-local
>> header for RX skb, otherwise things crash, eg. with AF_PACKET SOCK_RAW
>>
>> Add missing skb_reset_mac_header() for uncompressed ipv6 RX path.
>>
>> For the compressed one, it is done in lowpan_header_decompress().
>>
>> Log: (BlueZ 6lowpan-tester Client Recv Raw - Success)
>> ------
>> kernel BUG at net/core/skbuff.c:212!
>> Call Trace:
>> <IRQ>
>> ...
>> packet_rcv (net/packet/af_packet.c:2152)
>> ...
>> <TASK>
>> __local_bh_enable_ip (kernel/softirq.c:407)
>> netif_rx (net/core/dev.c:5648)
>> chan_recv_cb (net/bluetooth/6lowpan.c:294 net/bluetooth/6lowpan.c:359)
>> ------
>>
>> Fixes: 18722c247023 ("Bluetooth: Enable 6LoWPAN support for BT LE devices")
>> Signed-off-by: Pauli Virtanen <pav@iki.fi>
>> ---
>>   net/bluetooth/6lowpan.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
>> index f0c862091bff..f1d29fa4b411 100644
>> --- a/net/bluetooth/6lowpan.c
>> +++ b/net/bluetooth/6lowpan.c
>> @@ -289,6 +289,7 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
>>           local_skb->pkt_type = PACKET_HOST;
>>           local_skb->dev = dev;
>> +        skb_reset_mac_header(local_skb);
>>           skb_set_transport_header(local_skb, sizeof(struct ipv6hdr));
>>           if (give_skb_to_upper(local_skb, dev) != NET_RX_SUCCESS) {
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> 
> 
> Kind regards,
> 
> Paul


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] Bluetooth: 6lowpan: Don't hold spin lock over sleeping functions
  2025-11-01 12:09 ` [PATCH 3/4] Bluetooth: 6lowpan: Don't hold spin lock over sleeping functions Pauli Virtanen
@ 2025-11-03  5:24   ` kernel test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2025-11-03  5:24 UTC (permalink / raw)
  To: Pauli Virtanen, linux-bluetooth
  Cc: oe-kbuild-all, Pauli Virtanen, marcel, johan.hedberg, luiz.dentz,
	jukka.rissanen, linux-kernel

Hi Pauli,

kernel test robot noticed the following build errors:

[auto build test ERROR on bluetooth/master]
[also build test ERROR on bluetooth-next/master linus/master v6.18-rc4 next-20251031]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Pauli-Virtanen/Bluetooth-6lowpan-fix-BDADDR_LE-vs-ADDR_LE_DEV-address-type-confusion/20251101-201123
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git master
patch link:    https://lore.kernel.org/r/8736a4ce03f143b7a63cb99ab425e5403eafa9e4.1761998763.git.pav%40iki.fi
patch subject: [PATCH 3/4] Bluetooth: 6lowpan: Don't hold spin lock over sleeping functions
config: x86_64-randconfig-071-20251103 (https://download.01.org/0day-ci/archive/20251103/202511031234.Gw8GEsFK-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251103/202511031234.Gw8GEsFK-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511031234.Gw8GEsFK-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "l2cap_chan_hold_unless_zero" [net/bluetooth/bluetooth_6lowpan.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-11-03  5:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-01 12:09 [PATCH 1/4] Bluetooth: 6lowpan: reset link-local header on ipv6 recv path Pauli Virtanen
2025-11-01 12:09 ` [PATCH 2/4] Bluetooth: 6lowpan: fix BDADDR_LE vs ADDR_LE_DEV address type confusion Pauli Virtanen
2025-11-01 16:24   ` Paul Menzel
2025-11-01 16:45     ` Paul Menzel
2025-11-01 12:09 ` [PATCH 3/4] Bluetooth: 6lowpan: Don't hold spin lock over sleeping functions Pauli Virtanen
2025-11-03  5:24   ` kernel test robot
2025-11-01 12:09 ` [PATCH 4/4] Bluetooth: 6lowpan: add missing l2cap_chan_lock() Pauli Virtanen
2025-11-01 15:52 ` [PATCH 1/4] Bluetooth: 6lowpan: reset link-local header on ipv6 recv path Paul Menzel
2025-11-01 16:46   ` Paul Menzel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®