Skip to content

Commit 75d42b3

Browse files
kuba-mookuba-moo
committed
Merge tag 'for-net-2023-07-20' of git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth
Luiz Augusto von Dentz says: ==================== bluetooth pull request for net: - Fix building with coredump disabled - Fix use-after-free in hci_remove_adv_monitor - Use RCU for hci_conn_params and iterate safely in hci_sync - Fix locking issues on ISO and SCO - Fix bluetooth on Intel Macbook 2014 * tag 'for-net-2023-07-20' of git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth: Bluetooth: MGMT: Use correct address for memcpy() Bluetooth: btusb: Fix bluetooth on Intel Macbook 2014 Bluetooth: SCO: fix sco_conn related locking and validity issues Bluetooth: hci_conn: return ERR_PTR instead of NULL when there is no link Bluetooth: hci_sync: Avoid use-after-free in dbg for hci_remove_adv_monitor() Bluetooth: coredump: fix building with coredump disabled Bluetooth: ISO: fix iso_conn related locking and validity issues Bluetooth: hci_event: call disconnect callback before deleting conn Bluetooth: use RCU for hci_conn_params and iterate safely in hci_sync ==================== Link: https://lore.kernel.org/r/20230720190201.446469-1-luiz.dentz@gmail.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2 parents 9b39f75 + d1f0a98 commit 75d42b3

File tree

9 files changed

+217
-83
lines changed

9 files changed

+217
-83
lines changed

