[PATCH v2] Bluetooth: hci_sync: fix set_local_name race condition

[PATCH v2] Bluetooth: hci_sync: fix set_local_name race condition

[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

Re: [PATCH v2] Bluetooth: hci_sync: fix set_local_name race condition

[Paul Menzel]

[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

Re: [PATCH v2] Bluetooth: hci_sync: fix set_local_name race condition

[patchwork-bot+bluetooth]

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