[PATCH BlueZ v2] adapter/advertising: fix mgmt endian bug

[PATCH BlueZ v2] adapter/advertising: fix mgmt endian bug

[Nicolas Thibert]

Several places in adapter.c and advertising.c write a native-endian
value directly into a __le16/__le32 field of an mgmt command struct,
skipping the cpu_to_le16()/cpu_to_le32() conversion used everywhere
else in this codebase for the same purpose. This is a no-op on
little-endian hosts (where cpu_to_le16()/cpu_to_le32() are themselves
no-ops), which is why it has gone unnoticed, but corrupts the value on
big-endian hosts.

Confirmed live on MIPS big-endian (OpenWrt/ath79): set_blocked_keys()
sends key_count=2 (2 blocked keys), which the kernel's
__le16_to_cpu() correctly interprets as 512 (0x0002 byte-swapped is
0x0200) since the wire bytes were never actually swapped to little-
endian on the way out -- producing "expected 8706 bytes, got 36
bytes" / "Failed to set blocked keys: Invalid Parameters" errors in
dmesg/bluetoothd logs.

add_advertising()'s cp->duration and refresh_extended_adv()'s
cp.duration/cp.min_interval/cp.max_interval have the identical bug. It
is inert with bluetoothctl's default (0) duration/interval values (0
byte-swapped is still 0), but A/B tested live (patch removed vs.
applied, bluetoothctl's advertise submenu "interval 100 100" set
explicitly) confirms this is not just a theoretical correctness fix:
with the bug present, the device stops being discoverable by a real
BLE scanner the moment a non-default interval is requested, and
becomes discoverable again immediately once patched. Any application
that sets an explicit advertising interval or duration hits this.

v2: use cpu_to_le16()/cpu_to_le32() instead of htobs()/htobl(), as
requested by Luiz. Retested live on the same MIPS big-endian board
(blocked keys + explicit advertising interval): behaves identically
to the v1 fix, no regressions.
---
 src/adapter.c     | 5 +++--
 src/advertising.c | 8 ++++----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 7390ceeee..edbdb53b5 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -10219,10 +10219,11 @@ static bool set_blocked_keys(struct btd_adapter *adapter)
 					sizeof(blocked_keys)] = { 0 };
 	struct mgmt_cp_set_blocked_keys *cp =
 				(struct mgmt_cp_set_blocked_keys *)buffer;
+	const uint16_t key_count = ARRAY_SIZE(blocked_keys);
 	int i;
 
