* [PATCH v1] Bluetooth: hci_sync: fix set_local_name race condition
@ 2025-08-21 16:07 Pavel Shpakovskiy
2025-08-21 18:51 ` Paul Menzel
0 siblings, 1 reply; 2+ messages in thread
From: Pavel Shpakovskiy @ 2025-08-21 16:07 UTC (permalink / raw)
To: marcel, johan.hedberg, luiz.dentz, davem, edumazet, kuba, pabeni,
horms, brian.gix
Cc: linux-bluetooth, netdev, linux-kernel, kernel, Pavel Shpakovskiy
Function set_name_sync() uses hdev->dev_name field to send
HCI_OP_WRITE_LOCAL_NAME command, but copying from data to
hdev->dev_name is called after mgmt cmd was queued, so it
is possible when function set_name_sync() will read old name
value.
This change adds name as a parameter for function
hci_update_name_sync() to avoid race condition.
Fixes: 6f6ff38a1e14 ("Bluetooth: hci_sync: Convert MGMT_OP_SET_LOCAL_NAME")
Signed-off-by: Pavel Shpakovskiy <pashpakovskii@salutedevices.com>
---
include/net/bluetooth/hci_sync.h | 2 +-
net/bluetooth/hci_sync.c | 6 +++---
net/bluetooth/mgmt.c | 5 ++++-
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h
index 72558c826aa1b..eef12830eaec9 100644
--- a/include/net/bluetooth/hci_sync.h
+++ b/include/net/bluetooth/hci_sync.h
@@ -93,7 +93,7 @@ int hci_update_class_sync(struct hci_dev *hdev);
int hci_update_eir_sync(struct hci_dev *hdev);
int hci_update_class_sync(struct hci_dev *hdev);
-int hci_update_name_sync(struct hci_dev *hdev);
+int hci_update_name_sync(struct hci_dev *hdev, const u8 *name);
int hci_write_ssp_mode_sync(struct hci_dev *hdev, u8 mode);
int hci_get_random_address(struct hci_dev *hdev, bool require_privacy,
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index e56b1cbedab90..c2a6469e81cdf 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -3412,13 +3412,13 @@ int hci_update_scan_sync(struct hci_dev *hdev)
return hci_write_scan_enable_sync(hdev, scan);
}
-int hci_update_name_sync(struct hci_dev *hdev)
+int hci_update_name_sync(struct hci_dev *hdev, const u8 *name)
{
struct hci_cp_write_local_name cp;
memset(&cp, 0, sizeof(cp));
- memcpy(cp.name, hdev->dev_name, sizeof(cp.name));
+ memcpy(cp.name, name, sizeof(cp.name));
return __hci_cmd_sync_status(hdev, HCI_OP_WRITE_LOCAL_NAME,
sizeof(cp), &cp,
@@ -3471,7 +3471,7 @@ int hci_powered_update_sync(struct hci_dev *hdev)
hci_write_fast_connectable_sync(hdev, false);
hci_update_scan_sync(hdev);
hci_update_class_sync(hdev);
- hci_update_name_sync(hdev);
+ hci_update_name_sync(hdev, hdev->dev_name);
hci_update_eir_sync(hdev);
}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 46b22708dfbd2..da662e1823ae5 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3876,8 +3876,11 @@ static void set_name_complete(struct hci_dev *hdev, void *data, int err)
static int set_name_sync(struct hci_dev *hdev, void *data)
{
+ struct mgmt_pending_cmd *cmd = data;
+ struct mgmt_cp_set_local_name *cp = cmd->param;
+
if (lmp_bredr_capable(hdev)) {
- hci_update_name_sync(hdev);
+ hci_update_name_sync(hdev, cp->name);
hci_update_eir_sync(hdev);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v1] Bluetooth: hci_sync: fix set_local_name race condition
2025-08-21 16:07 [PATCH v1] Bluetooth: hci_sync: fix set_local_name race condition Pavel Shpakovskiy
@ 2025-08-21 18:51 ` Paul Menzel
0 siblings, 0 replies; 2+ messages in thread
From: Paul Menzel @ 2025-08-21 18:51 UTC (permalink / raw)
To: Pavel Shpakovskiy
Cc: marcel, johan.hedberg, luiz.dentz, davem, edumazet, kuba, pabeni,
horms, brian.gix, linux-bluetooth, netdev, linux-kernel, kernel
Dear Pavel,
Thank you for the patch. Some minor style comments below.
Am 21.08.25 um 18:07 schrieb Pavel Shpakovskiy:
> Function set_name_sync() uses hdev->dev_name field to send
> HCI_OP_WRITE_LOCAL_NAME command, but copying from data to
> hdev->dev_name is called after mgmt cmd was queued, so it
> is possible when function set_name_sync() will read old name
… is possible *that* function …?
> value.
>
> This change adds name as a parameter for function
> hci_update_name_sync() to avoid race condition.
(Using 75 characters per line would save some lines.)
> Fixes: 6f6ff38a1e14 ("Bluetooth: hci_sync: Convert MGMT_OP_SET_LOCAL_NAME")
> Signed-off-by: Pavel Shpakovskiy <pashpakovskii@salutedevices.com>
> ---
> include/net/bluetooth/hci_sync.h | 2 +-
> net/bluetooth/hci_sync.c | 6 +++---
> net/bluetooth/mgmt.c | 5 ++++-
> 3 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h
> index 72558c826aa1b..eef12830eaec9 100644
> --- a/include/net/bluetooth/hci_sync.h
> +++ b/include/net/bluetooth/hci_sync.h
> @@ -93,7 +93,7 @@ int hci_update_class_sync(struct hci_dev *hdev);
>
> int hci_update_eir_sync(struct hci_dev *hdev);
> int hci_update_class_sync(struct hci_dev *hdev);
> -int hci_update_name_sync(struct hci_dev *hdev);
> +int hci_update_name_sync(struct hci_dev *hdev, const u8 *name);
> int hci_write_ssp_mode_sync(struct hci_dev *hdev, u8 mode);
>
> int hci_get_random_address(struct hci_dev *hdev, bool require_privacy,
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index e56b1cbedab90..c2a6469e81cdf 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -3412,13 +3412,13 @@ int hci_update_scan_sync(struct hci_dev *hdev)
> return hci_write_scan_enable_sync(hdev, scan);
> }
>
> -int hci_update_name_sync(struct hci_dev *hdev)
> +int hci_update_name_sync(struct hci_dev *hdev, const u8 *name)
> {
> struct hci_cp_write_local_name cp;
>
> memset(&cp, 0, sizeof(cp));
>
> - memcpy(cp.name, hdev->dev_name, sizeof(cp.name));
> + memcpy(cp.name, name, sizeof(cp.name));
>
> return __hci_cmd_sync_status(hdev, HCI_OP_WRITE_LOCAL_NAME,
> sizeof(cp), &cp,
> @@ -3471,7 +3471,7 @@ int hci_powered_update_sync(struct hci_dev *hdev)
> hci_write_fast_connectable_sync(hdev, false);
> hci_update_scan_sync(hdev);
> hci_update_class_sync(hdev);
> - hci_update_name_sync(hdev);
> + hci_update_name_sync(hdev, hdev->dev_name);
> hci_update_eir_sync(hdev);
> }
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 46b22708dfbd2..da662e1823ae5 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -3876,8 +3876,11 @@ static void set_name_complete(struct hci_dev *hdev, void *data, int err)
>
> static int set_name_sync(struct hci_dev *hdev, void *data)
> {
> + struct mgmt_pending_cmd *cmd = data;
> + struct mgmt_cp_set_local_name *cp = cmd->param;
> +
> if (lmp_bredr_capable(hdev)) {
> - hci_update_name_sync(hdev);
> + hci_update_name_sync(hdev, cp->name);
> hci_update_eir_sync(hdev);
> }
>
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-08-21 18:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-21 16:07 [PATCH v1] Bluetooth: hci_sync: fix set_local_name race condition Pavel Shpakovskiy
2025-08-21 18:51 ` Paul Menzel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).