linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] Bluetooth: hci_sync: fix set_local_name race condition
@ 2025-08-22  9:20 Pavel Shpakovskiy
  2025-08-22 10:00 ` [v2] " bluez.test.bot
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Pavel Shpakovskiy @ 2025-08-22  9:20 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 that 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>
---
 Changelog v1->v2:
 * Fix some minor style comments for commit messsage.

 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] 4+ messages in thread

* RE: [v2] Bluetooth: hci_sync: fix set_local_name race condition
  2025-08-22  9:20 [PATCH v2] Bluetooth: hci_sync: fix set_local_name race condition Pavel Shpakovskiy
@ 2025-08-22 10:00 ` bluez.test.bot
  2025-08-22 10:22 ` [PATCH v2] " Paul Menzel
  2025-08-22 14:40 ` patchwork-bot+bluetooth
  2 siblings, 0 replies; 4+ messages in thread
From: bluez.test.bot @ 2025-08-22 10:00 UTC (permalink / raw)
  To: linux-bluetooth, pashpakovskii

[-- Attachment #1: Type: text/plain, Size: 2376 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=994430

---Test result---

Test Summary:
CheckPatch                    PENDING   0.32 seconds
GitLint                       PENDING   0.24 seconds
SubjectPrefix                 PASS      0.11 seconds
BuildKernel                   PASS      24.41 seconds
CheckAllWarning               PASS      26.76 seconds
CheckSparse                   PASS      30.14 seconds
BuildKernel32                 PASS      24.18 seconds
TestRunnerSetup               PASS      480.50 seconds
TestRunner_l2cap-tester       PASS      25.14 seconds
TestRunner_iso-tester         PASS      39.24 seconds
TestRunner_bnep-tester        PASS      6.13 seconds
TestRunner_mgmt-tester        FAIL      129.79 seconds
TestRunner_rfcomm-tester      PASS      9.60 seconds
TestRunner_sco-tester         PASS      14.93 seconds
TestRunner_ioctl-tester       PASS      10.32 seconds
TestRunner_mesh-tester        FAIL      11.60 seconds
TestRunner_smp-tester         PASS      8.92 seconds
TestRunner_userchan-tester    PASS      6.48 seconds
IncrementalBuild              PENDING   0.57 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: 484 (98.8%), Failed: 2, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.105 seconds
LL Privacy - Set Flags 4 (RL is full)                Failed       0.276 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.031 seconds
Mesh - Send cancel - 2                               Timed out    1.998 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] Bluetooth: hci_sync: fix set_local_name race condition
  2025-08-22  9:20 [PATCH v2] Bluetooth: hci_sync: fix set_local_name race condition Pavel Shpakovskiy
  2025-08-22 10:00 ` [v2] " bluez.test.bot
@ 2025-08-22 10:22 ` Paul Menzel
  2025-08-22 14:40 ` patchwork-bot+bluetooth
  2 siblings, 0 replies; 4+ messages in thread
From: Paul Menzel @ 2025-08-22 10:22 UTC (permalink / raw)
  To: Pavel Shpakovskiy
  Cc: marcel, johan.hedberg, luiz.dentz, davem, edumazet, kuba, pabeni,
	horms, linux-bluetooth, netdev, linux-kernel, kernel

[Cc: remove bouncing brian.gix@intel.com]

Dear Pavel,


Thank you for the improved version.

Am 22.08.25 um 11:20 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 that 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>
> ---
>   Changelog v1->v2:
>   * Fix some minor style comments for commit messsage.
> 
>   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] 4+ messages in thread

* Re: [PATCH v2] Bluetooth: hci_sync: fix set_local_name race condition
  2025-08-22  9:20 [PATCH v2] Bluetooth: hci_sync: fix set_local_name race condition Pavel Shpakovskiy
  2025-08-22 10:00 ` [v2] " bluez.test.bot
  2025-08-22 10:22 ` [PATCH v2] " Paul Menzel
@ 2025-08-22 14:40 ` patchwork-bot+bluetooth
  2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+bluetooth @ 2025-08-22 14:40 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

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Fri, 22 Aug 2025 12:20:55 +0300 you wrote:
> 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 that 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.
> 
> [...]

Here is the summary with links:
  - [v2] Bluetooth: hci_sync: fix set_local_name race condition
    https://git.kernel.org/bluetooth/bluetooth-next/c/c49a788e88e4

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-08-22 14:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-22  9:20 [PATCH v2] Bluetooth: hci_sync: fix set_local_name race condition Pavel Shpakovskiy
2025-08-22 10:00 ` [v2] " bluez.test.bot
2025-08-22 10:22 ` [PATCH v2] " Paul Menzel
2025-08-22 14:40 ` patchwork-bot+bluetooth

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).