-	cp->key_count = ARRAY_SIZE(blocked_keys);
-	for (i = 0; i < cp->key_count; ++i) {
+	cp->key_count = cpu_to_le16(key_count);
+	for (i = 0; i < key_count; ++i) {
 		cp->keys[i].type = blocked_keys[i].type;
 		memcpy(cp->keys[i].val, blocked_keys[i].val,
 						sizeof(cp->keys[i].val));
diff --git a/src/advertising.c b/src/advertising.c
index 1ed09c902..3f70fb4e9 100644
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -1042,7 +1042,7 @@ static int refresh_legacy_adv(struct btd_adv_client *client,
 
 	cp->flags = htobl(flags);
 	cp->instance = client->instance;
-	cp->duration = client->duration;
+	cp->duration = cpu_to_le16(client->duration);
 	cp->adv_data_len = adv_data_len;
 	cp->scan_rsp_len = scan_rsp_len;
 	memcpy(cp->data, adv_data, adv_data_len);
@@ -1093,13 +1093,13 @@ static int refresh_extended_adv(struct btd_adv_client *client,
 	 */
 
 	if (client->duration) {
-		cp.duration = client->duration;
+		cp.duration = cpu_to_le16(client->duration);
 		flags |= MGMT_ADV_PARAM_DURATION;
 	}
 
 	if (client->min_interval && client->max_interval) {
-		cp.min_interval = client->min_interval;
-		cp.max_interval = client->max_interval;
+		cp.min_interval = cpu_to_le32(client->min_interval);
+		cp.max_interval = cpu_to_le32(client->max_interval);
 		flags |= MGMT_ADV_PARAM_INTERVALS;
 	}
 
-- 
2.34.1

Re: [PATCH BlueZ v2] adapter/advertising: fix mgmt endian bug

[Bastien Nocera]

On Thu, 2026-09-03 at 11:10 +0200, Nicolas Thibert wrote:
> Several places in adapter.c and advertising.c write a native-endian
> value directly into a __le16/__le32 field of an mgmt command struct,
> skipping the cpu_to_le16()/cpu_to_le32() conversion used everywhere
> else in this codebase for the same purpose. This is a no-op on
> little-endian hosts (where cpu_to_le16()/cpu_to_le32() are themselves
> no-ops), which is why it has gone unnoticed, but corrupts the value
> on
> big-endian hosts.
> 
> Confirmed live on MIPS big-endian (OpenWrt/ath79): set_blocked_keys()
> sends key_count=2 (2 blocked keys), which the kernel's
> __le16_to_cpu() correctly interprets as 512 (0x0002 byte-swapped is
> 0x0200) since the wire bytes were never actually swapped to little-
> endian on the way out -- producing "expected 8706 bytes, got 36
> bytes" / "Failed to set blocked keys: Invalid Parameters" errors in
> dmesg/bluetoothd logs.
> 
> add_advertising()'s cp->duration and refresh_extended_adv()'s
> cp.duration/cp.min_interval/cp.max_interval have the identical bug.
> It
> is inert with bluetoothctl's default (0) duration/interval values (0
> byte-swapped is still 0), but A/B tested live (patch removed vs.
> applied, bluetoothctl's advertise submenu "interval 100 100" set
> explicitly) confirms this is not just a theoretical correctness fix:
> with the bug present, the device stops being discoverable by a real
> BLE scanner the moment a non-default interval is requested, and
> becomes discoverable again immediately once patched. Any application
> that sets an explicit advertising interval or duration hits this.
> 
> v2: use cpu_to_le16()/cpu_to_le32() instead of htobs()/htobl(), as
> requested by Luiz. Retested live on the same MIPS big-endian board
> (blocked keys + explicit advertising interval): behaves identically
> to the v1 fix, no regressions.

This paragraph above, goes below the "---" underneath this comment,
just above the diff stats.

configuration is 
[sendemail]
  annotate = true
or --annotate on the command-line.

I believe Luiz might be able to fix this before pushing the patch.

> ---
>  src/adapter.c     | 5 +++--
>  src/advertising.c | 8 ++++----
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/src/adapter.c b/src/adapter.c
> index 7390ceeee..edbdb53b5 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -10219,10 +10219,11 @@ static bool set_blocked_keys(struct
> btd_adapter *adapter)
>  					sizeof(blocked_keys)] = { 0
> };
>  	struct mgmt_cp_set_blocked_keys *cp =
>  				(struct mgmt_cp_set_blocked_keys
> *)buffer;
> +	const uint16_t key_count = ARRAY_SIZE(blocked_keys);
>  	int i;
>  
> -	cp->key_count = ARRAY_SIZE(blocked_keys);
> -	for (i = 0; i < cp->key_count; ++i) {
> +	cp->key_count = cpu_to_le16(key_count);
> +	for (i = 0; i < key_count; ++i) {
>  		cp->keys[i].type = blocked_keys[i].type;
>  		memcpy(cp->keys[i].val, blocked_keys[i].val,
>  						sizeof(cp-
> >keys[i].val));
> diff --git a/src/advertising.c b/src/advertising.c
> index 1ed09c902..3f70fb4e9 100644
> --- a/src/advertising.c
> +++ b/src/advertising.c
> @@ -1042,7 +1042,7 @@ static int refresh_legacy_adv(struct
> btd_adv_client *client,
>  
>  	cp->flags = htobl(flags);
>  	cp->instance = client->instance;
> -	cp->duration = client->duration;
> +	cp->duration = cpu_to_le16(client->duration);
>  	cp->adv_data_len = adv_data_len;
>  	cp->scan_rsp_len = scan_rsp_len;
>  	memcpy(cp->data, adv_data, adv_data_len);
> @@ -1093,13 +1093,13 @@ static int refresh_extended_adv(struct
> btd_adv_client *client,
>  	 */
>  
>  	if (client->duration) {
> -		cp.duration = client->duration;
> +		cp.duration = cpu_to_le16(client->duration);
>  		flags |= MGMT_ADV_PARAM_DURATION;
>  	}
>  
>  	if (client->min_interval && client->max_interval) {
> -		cp.min_interval = client->min_interval;
> -		cp.max_interval = client->max_interval;
> +		cp.min_interval = cpu_to_le32(client->min_interval);
> +		cp.max_interval = cpu_to_le32(client->max_interval);
>  		flags |= MGMT_ADV_PARAM_INTERVALS;
>  	}
>  

RE: [BlueZ,v2] adapter/advertising: fix mgmt endian bug

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PASS      0.28 seconds
GitLint                       PASS      0.20 seconds
BuildEll                      PASS      20.82 seconds
BluezMake                     PASS      619.17 seconds
CheckSmatch                   WARNING   324.39 seconds
bluezmakeextell               PASS      105.83 seconds
IncrementalBuild              PASS      614.67 seconds
ScanBuild                     PASS      1034.60 seconds

Details
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/advertising.c: note: in included file:./src/shared/mgmt.h:95:25: error: redefinition of unsigned int enum mgmt_io_capabilitysrc/advertising.c: note: in included file:./src/shared/mgmt.h:95:25: error: redefinition of unsigned int enum mgmt_io_capability


https://github.com/bluez/bluez/pull/2478

---
Regards,
Linux Bluetooth