* [PATCH 0/2] Bluetooth: Fix advertising instance accesses without hdev->lock
@ 2026-10-30 14:27 Cen Zhang
2026-10-30 14:27 ` [PATCH 1/2] Bluetooth: hci_sync: Fix remaining advertising instance UAFs Cen Zhang
2026-10-30 14:27 ` [PATCH 2/2] Bluetooth: hci_core: Initialize advertising RPA work only once Cen Zhang
0 siblings, 2 replies; 3+ messages in thread
From: Cen Zhang @ 2026-10-30 14:27 UTC (permalink / raw)
To: marcel, luiz.dentz
Cc: linux-bluetooth, linux-kernel, baijiaju1990, jjzuming, zzzccc427
Commit cdc36db204ff ("Bluetooth: hci_sync: Fix advertising data UAFs")
fixed the extended advertising data paths that use advertising
instances without hdev->lock. Patch 1 fixes the remaining request paths
with the same problem in the same way, and copies the instance numbers
under the lock for walks that send commands, as commit cac164b6246f
("Bluetooth: hci_sync: Copy the LE accept list under hdev->lock") does.
Patch 2 stops hci_add_adv_instance() from reinitializing the RPA work
of an existing instance, which lets hci_remove_adv_instance() free the
instance while the work runs.
Cen Zhang (2):
Bluetooth: hci_sync: Fix remaining advertising instance UAFs
Bluetooth: hci_core: Initialize advertising RPA work only once
net/bluetooth/hci_core.c | 5 +-
net/bluetooth/hci_sync.c | 296 +++++++++++++++++++++++++++++----------
net/bluetooth/mgmt.c | 34 ++++-
3 files changed, 255 insertions(+), 80 deletions(-)
---
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/2] Bluetooth: hci_sync: Fix remaining advertising instance UAFs
2026-10-30 14:27 [PATCH 0/2] Bluetooth: Fix advertising instance accesses without hdev->lock Cen Zhang
@ 2026-10-30 14:27 ` Cen Zhang
2026-10-30 14:27 ` [PATCH 2/2] Bluetooth: hci_core: Initialize advertising RPA work only once Cen Zhang
1 sibling, 0 replies; 3+ messages in thread
From: Cen Zhang @ 2026-10-30 14:27 UTC (permalink / raw)
To: marcel, luiz.dentz
Cc: linux-bluetooth, linux-kernel, baijiaju1990, jjzuming, zzzccc427
hci_enable_ext_advertising_sync() looks up the advertising instance
without hdev->lock and then reads it, while HCI event processing can
free the instance under the lock:
Request work HCI event work
hci_enable_ext_advertising_sync()
adv = hci_find_adv_instance()
hci_cc_le_clear_adv_sets()
hci_dev_lock(hdev)
hci_remove_adv_instance()
kfree(adv)
hci_dev_unlock(hdev)
reads adv->enabled
Since the conversion to hci_sync, the advertising request paths run
without hdev->lock, and the periodic advertising, advertising timeout
and mesh code added later does the same, while management commands and
HCI event processing still add and free instances under the lock. The
same unlocked use exists in:
- hci_set_adv_data_sync(), hci_disable_ext_adv_instance_sync(),
hci_remove_ext_adv_instance_sync(), the periodic advertising enable
and disable helpers, hci_schedule_adv_instance_sync(),
adv_expire_sync() and set_advertising_complete()
- hci_enable_advertising_sync() and hci_start_per_adv_sync(), which
keep the instance across HCI commands
- hci_remove_advertising_sync() and hci_clear_adv_instance_sync(),
which keep the next instance as a pointer
- mesh_send_sync(), which adds the mesh instance
- the walks of hdev->adv_instances in hci_dev_open_sync(),
hci_dev_close_sync(), hci_resume_advertising_sync() and
hci_powered_update_adv_sync()
Fix them as commit cdc36db204ff ("Bluetooth: hci_sync: Fix advertising
data UAFs") does for the extended advertising data paths: look the
instance up under hdev->lock, copy what is needed, and release the lock
before a command is sent, since command completion handlers take it.
Keep the next instance as an instance number. Walks that send no
command hold the lock, as hci_clear_adv_sync() does. The resume and
power on walks send a command per instance and cannot hold it, so they
copy the instance numbers under the lock first, as commit cac164b6246f
("Bluetooth: hci_sync: Copy the LE accept list under hdev->lock") does
for the accept list.
KASAN report as below:
BUG: KASAN: slab-use-after-free in
hci_enable_ext_advertising_sync+0x2fd/0x330
Read of size 1 at addr ffff88811673b810 by task kworker/u17:0/492
Workqueue: hci0 hci_cmd_sync_work
Call Trace:
hci_enable_ext_advertising_sync+0x2fd/0x330
add_ext_adv_data_sync+0x1f8/0x250
hci_cmd_sync_work+0x1b3/0x450
Allocated by task 490:
hci_add_adv_instance+0x627/0x960
add_ext_adv_params+0x4d4/0xaf0
hci_sock_sendmsg+0x128b/0x22e0
Freed by task 495:
kfree+0x307/0x580
hci_remove_adv_instance+0x226/0x4f0
hci_cc_le_clear_adv_sets+0x2a5/0x400
hci_cmd_complete_evt+0x373/0xab0
hci_event_packet+0x4e8/0xc70
hci_rx_work+0x367/0xf00
The buggy address belongs to the object at ffff88811673b800
which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 16 bytes inside of
freed 1024-byte region [ffff88811673b800, ffff88811673bc00)
Fixes: cba6b758711c ("Bluetooth: hci_sync: Make use of hci_cmd_sync_queue set 2")
Fixes: eca0ae4aea66 ("Bluetooth: Add initial implementation of BIS connections")
Fixes: c249ea9b4309 ("Bluetooth: Move Adv Instance timer to hci_sync")
Fixes: b338d91703fa ("Bluetooth: Implement support for Mesh")
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
Assisted-by: LLM
net/bluetooth/hci_sync.c | 296 +++++++++++++++++++++++++++++----------
net/bluetooth/mgmt.c | 34 ++++-
2 files changed, 252 insertions(+), 78 deletions(-)
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 9d71ce96bd8ec..72533236ee819 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -489,7 +489,8 @@ static void cancel_adv_timeout(struct hci_dev *hdev)
int hci_clear_adv_instance_sync(struct hci_dev *hdev, struct sock *sk,
u8 instance, bool force)
{
- struct adv_info *adv_instance, *n, *next_instance = NULL;
+ struct adv_info *adv_instance, *n;
+ u8 next_instance = 0;
int err;
u8 rem_inst;
@@ -497,12 +498,20 @@ int hci_clear_adv_instance_sync(struct hci_dev *hdev, struct sock *sk,
if (!instance || hdev->cur_adv_instance == instance)
cancel_adv_timeout(hdev);
+ /* This is safe as long as there is no command send while the
+ * lock is held.
+ */
+ hci_dev_lock(hdev);
+
/* Get the next instance to advertise BEFORE we remove
* the current one. This can be the same instance again
* if there is only one instance.
*/
- if (instance && hdev->cur_adv_instance == instance)
- next_instance = hci_get_next_instance(hdev, instance);
+ if (instance && hdev->cur_adv_instance == instance) {
+ adv_instance = hci_get_next_instance(hdev, instance);
+ if (adv_instance)
+ next_instance = adv_instance->instance;
+ }
if (instance == 0x00) {
list_for_each_entry_safe(adv_instance, n, &hdev->adv_instances,
@@ -521,9 +530,8 @@ int hci_clear_adv_instance_sync(struct hci_dev *hdev, struct sock *sk,
if (force || (adv_instance && adv_instance->timeout &&
!adv_instance->remaining_time)) {
/* Don't advertise a removed instance. */
- if (next_instance &&
- next_instance->instance == instance)
- next_instance = NULL;
+ if (next_instance == instance)
+ next_instance = 0;
err = hci_remove_adv_instance(hdev, instance);
if (!err)
@@ -531,12 +539,13 @@ int hci_clear_adv_instance_sync(struct hci_dev *hdev, struct sock *sk,
}
}
+ hci_dev_unlock(hdev);
+
if (!hdev_is_powered(hdev) || hci_dev_test_flag(hdev, HCI_ADVERTISING))
return 0;
if (next_instance && !ext_adv_capable(hdev))
- return hci_schedule_adv_instance_sync(hdev,
- next_instance->instance,
+ return hci_schedule_adv_instance_sync(hdev, next_instance,
false);
return 0;
@@ -1214,16 +1223,26 @@ static int hci_disable_ext_adv_instance_sync(struct hci_dev *hdev, u8 instance)
struct hci_cp_ext_adv_set *set;
u8 data[sizeof(*cp) + sizeof(*set) * 1];
u8 size;
- struct adv_info *adv = NULL;
+ u8 handle = instance;
/* If request specifies an instance that doesn't exist, fail */
if (instance > 0) {
+ struct adv_info *adv;
+ bool enabled;
+
+ hci_dev_lock(hdev);
adv = hci_find_adv_instance(hdev, instance);
- if (!adv)
+ if (!adv) {
+ hci_dev_unlock(hdev);
return -EINVAL;
+ }
+
+ enabled = adv->enabled;
+ handle = adv->handle;
+ hci_dev_unlock(hdev);
/* If not enabled there is nothing to do */
- if (!adv->enabled)
+ if (!enabled)
return 0;
}
@@ -1236,7 +1255,7 @@ static int hci_disable_ext_adv_instance_sync(struct hci_dev *hdev, u8 instance)
cp->num_of_sets = !!instance;
cp->enable = 0x00;
- set->handle = adv ? adv->handle : instance;
+ set->handle = handle;
size = sizeof(*cp) + sizeof(*set) * cp->num_of_sets;
@@ -1374,14 +1393,21 @@ static int hci_set_ext_adv_data_sync(struct hci_dev *hdev, u8 instance)
}
static int hci_set_adv_data_sync(struct hci_dev *hdev, u8 instance)
+ __context_unsafe(/* conditional locking */)
{
struct hci_cp_le_set_adv_data cp;
u8 len;
memset(&cp, 0, sizeof(cp));
+ if (instance)
+ hci_dev_lock(hdev);
+
len = eir_create_adv_data(hdev, instance, cp.data, sizeof(cp.data));
+ if (instance)
+ hci_dev_unlock(hdev);
+
/* There's nothing to do if the data hasn't changed */
if (hdev->adv_data_len == len &&
memcmp(cp.data, hdev->adv_data, len) == 0)
@@ -1670,17 +1696,33 @@ int hci_enable_ext_advertising_sync(struct hci_dev *hdev, u8 instance)
struct hci_cp_le_set_ext_adv_enable *cp;
struct hci_cp_ext_adv_set *set;
u8 data[sizeof(*cp) + sizeof(*set) * 1];
- struct adv_info *adv;
+ u8 handle = instance;
+ u16 duration = 0;
if (instance > 0) {
+ struct adv_info *adv;
+
+ hci_dev_lock(hdev);
adv = hci_find_adv_instance(hdev, instance);
- if (!adv)
+ if (!adv) {
+ hci_dev_unlock(hdev);
return -EINVAL;
+ }
/* If already enabled there is nothing to do */
- if (adv->enabled)
+ if (adv->enabled) {
+ hci_dev_unlock(hdev);
return 0;
- } else {
- adv = NULL;
+ }
+
+ handle = adv->handle;
+
+ /* Set duration per instance since controller is
+ * responsible for scheduling it.
+ */
+ if (adv->timeout)
+ duration = adv->mesh ? adv->timeout :
+ adv->timeout * MSEC_PER_SEC;
+ hci_dev_unlock(hdev);
}
cp = (void *)data;
@@ -1693,18 +1735,10 @@ int hci_enable_ext_advertising_sync(struct hci_dev *hdev, u8 instance)
memset(set, 0, sizeof(*set));
- set->handle = adv ? adv->handle : instance;
-
- /* Set duration per instance since controller is responsible for
- * scheduling it.
- */
- if (adv && adv->timeout) {
- u16 duration = adv->mesh ? adv->timeout :
- adv->timeout * MSEC_PER_SEC;
+ set->handle = handle;
- /* Time = N * 10 ms */
- set->duration = cpu_to_le16(duration / 10);
- }
+ /* Time = N * 10 ms */
+ set->duration = cpu_to_le16(duration / 10);
/* Track which instance owns the advertising set since the handle
* alone is ambiguous when instances share handle 0x00, see
@@ -1736,11 +1770,15 @@ int hci_start_ext_adv_sync(struct hci_dev *hdev, u8 instance)
int hci_disable_per_advertising_sync(struct hci_dev *hdev, u8 instance)
{
struct hci_cp_le_set_per_adv_enable cp;
- struct adv_info *adv = NULL;
+ struct adv_info *adv;
+ bool enabled;
/* If periodic advertising already disabled there is nothing to do. */
+ hci_dev_lock(hdev);
adv = hci_find_adv_instance(hdev, instance);
- if (!adv || !adv->periodic_enabled)
+ enabled = adv && adv->periodic_enabled;
+ hci_dev_unlock(hdev);
+ if (!enabled)
return 0;
memset(&cp, 0, sizeof(cp));
@@ -1809,11 +1847,15 @@ static int hci_set_per_adv_data_sync(struct hci_dev *hdev, u8 instance)
static int hci_enable_per_advertising_sync(struct hci_dev *hdev, u8 instance)
{
struct hci_cp_le_set_per_adv_enable cp;
- struct adv_info *adv = NULL;
+ struct adv_info *adv;
+ bool enabled;
/* If periodic advertising already enabled there is nothing to do. */
+ hci_dev_lock(hdev);
adv = hci_find_adv_instance(hdev, instance);
- if (adv && adv->periodic_enabled)
+ enabled = adv && adv->periodic_enabled;
+ hci_dev_unlock(hdev);
+ if (enabled)
return 0;
memset(&cp, 0, sizeof(cp));
@@ -1828,25 +1870,36 @@ static int hci_enable_per_advertising_sync(struct hci_dev *hdev, u8 instance)
/* Checks if periodic advertising data contains a Basic Announcement and if it
* does generates a Broadcast ID and add Broadcast Announcement.
*/
-static int hci_adv_bcast_annoucement(struct hci_dev *hdev, struct adv_info *adv)
+static int hci_adv_bcast_annoucement(struct hci_dev *hdev, u8 instance)
{
+ struct adv_info *adv;
u8 bid[3];
u8 ad[HCI_MAX_EXT_AD_LENGTH];
u8 len;
+ int err = 0;
- /* Skip if NULL adv as instance 0x00 is used for general purpose
+ /* Skip if instance 0x00 as it is used for general purpose
* advertising so it cannot used for the likes of Broadcast Announcement
* as it can be overwritten at any point.
*/
- if (!adv)
+ if (!instance)
return 0;
+ hci_dev_lock(hdev);
+
+ /* The instance may have been removed while waiting for the
+ * controller, in which case there is nothing to announce.
+ */
+ adv = hci_find_adv_instance(hdev, instance);
+ if (!adv)
+ goto unlock;
+
/* Check if PA data doesn't contains a Basic Audio Announcement then
* there is nothing to do.
*/
if (!eir_get_service_data(adv->per_adv_data, adv->per_adv_data_len,
0x1851, NULL))
- return 0;
+ goto unlock;
/* Check if advertising data already has a Broadcast Announcement since
* the process may want to control the Broadcast ID directly and in that
@@ -1854,34 +1907,44 @@ static int hci_adv_bcast_annoucement(struct hci_dev *hdev, struct adv_info *adv)
*/
if (eir_get_service_data(adv->adv_data, adv->adv_data_len, 0x1852,
NULL))
- return 0;
+ goto unlock;
/* Generate Broadcast ID */
get_random_bytes(bid, sizeof(bid));
len = eir_append_service_data(ad, 0, 0x1852, bid, sizeof(bid));
if (adv->adv_data_len > sizeof(ad) - len) {
bt_dev_err(hdev, "No room for Broadcast Announcement");
- return -EINVAL;
+ err = -EINVAL;
+ goto unlock;
}
memcpy(ad + len, adv->adv_data, adv->adv_data_len);
- hci_set_adv_instance_data(hdev, adv->instance, len + adv->adv_data_len,
+ hci_set_adv_instance_data(hdev, instance, len + adv->adv_data_len,
ad, 0, NULL);
- return hci_update_adv_data_sync(hdev, adv->instance);
+ hci_dev_unlock(hdev);
+
+ return hci_update_adv_data_sync(hdev, instance);
+
+unlock:
+ hci_dev_unlock(hdev);
+ return err;
}
int hci_start_per_adv_sync(struct hci_dev *hdev, u8 instance, u8 sid,
u8 data_len, u8 *data, u32 flags, u16 min_interval,
u16 max_interval, u16 sync_interval)
{
- struct adv_info *adv = NULL;
+ struct adv_info *adv;
+ u8 bcast_instance = 0;
int err;
bool added = false;
hci_disable_per_advertising_sync(hdev, instance);
if (instance) {
+ hci_dev_lock(hdev);
+
adv = hci_find_adv_instance(hdev, instance);
if (adv) {
if (sid != HCI_SID_INVALID && adv->sid != sid) {
@@ -1890,6 +1953,7 @@ int hci_start_per_adv_sync(struct hci_dev *hdev, u8 instance, u8 sid,
*/
adv = hci_find_adv_sid(hdev, sid);
if (!adv) {
+ hci_dev_unlock(hdev);
bt_dev_err(hdev,
"Unable to find adv_info");
return -EINVAL;
@@ -1908,11 +1972,17 @@ int hci_start_per_adv_sync(struct hci_dev *hdev, u8 instance, u8 sid,
data_len, data,
sync_interval,
sync_interval);
- if (IS_ERR(adv))
+ if (IS_ERR(adv)) {
+ hci_dev_unlock(hdev);
return PTR_ERR(adv);
+ }
adv->pending = false;
added = true;
}
+
+ bcast_instance = adv->instance;
+
+ hci_dev_unlock(hdev);
}
/* Start advertising */
@@ -1920,7 +1990,7 @@ int hci_start_per_adv_sync(struct hci_dev *hdev, u8 instance, u8 sid,
if (err < 0)
goto fail;
- err = hci_adv_bcast_annoucement(hdev, adv);
+ err = hci_adv_bcast_annoucement(hdev, bcast_instance);
if (err < 0)
goto fail;
@@ -1940,8 +2010,11 @@ int hci_start_per_adv_sync(struct hci_dev *hdev, u8 instance, u8 sid,
return 0;
fail:
- if (added)
+ if (added) {
+ hci_dev_lock(hdev);
hci_remove_adv_instance(hdev, instance);
+ hci_dev_unlock(hdev);
+ }
return err;
}
@@ -1971,6 +2044,7 @@ int hci_enable_advertising_sync(struct hci_dev *hdev)
u8 own_addr_type, enable = 0x01;
bool connectable;
u16 adv_min_interval, adv_max_interval;
+ bool scannable;
u32 flags;
u8 status;
@@ -1978,8 +2052,25 @@ int hci_enable_advertising_sync(struct hci_dev *hdev)
return hci_enable_ext_advertising_sync(hdev,
hdev->cur_adv_instance);
+ /* Snapshot the instance under hdev->lock since it may be removed
+ * while waiting for the commands below to complete.
+ */
+ hci_dev_lock(hdev);
+
flags = hci_adv_instance_flags(hdev, hdev->cur_adv_instance);
+ scannable = hci_adv_instance_is_scannable(hdev,
+ hdev->cur_adv_instance);
+
adv_instance = hci_find_adv_instance(hdev, hdev->cur_adv_instance);
+ if (adv_instance) {
+ adv_min_interval = adv_instance->min_interval;
+ adv_max_interval = adv_instance->max_interval;
+ } else {
+ adv_min_interval = hdev->le_adv_min_interval;
+ adv_max_interval = hdev->le_adv_max_interval;
+ }
+
+ hci_dev_unlock(hdev);
/* If the "connectable" instance flag was not set, then choose between
* ADV_IND and ADV_NONCONN_IND based on the global connectable setting.
@@ -2013,18 +2104,10 @@ int hci_enable_advertising_sync(struct hci_dev *hdev)
memset(&cp, 0, sizeof(cp));
- if (adv_instance) {
- adv_min_interval = adv_instance->min_interval;
- adv_max_interval = adv_instance->max_interval;
- } else {
- adv_min_interval = hdev->le_adv_min_interval;
- adv_max_interval = hdev->le_adv_max_interval;
- }
-
if (connectable) {
cp.type = LE_ADV_IND;
} else {
- if (hci_adv_instance_is_scannable(hdev, hdev->cur_adv_instance))
+ if (scannable)
cp.type = LE_ADV_SCAN_IND;
else
cp.type = LE_ADV_NONCONN_IND;
@@ -2077,8 +2160,16 @@ int hci_remove_ext_adv_instance_sync(struct hci_dev *hdev, u8 instance,
return err;
/* If request specifies an instance that doesn't exist, fail */
- if (instance > 0 && !hci_find_adv_instance(hdev, instance))
- return -EINVAL;
+ if (instance > 0) {
+ bool exists;
+
+ hci_dev_lock(hdev);
+ exists = !!hci_find_adv_instance(hdev, instance);
+ hci_dev_unlock(hdev);
+
+ if (!exists)
+ return -EINVAL;
+ }
return __hci_cmd_sync_status_sk(hdev, HCI_OP_LE_REMOVE_ADV_SET,
sizeof(instance), &instance, 0,
@@ -2109,9 +2200,13 @@ int hci_schedule_adv_instance_sync(struct hci_dev *hdev, u8 instance,
if (hdev->adv_instance_timeout)
return -EBUSY;
+ hci_dev_lock(hdev);
+
adv = hci_find_adv_instance(hdev, instance);
- if (!adv)
+ if (!adv) {
+ hci_dev_unlock(hdev);
return -ENOENT;
+ }
/* A zero timeout means unlimited advertising. As long as there is
* only one instance, duration should be ignored. We still set a timeout
@@ -2141,6 +2236,8 @@ int hci_schedule_adv_instance_sync(struct hci_dev *hdev, u8 instance,
secs_to_jiffies(timeout));
}
+ hci_dev_unlock(hdev);
+
/* If we're just re-scheduling the same instance again then do not
* execute any HCI commands. This happens when a single instance is
* being advertised.
@@ -2238,7 +2335,7 @@ static int hci_remove_adv_sync(struct hci_dev *hdev, u8 instance,
int hci_remove_advertising_sync(struct hci_dev *hdev, struct sock *sk,
u8 instance, bool force)
{
- struct adv_info *next = NULL;
+ u8 next_instance = 0;
int err;
/* Cancel any timeout concerning the removed instance(s). */
@@ -2249,20 +2346,34 @@ int hci_remove_advertising_sync(struct hci_dev *hdev, struct sock *sk,
* the current one. This can be the same instance again
* if there is only one instance.
*/
- if (hdev->cur_adv_instance == instance)
+ if (hdev->cur_adv_instance == instance) {
+ struct adv_info *next;
+
+ hci_dev_lock(hdev);
next = hci_get_next_instance(hdev, instance);
+ if (next)
+ next_instance = next->instance;
+ hci_dev_unlock(hdev);
+ }
if (!instance) {
err = hci_clear_adv_sync(hdev, sk, force);
if (err)
return err;
} else {
- struct adv_info *adv = hci_find_adv_instance(hdev, instance);
+ struct adv_info *adv;
+ bool remove;
+
+ hci_dev_lock(hdev);
+ adv = hci_find_adv_instance(hdev, instance);
+ remove = force ||
+ (adv && adv->timeout && !adv->remaining_time);
+ hci_dev_unlock(hdev);
- if (force || (adv && adv->timeout && !adv->remaining_time)) {
+ if (remove) {
/* Don't advertise a removed instance. */
- if (next && next->instance == instance)
- next = NULL;
+ if (next_instance == instance)
+ next_instance = 0;
err = hci_remove_adv_sync(hdev, instance, sk);
if (err)
@@ -2273,8 +2384,8 @@ int hci_remove_advertising_sync(struct hci_dev *hdev, struct sock *sk,
if (!hdev_is_powered(hdev) || hci_dev_test_flag(hdev, HCI_ADVERTISING))
return 0;
- if (next && !ext_adv_capable(hdev))
- hci_schedule_adv_instance_sync(hdev, next->instance, false);
+ if (next_instance && !ext_adv_capable(hdev))
+ hci_schedule_adv_instance_sync(hdev, next_instance, false);
return 0;
}
@@ -2732,10 +2843,33 @@ static int hci_pause_advertising_sync(struct hci_dev *hdev)
return 0;
}
+/* Copy the instance identifiers under hdev->lock, in list order, so that
+ * the caller can walk them while waiting for HCI commands to complete.
+ */
+static u8 *adv_instance_list_copy(struct hci_dev *hdev, size_t *n)
+{
+ struct adv_info *adv;
+ size_t i = 0;
+ u8 *ids;
+
+ hci_dev_lock(hdev);
+
+ *n = list_count_nodes(&hdev->adv_instances);
+ ids = kvzalloc_objs(u8, *n);
+ if (!ids)
+ goto unlock;
+
+ list_for_each_entry(adv, &hdev->adv_instances, list)
+ ids[i++] = adv->instance;
+
+unlock:
+ hci_dev_unlock(hdev);
+ return ids;
+}
+
/* This function enables all user advertising instances */
static int hci_resume_advertising_sync(struct hci_dev *hdev)
{
- struct adv_info *adv, *tmp;
int err = 0;
/* If advertising has not been paused there is nothing to do. */
@@ -2752,18 +2886,27 @@ static int hci_resume_advertising_sync(struct hci_dev *hdev)
bt_dev_dbg(hdev, "Resuming advertising instances");
if (ext_adv_capable(hdev)) {
+ size_t i, n;
+ u8 *ids;
+
+ ids = adv_instance_list_copy(hdev, &n);
+ if (!ids)
+ return -ENOMEM;
+
/* Call for each tracked instance to be re-enabled */
- list_for_each_entry_safe(adv, tmp, &hdev->adv_instances, list) {
+ for (i = 0; i < n; i++) {
err = hci_enable_ext_advertising_sync(hdev,
- adv->instance);
+ ids[i]);
if (!err)
continue;
/* If the instance cannot be resumed remove it */
- hci_remove_ext_adv_instance_sync(hdev, adv->instance,
+ hci_remove_ext_adv_instance_sync(hdev, ids[i],
NULL);
}
+ kvfree(ids);
+
/* If current advertising instance is set to instance 0x00
* then we need to re-enable it.
*/
@@ -3585,7 +3728,8 @@ int hci_write_le_host_supported_sync(struct hci_dev *hdev, u8 le, u8 simul)
static int hci_powered_update_adv_sync(struct hci_dev *hdev)
{
- struct adv_info *adv, *tmp;
+ size_t i, n;
+ u8 *ids;
int err;
if (!hci_dev_test_flag(hdev, HCI_LE_ENABLED))
@@ -3621,9 +3765,15 @@ static int hci_powered_update_adv_sync(struct hci_dev *hdev)
hci_enable_advertising_sync(hdev);
}
+ ids = adv_instance_list_copy(hdev, &n);
+ if (!ids)
+ return -ENOMEM;
+
/* Call for each tracked instance to be scheduled */
- list_for_each_entry_safe(adv, tmp, &hdev->adv_instances, list)
- hci_schedule_adv_instance_sync(hdev, adv->instance, true);
+ for (i = 0; i < n; i++)
+ hci_schedule_adv_instance_sync(hdev, ids[i], true);
+
+ kvfree(ids);
return 0;
}
@@ -5597,7 +5747,9 @@ int hci_dev_open_sync(struct hci_dev *hdev)
if (!ret) {
hci_dev_hold(hdev);
hci_dev_set_flag(hdev, HCI_RPA_EXPIRED);
+ hci_dev_lock(hdev);
hci_adv_instances_set_rpa_expired(hdev, true);
+ hci_dev_unlock(hdev);
set_bit(HCI_UP, &hdev->flags);
hci_sock_dev_event(hdev, HCI_DEV_UP);
hci_leds_update_powered(hdev, true);
@@ -5740,8 +5892,10 @@ int hci_dev_close_sync(struct hci_dev *hdev)
cancel_delayed_work_sync(&hdev->rpa_expired);
+ hci_dev_lock(hdev);
list_for_each_entry(adv_instance, &hdev->adv_instances, list)
cancel_delayed_work_sync(&adv_instance->rpa_expired_cb);
+ hci_dev_unlock(hdev);
}
/* Avoid potential lockdep warnings from the *_flush() calls by
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3cb4f9ece83cb..d8b1b9b241eb7 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2341,6 +2341,9 @@ static int mesh_send_sync(struct hci_dev *hdev, void *data)
timeout = 1000;
duration = send->cnt * INTERVAL_TO_MS(hdev->le_adv_max_interval);
+
+ hci_dev_lock(hdev);
+
adv = hci_add_adv_instance(hdev, instance, 0,
send->adv_data_len, send->adv_data,
0, NULL,
@@ -2375,6 +2378,8 @@ static int mesh_send_sync(struct hci_dev *hdev, void *data)
instance = 0;
}
+ hci_dev_unlock(hdev);
+
if (instance)
return hci_schedule_adv_instance_sync(hdev, instance, true);
@@ -4013,22 +4018,34 @@ static int user_passkey_neg_reply(struct sock *sk, struct hci_dev *hdev,
static int adv_expire_sync(struct hci_dev *hdev, u32 flags)
{
struct adv_info *adv_instance;
+ u8 instance;
+
+ hci_dev_lock(hdev);
adv_instance = hci_find_adv_instance(hdev, hdev->cur_adv_instance);
if (!adv_instance)
- return 0;
+ goto unlock;
/* stop if current instance doesn't need to be changed */
if (!(adv_instance->flags & flags))
- return 0;
+ goto unlock;
cancel_adv_timeout(hdev);
adv_instance = hci_get_next_instance(hdev, adv_instance->instance);
if (!adv_instance)
- return 0;
+ goto unlock;
+
+ instance = adv_instance->instance;
+
+ hci_dev_unlock(hdev);
- hci_schedule_adv_instance_sync(hdev, adv_instance->instance, true);
+ hci_schedule_adv_instance_sync(hdev, instance, true);
+
+ return 0;
+
+unlock:
+ hci_dev_unlock(hdev);
return 0;
}
@@ -6610,12 +6627,15 @@ static void set_advertising_complete(struct hci_dev *hdev, void *data, int err)
instance = hdev->cur_adv_instance;
if (!instance) {
+ hci_dev_lock(hdev);
adv_instance = list_first_entry_or_null(&hdev->adv_instances,
struct adv_info, list);
- if (!adv_instance)
- return;
+ if (adv_instance)
+ instance = adv_instance->instance;
+ hci_dev_unlock(hdev);
- instance = adv_instance->instance;
+ if (!instance)
+ return;
}
err = hci_schedule_adv_instance_sync(hdev, instance, true);
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] Bluetooth: hci_core: Initialize advertising RPA work only once
2026-10-30 14:27 [PATCH 0/2] Bluetooth: Fix advertising instance accesses without hdev->lock Cen Zhang
2026-10-30 14:27 ` [PATCH 1/2] Bluetooth: hci_sync: Fix remaining advertising instance UAFs Cen Zhang
@ 2026-10-30 14:27 ` Cen Zhang
1 sibling, 0 replies; 3+ messages in thread
From: Cen Zhang @ 2026-10-30 14:27 UTC (permalink / raw)
To: marcel, luiz.dentz
Cc: linux-bluetooth, linux-kernel, baijiaju1990, jjzuming, zzzccc427
hci_add_adv_instance() also updates existing instances and calls
INIT_DELAYED_WORK() on their rpa_expired_cb every time, even when the
work is pending or running. cancel_delayed_work_sync() then no longer
finds the work, so hci_remove_adv_instance() can free the instance
under it:
hdev->workqueue MGMT_OP_ADD_EXT_ADV_PARAMS
adv_instance_rpa_expired()
hci_dev_lock(hdev)
hci_add_adv_instance()
INIT_DELAYED_WORK()
mgmt_pending_new() fails
hci_remove_adv_instance()
cancel_delayed_work_sync()
returns without waiting
kfree(adv)
hci_dev_unlock(hdev)
adv->rpa_expired = true
The work has been initialized on every update since it was added.
Initialize it only when the instance is allocated.
KASAN report as below:
BUG: KASAN: slab-use-after-free in adv_instance_rpa_expired+0x139/0x160
Write of size 1 at addr ffff888108d84b2e by task kworker/u17:0/504
Workqueue: hci0 adv_instance_rpa_expired
Call Trace:
adv_instance_rpa_expired+0x139/0x160
Allocated by task 560:
hci_add_adv_instance+0x68d/0x9d0
add_ext_adv_params+0x4d4/0xb60
hci_sock_sendmsg+0x128b/0x22e0
Freed by task 594:
kfree+0x307/0x580
hci_remove_adv_instance+0x230/0x540
add_ext_adv_params+0xa43/0xb60
hci_sock_sendmsg+0x128b/0x22e0
Last potentially related work creation:
insert_work+0x37/0x210
__queue_work+0xaba/0x1270
call_timer_fn+0x171/0x500
The buggy address belongs to the object at ffff888108d84800
which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 814 bytes inside of
freed 1024-byte region [ffff888108d84800, ffff888108d84c00)
Fixes: a73c046a2869 ("Bluetooth: Implement Set ADV set random address")
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
Assisted-by: LLM
net/bluetooth/hci_core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 2a29cfdee9728..79573193471bc 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1796,6 +1796,9 @@ struct adv_info *hci_add_adv_instance(struct hci_dev *hdev, u8 instance,
if (!adv)
return ERR_PTR(-ENOMEM);
+ INIT_DELAYED_WORK(&adv->rpa_expired_cb,
+ adv_instance_rpa_expired);
+
adv->pending = true;
adv->instance = instance;
@@ -1832,8 +1835,6 @@ struct adv_info *hci_add_adv_instance(struct hci_dev *hdev, u8 instance,
else
adv->duration = duration;
- INIT_DELAYED_WORK(&adv->rpa_expired_cb, adv_instance_rpa_expired);
-
BT_DBG("%s for %dMR", hdev->name, instance);
return adv;
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-30 14:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-30 14:27 [PATCH 0/2] Bluetooth: Fix advertising instance accesses without hdev->lock Cen Zhang
2026-10-30 14:27 ` [PATCH 1/2] Bluetooth: hci_sync: Fix remaining advertising instance UAFs Cen Zhang
2026-10-30 14:27 ` [PATCH 2/2] Bluetooth: hci_core: Initialize advertising RPA work only once Cen Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox