mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-08-31 10:31:33 -04:00
Bluetooth: mgmt: fix 'hdev->discovery.uuids' NULL dereference
'uuid_count' member of struct 'discovery_state' is assigned and read
without any locks, so there is a chance of situation when
uuid_count != 0, but uuids is NULL and there will be NULL pointer
dereference.
Possible race:
'hci_update_passive_scan_sync'
'hci_discovery_filter_clear'
hdev->discovery.uuid_count = 0;
<----------------------preempted----------------------------->
'start_service_discovery'
// Set uuid_count to value != 0
hdev->discovery.uuid_count = uuid_count;
hdev->discovery.uuids = kmemdup(...);
<----------------------preempted----------------------------->
spin_lock(&hdev->discovery.lock);
kfree(hdev->discovery.uuids);
hdev->discovery.uuids = NULL;
spin_unlock(&hdev->discovery.lock);
Now uuids == NULL and uuid_count != 0.
So 'mgmt_device_found' -> 'is_filter_match' -> 'eir_has_uuids' receives
non consistent discovery state, where NULL dereference of uuids happens.
To fix it let's add discovery.lock around every read/write of uuid_count,
uuids pair of struct members. It is also important to assign uuid_count
value only after success kmemdup() allocation in
start_service_discovery(), otherwise uuids is NULL, because kmemdup failed,
but uuid_count is already assigned to non zero value.
The following panic happens:
[ ] ------------[ cut here ]------------
[ ] Unable to handle kernel NULL pointer dereference at virtual
address 0000000000000000
[ ] Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP
[ ] CPU: 0 PID: 15056 Comm: kworker/u9:2
[ ] Workqueue: hci0 hci_rx_work
[ ] pstate: 10400009 (nzcV daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ ] pc : eir_has_uuids+0x2d8/0x590
[ ] lr : is_filter_match+0x258/0x320
...
[ ] Call trace:
[ ] eir_has_uuids+0x2d8/0x590
[ ] is_filter_match+0x258/0x320
[ ] mgmt_device_found+0x5b0/0xafc
[ ] process_adv_report.part.0+0x8c8/0xf14
[ ] hci_le_adv_report_evt+0x338/0x3f0
[ ] hci_le_meta_evt+0x1f0/0x4c8
[ ] hci_event_packet+0x440/0xc9c
[ ] hci_rx_work+0x44c/0xaf8
[ ] process_one_work+0x54c/0x103c
[ ] worker_thread+0x6c4/0x10c4
[ ] kthread+0x274/0x2ec
[ ] ret_from_fork+0x10/0x20
[ ] Code: 14000004 91004021 eb14003f 54000180 (f9400024)
[ ] ---[ end trace 0000000000000000 ]---
Fixes: 2935e55685 ("Bluetooth: hci_sync: fix double free in 'hci_discovery_filter_clear()'")
Signed-off-by: Pavel Shpakovskiy <pashpakovskii@salutedevices.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This commit is contained in:
committed by
Luiz Augusto von Dentz
parent
d4bfa78fd6
commit
59eecbe2f2
@@ -935,9 +935,9 @@ static inline void hci_discovery_filter_clear(struct hci_dev *hdev)
|
||||
hdev->discovery.result_filtering = false;
|
||||
hdev->discovery.report_invalid_rssi = true;
|
||||
hdev->discovery.rssi = HCI_RSSI_INVALID;
|
||||
hdev->discovery.uuid_count = 0;
|
||||
|
||||
spin_lock(&hdev->discovery.lock);
|
||||
hdev->discovery.uuid_count = 0;
|
||||
kfree(hdev->discovery.uuids);
|
||||
hdev->discovery.uuids = NULL;
|
||||
spin_unlock(&hdev->discovery.lock);
|
||||
|
||||
@@ -6171,6 +6171,7 @@ static int start_service_discovery(struct sock *sk, struct hci_dev *hdev,
|
||||
struct mgmt_pending_cmd *cmd;
|
||||
const u16 max_uuid_count = ((U16_MAX - sizeof(*cp)) / 16);
|
||||
u16 uuid_count, expected_len;
|
||||
u8 (*uuids)[16] = NULL;
|
||||
u8 status;
|
||||
int err;
|
||||
|
||||
@@ -6247,12 +6248,10 @@ static int start_service_discovery(struct sock *sk, struct hci_dev *hdev,
|
||||
hdev->discovery.result_filtering = true;
|
||||
hdev->discovery.type = cp->type;
|
||||
hdev->discovery.rssi = cp->rssi;
|
||||
hdev->discovery.uuid_count = uuid_count;
|
||||
|
||||
if (uuid_count > 0) {
|
||||
hdev->discovery.uuids = kmemdup(cp->uuids, uuid_count * 16,
|
||||
GFP_KERNEL);
|
||||
if (!hdev->discovery.uuids) {
|
||||
uuids = kmemdup(cp->uuids, uuid_count * sizeof(*uuids), GFP_KERNEL);
|
||||
if (!uuids) {
|
||||
err = mgmt_cmd_complete(sk, hdev->id,
|
||||
MGMT_OP_START_SERVICE_DISCOVERY,
|
||||
MGMT_STATUS_FAILED,
|
||||
@@ -6262,6 +6261,11 @@ static int start_service_discovery(struct sock *sk, struct hci_dev *hdev,
|
||||
}
|
||||
}
|
||||
|
||||
spin_lock(&hdev->discovery.lock);
|
||||
hdev->discovery.uuids = uuids;
|
||||
hdev->discovery.uuid_count = uuid_count;
|
||||
spin_unlock(&hdev->discovery.lock);
|
||||
|
||||
err = hci_cmd_sync_queue(hdev, start_discovery_sync, cmd,
|
||||
start_discovery_complete);
|
||||
if (err < 0) {
|
||||
@@ -10505,6 +10509,7 @@ static bool is_filter_match(struct hci_dev *hdev, s8 rssi, u8 *eir,
|
||||
!hci_test_quirk(hdev, HCI_QUIRK_STRICT_DUPLICATE_FILTER))))
|
||||
return false;
|
||||
|
||||
spin_lock(&hdev->discovery.lock);
|
||||
if (hdev->discovery.uuid_count != 0) {
|
||||
/* If a list of UUIDs is provided in filter, results with no
|
||||
* matching UUID should be dropped.
|
||||
@@ -10513,9 +10518,12 @@ static bool is_filter_match(struct hci_dev *hdev, s8 rssi, u8 *eir,
|
||||
hdev->discovery.uuids) &&
|
||||
!eir_has_uuids(scan_rsp, scan_rsp_len,
|
||||
hdev->discovery.uuid_count,
|
||||
hdev->discovery.uuids))
|
||||
hdev->discovery.uuids)) {
|
||||
spin_unlock(&hdev->discovery.lock);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
spin_unlock(&hdev->discovery.lock);
|
||||
|
||||
/* If duplicate filtering does not report RSSI changes, then restart
|
||||
* scanning to ensure updated result with updated RSSI values.
|
||||
|
||||
Reference in New Issue
Block a user