‎drivers/bluetooth/btusb.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4104,6 +4104,7 @@ static int btusb_probe(struct usb_interface *intf,
41044104
BT_DBG("intf %p id %p", intf, id);
41054105

41064106
if ((id->driver_info & BTUSB_IFNUM_2) &&
4107+
(intf->cur_altsetting->desc.bInterfaceNumber != 0) &&
41074108
(intf->cur_altsetting->desc.bInterfaceNumber != 2))
41084109
return -ENODEV;
41094110

‎include/net/bluetooth/hci_core.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -593,9 +593,7 @@ struct hci_dev {
593593
const char *fw_info;
594594
struct dentry *debugfs;
595595

596-
#ifdef CONFIG_DEV_COREDUMP
597596
struct hci_devcoredump dump;
598-
#endif
599597

600598
struct device dev;
601599

@@ -822,6 +820,7 @@ struct hci_conn_params {
822820

823821
struct hci_conn *conn;
824822
bool explicit_connect;
823+
/* Accessed without hdev->lock: */
825824
hci_conn_flags_t flags;
826825
u8 privacy_mode;
827826
};
@@ -1573,7 +1572,11 @@ struct hci_conn_params *hci_conn_params_add(struct hci_dev *hdev,
15731572
bdaddr_t *addr, u8 addr_type);
15741573
void hci_conn_params_del(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type);
15751574
void hci_conn_params_clear_disabled(struct hci_dev *hdev);
1575+
void hci_conn_params_free(struct hci_conn_params *param);
15761576

1577+
void hci_pend_le_list_del_init(struct hci_conn_params *param);
1578+
void hci_pend_le_list_add(struct hci_conn_params *param,
1579+
struct list_head *list);
15771580
struct hci_conn_params *hci_pend_le_action_lookup(struct list_head *list,
15781581
bdaddr_t *addr,
15791582
u8 addr_type);

‎net/bluetooth/hci_conn.c‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ static void hci_connect_le_scan_cleanup(struct hci_conn *conn, u8 status)
118118
*/
119119
params->explicit_connect = false;
120120

121-
list_del_init(&params->action);
121+
hci_pend_le_list_del_init(params);
122122

123123
switch (params->auto_connect) {
124124
case HCI_AUTO_CONN_EXPLICIT:
@@ -127,10 +127,10 @@ static void hci_connect_le_scan_cleanup(struct hci_conn *conn, u8 status)
127127
return;
128128
case HCI_AUTO_CONN_DIRECT:
129129
case HCI_AUTO_CONN_ALWAYS:
130-
list_add(&params->action, &hdev->pend_le_conns);
130+
hci_pend_le_list_add(params, &hdev->pend_le_conns);
131131
break;
132132
case HCI_AUTO_CONN_REPORT:
133-
list_add(&params->action, &hdev->pend_le_reports);
133+
hci_pend_le_list_add(params, &hdev->pend_le_reports);
134134
break;
135135
default:
136136
break;
@@ -1426,8 +1426,8 @@ static int hci_explicit_conn_params_set(struct hci_dev *hdev,
14261426
if (params->auto_connect == HCI_AUTO_CONN_DISABLED ||
14271427
params->auto_connect == HCI_AUTO_CONN_REPORT ||
14281428
params->auto_connect == HCI_AUTO_CONN_EXPLICIT) {
1429-
list_del_init(&params->action);
1430-
list_add(&params->action, &hdev->pend_le_conns);
1429+
hci_pend_le_list_del_init(params);
1430+
hci_pend_le_list_add(params, &hdev->pend_le_conns);
14311431
}
14321432

14331433
params->explicit_connect = true;
@@ -1684,7 +1684,7 @@ struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
16841684
if (!link) {
16851685
hci_conn_drop(acl);
16861686
hci_conn_drop(sco);
1687-
return NULL;
1687+
return ERR_PTR(-ENOLINK);
16881688
}
16891689

16901690
sco->setting = setting;
@@ -2254,7 +2254,7 @@ struct hci_conn *hci_connect_cis(struct hci_dev *hdev, bdaddr_t *dst,
22542254
if (!link) {
22552255
hci_conn_drop(le);
22562256
hci_conn_drop(cis);
2257-
return NULL;
2257+
return ERR_PTR(-ENOLINK);
22582258
}
22592259

22602260
/* If LE is already connected and CIS handle is already set proceed to

‎net/bluetooth/hci_core.c‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1972,6 +1972,7 @@ static int hci_remove_adv_monitor(struct hci_dev *hdev,
19721972
struct adv_monitor *monitor)
19731973
{
19741974
int status = 0;
1975+
int handle;
19751976

19761977
switch (hci_get_adv_monitor_offload_ext(hdev)) {
19771978
case HCI_ADV_MONITOR_EXT_NONE: /* also goes here when powered off */
@@ -1980,9 +1981,10 @@ static int hci_remove_adv_monitor(struct hci_dev *hdev,
19801981
goto free_monitor;
19811982

19821983
case HCI_ADV_MONITOR_EXT_MSFT:
1984+
handle = monitor->handle;
19831985
status = msft_remove_monitor(hdev, monitor);
19841986
bt_dev_dbg(hdev, "%s remove monitor %d msft status %d",
1985-
hdev->name, monitor->handle, status);
1987+
hdev->name, handle, status);
19861988
break;
19871989
}
19881990

@@ -2249,21 +2251,45 @@ struct hci_conn_params *hci_conn_params_lookup(struct hci_dev *hdev,
22492251
return NULL;
22502252
}
22512253

2252-
/* This function requires the caller holds hdev->lock */
2254+
/* This function requires the caller holds hdev->lock or rcu_read_lock */
22532255
struct hci_conn_params *hci_pend_le_action_lookup(struct list_head *list,
22542256
bdaddr_t *addr, u8 addr_type)
22552257
{
22562258
struct hci_conn_params *param;
22572259

2258-
list_for_each_entry(param, list, action) {
2260+
rcu_read_lock();
2261+
2262+
list_for_each_entry_rcu(param, list, action) {
22592263
if (bacmp(&param->addr, addr) == 0 &&
2260-
param->addr_type == addr_type)
2264+
param->addr_type == addr_type) {
2265+
rcu_read_unlock();
22612266
return param;
2267+
}
22622268
}
22632269

2270+
rcu_read_unlock();
2271+
22642272
return NULL;
22652273
}
22662274

2275+
/* This function requires the caller holds hdev->lock */
2276+
void hci_pend_le_list_del_init(struct hci_conn_params *param)
2277+
{
2278+
if (list_empty(&param->action))
2279+
return;
2280+
2281+
list_del_rcu(&param->action);
2282+
synchronize_rcu();
2283+
INIT_LIST_HEAD(&param->action);
2284+
}
2285+
2286+
/* This function requires the caller holds hdev->lock */
2287+
void hci_pend_le_list_add(struct hci_conn_params *param,
2288+
struct list_head *list)
2289+
{
2290+
list_add_rcu(&param->action, list);
2291+
}
2292+
22672293
/* This function requires the caller holds hdev->lock */
22682294
struct hci_conn_params *hci_conn_params_add(struct hci_dev *hdev,
22692295
bdaddr_t *addr, u8 addr_type)
@@ -2297,14 +2323,15 @@ struct hci_conn_params *hci_conn_params_add(struct hci_dev *hdev,
22972323
return params;
22982324
}
22992325

2300-
static void hci_conn_params_free(struct hci_conn_params *params)
2326+
void hci_conn_params_free(struct hci_conn_params *params)
23012327
{
2328+
hci_pend_le_list_del_init(params);
2329+
23022330
if (params->conn) {
23032331
hci_conn_drop(params->conn);
23042332
hci_conn_put(params->conn);
23052333
}
23062334

2307-
list_del(&params->action);
23082335
list_del(&params->list);
23092336
kfree(params);
23102337
}
@@ -2342,8 +2369,7 @@ void hci_conn_params_clear_disabled(struct hci_dev *hdev)
23422369
continue;
23432370
}
23442371

2345-
list_del(&params->list);
2346-
kfree(params);
2372+
hci_conn_params_free(params);
23472373
}
23482374

23492375
BT_DBG("All LE disabled connection parameters were removed");

‎net/bluetooth/hci_event.c‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1564,7 +1564,7 @@ static u8 hci_cc_le_set_privacy_mode(struct hci_dev *hdev, void *data,
15641564

15651565
params = hci_conn_params_lookup(hdev, &cp->bdaddr, cp->bdaddr_type);
15661566
if (params)
1567-
params->privacy_mode = cp->mode;
1567+
WRITE_ONCE(params->privacy_mode, cp->mode);
15681568

15691569
hci_dev_unlock(hdev);
15701570

@@ -2784,6 +2784,9 @@ static void hci_cs_disconnect(struct hci_dev *hdev, u8 status)
27842784
hci_enable_advertising(hdev);
27852785
}
27862786

2787+
/* Inform sockets conn is gone before we delete it */
2788+
hci_disconn_cfm(conn, HCI_ERROR_UNSPECIFIED);
2789+
27872790
goto done;
27882791
}
27892792

@@ -2804,8 +2807,8 @@ static void hci_cs_disconnect(struct hci_dev *hdev, u8 status)
28042807

28052808
case HCI_AUTO_CONN_DIRECT:
28062809
case HCI_AUTO_CONN_ALWAYS:
2807-
list_del_init(&params->action);
2808-
list_add(&params->action, &hdev->pend_le_conns);
2810+
hci_pend_le_list_del_init(params);
2811+
hci_pend_le_list_add(params, &hdev->pend_le_conns);
28092812
break;
28102813

28112814
default:
@@ -3423,8 +3426,8 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, void *data,
34233426

34243427
case HCI_AUTO_CONN_DIRECT:
34253428
case HCI_AUTO_CONN_ALWAYS:
3426-
list_del_init(&params->action);
3427-
list_add(&params->action, &hdev->pend_le_conns);
3429+
hci_pend_le_list_del_init(params);
3430+
hci_pend_le_list_add(params, &hdev->pend_le_conns);
34283431
hci_update_passive_scan(hdev);
34293432
break;
34303433

@@ -5962,7 +5965,7 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
59625965
params = hci_pend_le_action_lookup(&hdev->pend_le_conns, &conn->dst,
59635966
conn->dst_type);
59645967
if (params) {
5965-
list_del_init(&params->action);
5968+
hci_pend_le_list_del_init(params);
59665969
if (params->conn) {
59675970
hci_conn_drop(params->conn);
59685971
hci_conn_put(params->conn);

0 commit comments

Comments
 (0)