Skip to content

Commit 98454bc

Browse files
pvVudentz
authored andcommitted
Bluetooth: 6lowpan: Don't hold spin lock over sleeping functions
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. Take devices_lock instead of RCU, because the kfree_rcu(); l2cap_chan_put(); construct in chan_close_cb() does not guarantee peer->chan is necessarily valid in RCU. 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: 9030582 ("Bluetooth: 6lowpan: Converting rwlocks to use RCU") Signed-off-by: Pauli Virtanen <pav@iki.fi> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
1 parent e060088 commit 98454bc

1 file changed

Lines changed: 43 additions & 25 deletions

File tree

net/bluetooth/6lowpan.c

Lines changed: 43 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ static bool enable_6lowpan;
5353
static struct l2cap_chan *listen_chan;
5454
static DEFINE_MUTEX(set_lock);
5555

56+
enum {
57+
LOWPAN_PEER_CLOSING,
58+
LOWPAN_PEER_MAXBITS
59+
};
60+
5661
struct lowpan_peer {
5762
struct list_head list;
5863
struct rcu_head rcu;
@@ -61,6 +66,8 @@ struct lowpan_peer {
6166
/* peer addresses in various formats */
6267
unsigned char lladdr[ETH_ALEN];
6368
struct in6_addr peer_addr;
69+
70+
DECLARE_BITMAP(flags, LOWPAN_PEER_MAXBITS);
6471
};
6572

6673
struct lowpan_btle_dev {
@@ -1014,41 +1021,52 @@ static int get_l2cap_conn(char *buf, bdaddr_t *addr, u8 *addr_type,
10141021
static void disconnect_all_peers(void)
10151022
{
10161023
struct lowpan_btle_dev *entry;
1017-
struct lowpan_peer *peer, *tmp_peer, *new_peer;
1018-
struct list_head peers;
1019-
1020-
INIT_LIST_HEAD(&peers);
1024+
struct lowpan_peer *peer;
1025+
int nchans;
10211026

1022-
/* We make a separate list of peers as the close_cb() will
1023-
* modify the device peers list so it is better not to mess
1024-
* with the same list at the same time.
1027+
/* l2cap_chan_close() cannot be called from RCU, and lock ordering
1028+
* chan->lock > devices_lock prevents taking write side lock, so copy
1029+
* then close.
10251030
*/
10261031

10271032
rcu_read_lock();
1033+
list_for_each_entry_rcu(entry, &bt_6lowpan_devices, list)
1034+
list_for_each_entry_rcu(peer, &entry->peers, list)
1035+
clear_bit(LOWPAN_PEER_CLOSING, peer->flags);
1036+
rcu_read_unlock();
10281037

1029-
list_for_each_entry_rcu(entry, &bt_6lowpan_devices, list) {
1030-
list_for_each_entry_rcu(peer, &entry->peers, list) {
1031-
new_peer = kmalloc(sizeof(*new_peer), GFP_ATOMIC);
1032-
if (!new_peer)
1033-
break;
1038+
do {
1039+
struct l2cap_chan *chans[32];
1040+
int i;
10341041

1035-
new_peer->chan = peer->chan;
1036-
INIT_LIST_HEAD(&new_peer->list);
1042+
nchans = 0;
10371043

1038-
list_add(&new_peer->list, &peers);
1039-
}
1040-
}
1044+
spin_lock(&devices_lock);
10411045

1042-
rcu_read_unlock();
1046+
list_for_each_entry_rcu(entry, &bt_6lowpan_devices, list) {
1047+
list_for_each_entry_rcu(peer, &entry->peers, list) {
1048+
if (test_and_set_bit(LOWPAN_PEER_CLOSING,
1049+
peer->flags))
1050+
continue;
10431051

1044-
spin_lock(&devices_lock);
1045-
list_for_each_entry_safe(peer, tmp_peer, &peers, list) {
1046-
l2cap_chan_close(peer->chan, ENOENT);
1052+
l2cap_chan_hold(peer->chan);
1053+
chans[nchans++] = peer->chan;
10471054

1048-
list_del_rcu(&peer->list);
1049-
kfree_rcu(peer, rcu);
1050-
}
1051-
spin_unlock(&devices_lock);
1055+
if (nchans >= ARRAY_SIZE(chans))
1056+
goto done;
1057+
}
1058+
}
1059+
1060+
done:
1061+
spin_unlock(&devices_lock);
1062+
1063+
for (i = 0; i < nchans; ++i) {
1064+
l2cap_chan_lock(chans[i]);
1065+
l2cap_chan_close(chans[i], ENOENT);
1066+
l2cap_chan_unlock(chans[i]);
1067+
l2cap_chan_put(chans[i]);
1068+
}
1069+
} while (nchans);
10521070
}
10531071

10541072
struct set_enable {

0 commit comments

Comments
 (0)