* [PATCH v1] Bluetooth: hci_sync: fix set_local_name race condition
@ 2025-08-21 16:07 Pavel Shpakovskiy
2025-08-21 16:37 ` [v1] " bluez.test.bot
2025-08-21 18:51 ` [PATCH v1] " Paul Menzel
0 siblings, 2 replies; 3+ 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] 3+ messages in thread
* RE: [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 16:37 ` bluez.test.bot
2025-08-21 18:51 ` [PATCH v1] " Paul Menzel
1 sibling, 0 replies; 3+ messages in thread
From: bluez.test.bot @ 2025-08-21 16:37 UTC (permalink / raw)
To: linux-bluetooth, pashpakovskii
[-- Attachment #1: Type: text/plain, Size: 2296 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=994144
---Test result---
Test Summary:
CheckPatch PENDING 0.33 seconds
GitLint PENDING 0.32 seconds
SubjectPrefix PASS 0.09 seconds
BuildKernel PASS 24.62 seconds
CheckAllWarning PASS 27.18 seconds
CheckSparse PASS 30.55 seconds
BuildKernel32 PASS 24.49 seconds
TestRunnerSetup PASS 486.68 seconds
TestRunner_l2cap-tester PASS 25.61 seconds
TestRunner_iso-tester PASS 41.49 seconds
TestRunner_bnep-tester PASS 6.33 seconds
TestRunner_mgmt-tester FAIL 129.85 seconds
TestRunner_rfcomm-tester PASS 9.58 seconds
TestRunner_sco-tester PASS 15.06 seconds
TestRunner_ioctl-tester PASS 10.38 seconds
TestRunner_mesh-tester FAIL 11.64 seconds
TestRunner_smp-tester PASS 8.84 seconds
TestRunner_userchan-tester PASS 6.51 seconds
IncrementalBuild PENDING 0.43 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 490, Passed: 485 (99.0%), Failed: 1, Not Run: 4
Failed Test Cases
Read Exp Feature - Success Failed 0.109 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0
Failed Test Cases
Mesh - Send cancel - 1 Timed out 2.027 seconds
Mesh - Send cancel - 2 Timed out 1.997 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 3+ 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 16:37 ` [v1] " bluez.test.bot
@ 2025-08-21 18:51 ` Paul Menzel
1 sibling, 0 replies; 3+ 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] 3+ messages in thread
end of thread, other threads:[~2025-08-21 18:52 UTC | newest]
Thread overview: 3+ 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 16:37 ` [v1] " bluez.test.bot
2025-08-21 18:51 ` [PATCH v1] " 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).