From: Xiangyu Chen xiangyu.chen@windriver.com
Backport to fix CVE-2024-49951, the main fix is Bluetooth: MGMT: Fix possible crash on mgmt_index_removed
This required 1 extra commit to make sure the picks are clean: Bluetooth: hci_sync: Add helper functions to manipulate cmd_sync queue
Luiz Augusto von Dentz (2): Bluetooth: hci_sync: Add helper functions to manipulate cmd_sync queue Bluetooth: MGMT: Fix possible crash on mgmt_index_removed
include/net/bluetooth/hci_sync.h | 12 +++ net/bluetooth/hci_sync.c | 132 +++++++++++++++++++++++++++++-- net/bluetooth/mgmt.c | 23 +++--- 3 files changed, 150 insertions(+), 17 deletions(-)
From: Luiz Augusto von Dentz luiz.von.dentz@intel.com
[ Upstream commit 505ea2b295929e7be2b4e1bc86ee31cb7862fb01 ]
This adds functions to queue, dequeue and lookup into the cmd_sync list.
Signed-off-by: Luiz Augusto von Dentz luiz.von.dentz@intel.com Signed-off-by: Xiangyu Chen xiangyu.chen@windriver.com --- include/net/bluetooth/hci_sync.h | 12 +++ net/bluetooth/hci_sync.c | 132 +++++++++++++++++++++++++++++-- 2 files changed, 136 insertions(+), 8 deletions(-)
diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h index 7accd5ff0760..3a7658d66022 100644 --- a/include/net/bluetooth/hci_sync.h +++ b/include/net/bluetooth/hci_sync.h @@ -47,6 +47,18 @@ int hci_cmd_sync_submit(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, void *data, hci_cmd_sync_work_destroy_t destroy); int hci_cmd_sync_queue(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, void *data, hci_cmd_sync_work_destroy_t destroy); +struct hci_cmd_sync_work_entry * +hci_cmd_sync_lookup_entry(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, + void *data, hci_cmd_sync_work_destroy_t destroy); +int hci_cmd_sync_queue_once(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, + void *data, hci_cmd_sync_work_destroy_t destroy); +void hci_cmd_sync_cancel_entry(struct hci_dev *hdev, + struct hci_cmd_sync_work_entry *entry); +bool hci_cmd_sync_dequeue(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, + void *data, hci_cmd_sync_work_destroy_t destroy); +bool hci_cmd_sync_dequeue_once(struct hci_dev *hdev, + hci_cmd_sync_work_func_t func, void *data, + hci_cmd_sync_work_destroy_t destroy);
int hci_update_eir_sync(struct hci_dev *hdev); int hci_update_class_sync(struct hci_dev *hdev); diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 862ac5e1f4b4..b7a7b2afaa04 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -650,6 +650,17 @@ void hci_cmd_sync_init(struct hci_dev *hdev) INIT_DELAYED_WORK(&hdev->adv_instance_expire, adv_timeout_expire); }
+static void _hci_cmd_sync_cancel_entry(struct hci_dev *hdev, + struct hci_cmd_sync_work_entry *entry, + int err) +{ + if (entry->destroy) + entry->destroy(hdev, entry->data, err); + + list_del(&entry->list); + kfree(entry); +} + void hci_cmd_sync_clear(struct hci_dev *hdev) { struct hci_cmd_sync_work_entry *entry, *tmp; @@ -658,13 +669,8 @@ void hci_cmd_sync_clear(struct hci_dev *hdev) cancel_work_sync(&hdev->reenable_adv_work);
mutex_lock(&hdev->cmd_sync_work_lock); - list_for_each_entry_safe(entry, tmp, &hdev->cmd_sync_work_list, list) { - if (entry->destroy) - entry->destroy(hdev, entry->data, -ECANCELED); - - list_del(&entry->list); - kfree(entry); - } + list_for_each_entry_safe(entry, tmp, &hdev->cmd_sync_work_list, list) + _hci_cmd_sync_cancel_entry(hdev, entry, -ECANCELED); mutex_unlock(&hdev->cmd_sync_work_lock); }
@@ -756,6 +762,115 @@ int hci_cmd_sync_queue(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, } EXPORT_SYMBOL(hci_cmd_sync_queue);
+static struct hci_cmd_sync_work_entry * +_hci_cmd_sync_lookup_entry(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, + void *data, hci_cmd_sync_work_destroy_t destroy) +{ + struct hci_cmd_sync_work_entry *entry, *tmp; + + list_for_each_entry_safe(entry, tmp, &hdev->cmd_sync_work_list, list) { + if (func && entry->func != func) + continue; + + if (data && entry->data != data) + continue; + + if (destroy && entry->destroy != destroy) + continue; + + return entry; + } + + return NULL; +} + +/* Queue HCI command entry once: + * + * - Lookup if an entry already exist and only if it doesn't creates a new entry + * and queue it. + */ +int hci_cmd_sync_queue_once(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, + void *data, hci_cmd_sync_work_destroy_t destroy) +{ + if (hci_cmd_sync_lookup_entry(hdev, func, data, destroy)) + return 0; + + return hci_cmd_sync_queue(hdev, func, data, destroy); +} +EXPORT_SYMBOL(hci_cmd_sync_queue_once); + +/* Lookup HCI command entry: + * + * - Return first entry that matches by function callback or data or + * destroy callback. + */ +struct hci_cmd_sync_work_entry * +hci_cmd_sync_lookup_entry(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, + void *data, hci_cmd_sync_work_destroy_t destroy) +{ + struct hci_cmd_sync_work_entry *entry; + + mutex_lock(&hdev->cmd_sync_work_lock); + entry = _hci_cmd_sync_lookup_entry(hdev, func, data, destroy); + mutex_unlock(&hdev->cmd_sync_work_lock); + + return entry; +} +EXPORT_SYMBOL(hci_cmd_sync_lookup_entry); + +/* Cancel HCI command entry */ +void hci_cmd_sync_cancel_entry(struct hci_dev *hdev, + struct hci_cmd_sync_work_entry *entry) +{ + mutex_lock(&hdev->cmd_sync_work_lock); + _hci_cmd_sync_cancel_entry(hdev, entry, -ECANCELED); + mutex_unlock(&hdev->cmd_sync_work_lock); +} +EXPORT_SYMBOL(hci_cmd_sync_cancel_entry); + +/* Dequeue one HCI command entry: + * + * - Lookup and cancel first entry that matches. + */ +bool hci_cmd_sync_dequeue_once(struct hci_dev *hdev, + hci_cmd_sync_work_func_t func, + void *data, hci_cmd_sync_work_destroy_t destroy) +{ + struct hci_cmd_sync_work_entry *entry; + + entry = hci_cmd_sync_lookup_entry(hdev, func, data, destroy); + if (!entry) + return false; + + hci_cmd_sync_cancel_entry(hdev, entry); + + return true; +} +EXPORT_SYMBOL(hci_cmd_sync_dequeue_once); + +/* Dequeue HCI command entry: + * + * - Lookup and cancel any entry that matches by function callback or data or + * destroy callback. + */ +bool hci_cmd_sync_dequeue(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, + void *data, hci_cmd_sync_work_destroy_t destroy) +{ + struct hci_cmd_sync_work_entry *entry; + bool ret = false; + + mutex_lock(&hdev->cmd_sync_work_lock); + while ((entry = _hci_cmd_sync_lookup_entry(hdev, func, data, + destroy))) { + _hci_cmd_sync_cancel_entry(hdev, entry, -ECANCELED); + ret = true; + } + mutex_unlock(&hdev->cmd_sync_work_lock); + + return ret; +} +EXPORT_SYMBOL(hci_cmd_sync_dequeue); + int hci_update_eir_sync(struct hci_dev *hdev) { struct hci_cp_write_eir cp; @@ -3023,7 +3138,8 @@ int hci_update_passive_scan(struct hci_dev *hdev) hci_dev_test_flag(hdev, HCI_UNREGISTER)) return 0;
- return hci_cmd_sync_queue(hdev, update_passive_scan_sync, NULL, NULL); + return hci_cmd_sync_queue_once(hdev, update_passive_scan_sync, NULL, + NULL); }
int hci_write_sc_support_sync(struct hci_dev *hdev, u8 val)
[ Sasha's backport helper bot ]
Hi,
The upstream commit SHA1 provided is correct: 505ea2b295929e7be2b4e1bc86ee31cb7862fb01
WARNING: Author mismatch between patch and upstream commit: Backport author: Xiangyu Chen xiangyu.chen@eng.windriver.com Commit author: Luiz Augusto von Dentz luiz.von.dentz@intel.com
Status in newer kernel trees: 6.12.y | Present (exact SHA1) 6.11.y | Present (exact SHA1) 6.6.y | Present (different SHA1: 1499f79995c7) 6.1.y | Not found
Note: The patch differs from the upstream commit: --- --- - 2024-11-26 08:03:56.550627469 -0500 +++ /tmp/tmp.7ZWCalBDha 2024-11-26 08:03:56.544829170 -0500 @@ -1,17 +1,20 @@ +[ Upstream commit 505ea2b295929e7be2b4e1bc86ee31cb7862fb01 ] + This adds functions to queue, dequeue and lookup into the cmd_sync list.
Signed-off-by: Luiz Augusto von Dentz luiz.von.dentz@intel.com +Signed-off-by: Xiangyu Chen xiangyu.chen@windriver.com --- include/net/bluetooth/hci_sync.h | 12 +++ net/bluetooth/hci_sync.c | 132 +++++++++++++++++++++++++++++-- 2 files changed, 136 insertions(+), 8 deletions(-)
diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h -index ed334c253ebcd..4ff4aa68ee196 100644 +index 7accd5ff0760..3a7658d66022 100644 --- a/include/net/bluetooth/hci_sync.h +++ b/include/net/bluetooth/hci_sync.h -@@ -48,6 +48,18 @@ int hci_cmd_sync_submit(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, +@@ -47,6 +47,18 @@ int hci_cmd_sync_submit(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, void *data, hci_cmd_sync_work_destroy_t destroy); int hci_cmd_sync_queue(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, void *data, hci_cmd_sync_work_destroy_t destroy); @@ -31,10 +34,10 @@ int hci_update_eir_sync(struct hci_dev *hdev); int hci_update_class_sync(struct hci_dev *hdev); diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c -index e1fdcb3c27062..5b314bf844f84 100644 +index 862ac5e1f4b4..b7a7b2afaa04 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c -@@ -566,6 +566,17 @@ void hci_cmd_sync_init(struct hci_dev *hdev) +@@ -650,6 +650,17 @@ void hci_cmd_sync_init(struct hci_dev *hdev) INIT_DELAYED_WORK(&hdev->adv_instance_expire, adv_timeout_expire); }
@@ -52,7 +55,7 @@ void hci_cmd_sync_clear(struct hci_dev *hdev) { struct hci_cmd_sync_work_entry *entry, *tmp; -@@ -574,13 +585,8 @@ void hci_cmd_sync_clear(struct hci_dev *hdev) +@@ -658,13 +669,8 @@ void hci_cmd_sync_clear(struct hci_dev *hdev) cancel_work_sync(&hdev->reenable_adv_work);
mutex_lock(&hdev->cmd_sync_work_lock); @@ -68,7 +71,7 @@ mutex_unlock(&hdev->cmd_sync_work_lock); }
-@@ -669,6 +675,115 @@ int hci_cmd_sync_queue(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, +@@ -756,6 +762,115 @@ int hci_cmd_sync_queue(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, } EXPORT_SYMBOL(hci_cmd_sync_queue);
@@ -184,7 +187,7 @@ int hci_update_eir_sync(struct hci_dev *hdev) { struct hci_cp_write_eir cp; -@@ -2881,7 +2996,8 @@ int hci_update_passive_scan(struct hci_dev *hdev) +@@ -3023,7 +3138,8 @@ int hci_update_passive_scan(struct hci_dev *hdev) hci_dev_test_flag(hdev, HCI_UNREGISTER)) return 0;
@@ -194,3 +197,6 @@ }
int hci_write_sc_support_sync(struct hci_dev *hdev, u8 val) +-- +2.43.0 + ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.1.y | Success | Success |
From: Luiz Augusto von Dentz luiz.von.dentz@intel.com
[ Upstream commit f53e1c9c726d83092167f2226f32bd3b73f26c21 ]
If mgmt_index_removed is called while there are commands queued on cmd_sync it could lead to crashes like the bellow trace:
0x0000053D: __list_del_entry_valid_or_report+0x98/0xdc 0x0000053D: mgmt_pending_remove+0x18/0x58 [bluetooth] 0x0000053E: mgmt_remove_adv_monitor_complete+0x80/0x108 [bluetooth] 0x0000053E: hci_cmd_sync_work+0xbc/0x164 [bluetooth]
So while handling mgmt_index_removed this attempts to dequeue commands passed as user_data to cmd_sync.
Fixes: 7cf5c2978f23 ("Bluetooth: hci_sync: Refactor remove Adv Monitor") Reported-by: jiaymao quic_jiaymao@quicinc.com Signed-off-by: Luiz Augusto von Dentz luiz.von.dentz@intel.com [Xiangyu: BP to fix CVE: CVE-2024-49951, Minor conflict resolution] Signed-off-by: Xiangyu Chen xiangyu.chen@windriver.com --- net/bluetooth/mgmt.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 5a1015ccc063..82edd9981ab0 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -1457,10 +1457,15 @@ static void cmd_status_rsp(struct mgmt_pending_cmd *cmd, void *data)
static void cmd_complete_rsp(struct mgmt_pending_cmd *cmd, void *data) { - if (cmd->cmd_complete) { - u8 *status = data; + struct cmd_lookup *match = data; + + /* dequeue cmd_sync entries using cmd as data as that is about to be + * removed/freed. + */ + hci_cmd_sync_dequeue(match->hdev, NULL, cmd, NULL);
- cmd->cmd_complete(cmd, *status); + if (cmd->cmd_complete) { + cmd->cmd_complete(cmd, match->mgmt_status); mgmt_pending_remove(cmd);
return; @@ -9424,14 +9429,14 @@ void mgmt_index_added(struct hci_dev *hdev) void mgmt_index_removed(struct hci_dev *hdev) { struct mgmt_ev_ext_index ev; - u8 status = MGMT_STATUS_INVALID_INDEX; + struct cmd_lookup match = { NULL, hdev, MGMT_STATUS_INVALID_INDEX };
if (test_bit(HCI_QUIRK_RAW_DEVICE, &hdev->quirks)) return;
switch (hdev->dev_type) { case HCI_PRIMARY: - mgmt_pending_foreach(0, hdev, cmd_complete_rsp, &status); + mgmt_pending_foreach(0, hdev, cmd_complete_rsp, &match);
if (hci_dev_test_flag(hdev, HCI_UNCONFIGURED)) { mgmt_index_event(MGMT_EV_UNCONF_INDEX_REMOVED, hdev, @@ -9489,7 +9494,7 @@ void mgmt_power_on(struct hci_dev *hdev, int err) void __mgmt_power_off(struct hci_dev *hdev) { struct cmd_lookup match = { NULL, hdev }; - u8 status, zero_cod[] = { 0, 0, 0 }; + u8 zero_cod[] = { 0, 0, 0 };
mgmt_pending_foreach(MGMT_OP_SET_POWERED, hdev, settings_rsp, &match);
@@ -9501,11 +9506,11 @@ void __mgmt_power_off(struct hci_dev *hdev) * status responses. */ if (hci_dev_test_flag(hdev, HCI_UNREGISTER)) - status = MGMT_STATUS_INVALID_INDEX; + match.mgmt_status = MGMT_STATUS_INVALID_INDEX; else - status = MGMT_STATUS_NOT_POWERED; + match.mgmt_status = MGMT_STATUS_NOT_POWERED;
- mgmt_pending_foreach(0, hdev, cmd_complete_rsp, &status); + mgmt_pending_foreach(0, hdev, cmd_complete_rsp, &match);
if (memcmp(hdev->dev_class, zero_cod, sizeof(zero_cod)) != 0) { mgmt_limited_event(MGMT_EV_CLASS_OF_DEV_CHANGED, hdev,
[ Sasha's backport helper bot ]
Hi,
The upstream commit SHA1 provided is correct: f53e1c9c726d83092167f2226f32bd3b73f26c21
WARNING: Author mismatch between patch and upstream commit: Backport author: Xiangyu Chen xiangyu.chen@eng.windriver.com Commit author: Luiz Augusto von Dentz luiz.von.dentz@intel.com
Status in newer kernel trees: 6.12.y | Present (exact SHA1) 6.11.y | Present (different SHA1: 8c3f7943a291) 6.6.y | Present (different SHA1: 4883296505aa) 6.1.y | Not found
Note: The patch differs from the upstream commit: --- --- - 2024-11-26 08:07:32.599317773 -0500 +++ /tmp/tmp.XHyzHU5ddN 2024-11-26 08:07:32.593820712 -0500 @@ -1,3 +1,5 @@ +[ Upstream commit f53e1c9c726d83092167f2226f32bd3b73f26c21 ] + If mgmt_index_removed is called while there are commands queued on cmd_sync it could lead to crashes like the bellow trace:
@@ -12,15 +14,17 @@ Fixes: 7cf5c2978f23 ("Bluetooth: hci_sync: Refactor remove Adv Monitor") Reported-by: jiaymao quic_jiaymao@quicinc.com Signed-off-by: Luiz Augusto von Dentz luiz.von.dentz@intel.com +[Xiangyu: BP to fix CVE: CVE-2024-49951, Minor conflict resolution] +Signed-off-by: Xiangyu Chen xiangyu.chen@windriver.com --- net/bluetooth/mgmt.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c -index e4f564d6f6fbf..4157d9f23f46e 100644 +index 5a1015ccc063..82edd9981ab0 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c -@@ -1453,10 +1453,15 @@ static void cmd_status_rsp(struct mgmt_pending_cmd *cmd, void *data) +@@ -1457,10 +1457,15 @@ static void cmd_status_rsp(struct mgmt_pending_cmd *cmd, void *data)
static void cmd_complete_rsp(struct mgmt_pending_cmd *cmd, void *data) { @@ -39,7 +43,7 @@ mgmt_pending_remove(cmd);
return; -@@ -9394,12 +9399,12 @@ void mgmt_index_added(struct hci_dev *hdev) +@@ -9424,14 +9429,14 @@ void mgmt_index_added(struct hci_dev *hdev) void mgmt_index_removed(struct hci_dev *hdev) { struct mgmt_ev_ext_index ev; @@ -49,12 +53,14 @@ if (test_bit(HCI_QUIRK_RAW_DEVICE, &hdev->quirks)) return;
-- mgmt_pending_foreach(0, hdev, cmd_complete_rsp, &status); -+ mgmt_pending_foreach(0, hdev, cmd_complete_rsp, &match); - - if (hci_dev_test_flag(hdev, HCI_UNCONFIGURED)) { - mgmt_index_event(MGMT_EV_UNCONF_INDEX_REMOVED, hdev, NULL, 0, -@@ -9450,7 +9455,7 @@ void mgmt_power_on(struct hci_dev *hdev, int err) + switch (hdev->dev_type) { + case HCI_PRIMARY: +- mgmt_pending_foreach(0, hdev, cmd_complete_rsp, &status); ++ mgmt_pending_foreach(0, hdev, cmd_complete_rsp, &match); + + if (hci_dev_test_flag(hdev, HCI_UNCONFIGURED)) { + mgmt_index_event(MGMT_EV_UNCONF_INDEX_REMOVED, hdev, +@@ -9489,7 +9494,7 @@ void mgmt_power_on(struct hci_dev *hdev, int err) void __mgmt_power_off(struct hci_dev *hdev) { struct cmd_lookup match = { NULL, hdev }; @@ -63,7 +69,7 @@
mgmt_pending_foreach(MGMT_OP_SET_POWERED, hdev, settings_rsp, &match);
-@@ -9462,11 +9467,11 @@ void __mgmt_power_off(struct hci_dev *hdev) +@@ -9501,11 +9506,11 @@ void __mgmt_power_off(struct hci_dev *hdev) * status responses. */ if (hci_dev_test_flag(hdev, HCI_UNREGISTER)) @@ -78,3 +84,6 @@
if (memcmp(hdev->dev_class, zero_cod, sizeof(zero_cod)) != 0) { mgmt_limited_event(MGMT_EV_CLASS_OF_DEV_CHANGED, hdev, +-- +2.43.0 + ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.1.y | Success | Success |
linux-stable-mirror@lists.linaro.org