* [PATCH BlueZ] adapter/advertising: fix mgmt endian bug
@ 2026-09-02 14:00 Nicolas Thibert
2026-09-02 14:21 ` Luiz Augusto von Dentz
2026-09-02 17:16 ` [BlueZ] " bluez.test.bot
0 siblings, 2 replies; 6+ messages in thread
From: Nicolas Thibert @ 2026-09-02 14:00 UTC (permalink / raw)
To: linux-bluetooth
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 htobs()/htobl() conversion used everywhere else in this
codebase for the same purpose. This is a no-op on little-endian hosts
(where htobs()/htobl() 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.
---
src/adapter.c | 5 +++--
src/advertising.c | 6 +++---
2 files changed, 6 insertions(+), 5 deletions(-)
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -9938,10 +9938,11 @@ static bool set_blocked_keys(struct btd_
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 = htobs(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));
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -995,7 +995,7 @@ static int refresh_legacy_adv(struct btd
cp->flags = htobl(flags);
cp->instance = client->instance;
- cp->duration = client->duration;
+ cp->duration = htobs(client->duration);
cp->adv_data_len = adv_data_len;
cp->scan_rsp_len = scan_rsp_len;
memcpy(cp->data, adv_data, adv_data_len);
@@ -1046,13 +1046,13 @@ static int refresh_extended_adv(struct b
*/
if (client->duration) {
- cp.duration = client->duration;
+ cp.duration = htobs(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 = htobl(client->min_interval);
+ cp.max_interval = htobl(client->max_interval);
flags |= MGMT_ADV_PARAM_INTERVALS;
}
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH BlueZ] adapter/advertising: fix mgmt endian bug
2026-09-02 14:00 [PATCH BlueZ] adapter/advertising: fix mgmt endian bug Nicolas Thibert
@ 2026-09-02 14:21 ` Luiz Augusto von Dentz
2026-09-02 16:07 ` Nicolas Thibert
2026-09-02 17:16 ` [BlueZ] " bluez.test.bot
1 sibling, 1 reply; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2026-09-02 14:21 UTC (permalink / raw)
To: Nicolas Thibert; +Cc: linux-bluetooth
Hi Nicolas,
On Wed, Sep 2, 2026 at 10:19 AM Nicolas Thibert <nithibert@gmail.com> 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 htobs()/htobl() conversion used everywhere else in this
> codebase for the same purpose. This is a no-op on little-endian hosts
> (where htobs()/htobl() 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.
> ---
> src/adapter.c | 5 +++--
> src/advertising.c | 6 +++---
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -9938,10 +9938,11 @@ static bool set_blocked_keys(struct btd_
> 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 = htobs(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));
> --- a/src/advertising.c
> +++ b/src/advertising.c
> @@ -995,7 +995,7 @@ static int refresh_legacy_adv(struct btd
>
> cp->flags = htobl(flags);
> cp->instance = client->instance;
> - cp->duration = client->duration;
> + cp->duration = htobs(client->duration);
> cp->adv_data_len = adv_data_len;
> cp->scan_rsp_len = scan_rsp_len;
> memcpy(cp->data, adv_data, adv_data_len);
> @@ -1046,13 +1046,13 @@ static int refresh_extended_adv(struct b
> */
> if (client->duration) {
> - cp.duration = client->duration;
> + cp.duration = htobs(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 = htobl(client->min_interval);
> + cp.max_interval = htobl(client->max_interval);
Use cpu_to_le** instead of htobl.
> flags |= MGMT_ADV_PARAM_INTERVALS;
> }
>
> --
> 2.34.1
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH BlueZ] adapter/advertising: fix mgmt endian bug
2026-09-02 14:21 ` Luiz Augusto von Dentz
@ 2026-09-02 16:07 ` Nicolas Thibert
2026-09-03 7:27 ` Nicolas Thibert
0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Thibert @ 2026-09-02 16:07 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
Ty for your answer, here is a correction
Regards,
Nicolas
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 | 6 +++---
2 files changed, 6 insertions(+), 5 deletions(-)
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -9938,10 +9938,11 @@ static bool set_blocked_keys(struct btd_
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));
+ 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));
--- a/src/advertising.c
+++ b/src/advertising.c
memcpy(cp->keys[i].val, blocked_keys[i].val,
sizeof(cp->keys[i].val));
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -995,7 +995,7 @@ static int refresh_legacy_adv(struct btd
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);
@@ -1046,13 +1046,13 @@ static int refresh_extended_adv(struct b
*/
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
Le mer. 2 sept. 2026 à 16:21, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> a écrit :
>
> Hi Nicolas,
>
> On Wed, Sep 2, 2026 at 10:19 AM Nicolas Thibert <nithibert@gmail.com> 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 htobs()/htobl() conversion used everywhere else in this
> > codebase for the same purpose. This is a no-op on little-endian hosts
> > (where htobs()/htobl() 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.
> > ---
> > src/adapter.c | 5 +++--
> > src/advertising.c | 6 +++---
> > 2 files changed, 6 insertions(+), 5 deletions(-)
> >
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> > @@ -9938,10 +9938,11 @@ static bool set_blocked_keys(struct btd_
> > 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 = htobs(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));
> > --- a/src/advertising.c
> > +++ b/src/advertising.c
> > @@ -995,7 +995,7 @@ static int refresh_legacy_adv(struct btd
> >
> > cp->flags = htobl(flags);
> > cp->instance = client->instance;
> > - cp->duration = client->duration;
> > + cp->duration = htobs(client->duration);
> > cp->adv_data_len = adv_data_len;
> > cp->scan_rsp_len = scan_rsp_len;
> > memcpy(cp->data, adv_data, adv_data_len);
> > @@ -1046,13 +1046,13 @@ static int refresh_extended_adv(struct b
> > */
> > if (client->duration) {
> > - cp.duration = client->duration;
> > + cp.duration = htobs(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 = htobl(client->min_interval);
> > + cp.max_interval = htobl(client->max_interval);
>
> Use cpu_to_le** instead of htobl.
>
> > flags |= MGMT_ADV_PARAM_INTERVALS;
> > }
> >
> > --
> > 2.34.1
> >
>
>
> --
> Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [BlueZ] adapter/advertising: fix mgmt endian bug
2026-09-02 14:00 [PATCH BlueZ] adapter/advertising: fix mgmt endian bug Nicolas Thibert
2026-09-02 14:21 ` Luiz Augusto von Dentz
@ 2026-09-02 17:16 ` bluez.test.bot
1 sibling, 0 replies; 6+ messages in thread
From: bluez.test.bot @ 2026-09-02 17:16 UTC (permalink / raw)
To: linux-bluetooth, nithibert
[-- Attachment #1: Type: text/plain, Size: 477 bytes --]
This is an automated email and please do not reply to this email.
Dear Submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository.
----- Output -----
error: corrupt patch at line 9
hint: Use 'git am --show-current-patch' to see the failed patch
Please resolve the issue and submit the patches again.
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH BlueZ] adapter/advertising: fix mgmt endian bug
2026-09-02 16:07 ` Nicolas Thibert
@ 2026-09-03 7:27 ` Nicolas Thibert
2026-09-03 8:14 ` Bastien Nocera
0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Thibert @ 2026-09-03 7:27 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz it seems the patch didnt apply correctly, here is a corrected version
Regards
Nicolas
---
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
Le mer. 2 sept. 2026 à 18:07, Nicolas Thibert <nithibert@gmail.com> a écrit :
>
> Hi Luiz,
> Ty for your answer, here is a correction
> Regards,
> Nicolas
>
> 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 | 6 +++---
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -9938,10 +9938,11 @@ static bool set_blocked_keys(struct btd_
> 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));
> + 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));
> --- a/src/advertising.c
> +++ b/src/advertising.c
> memcpy(cp->keys[i].val, blocked_keys[i].val,
> sizeof(cp->keys[i].val));
> --- a/src/advertising.c
> +++ b/src/advertising.c
> @@ -995,7 +995,7 @@ static int refresh_legacy_adv(struct btd
>
> 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);
> @@ -1046,13 +1046,13 @@ static int refresh_extended_adv(struct b
> */
>
> 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
>
> Le mer. 2 sept. 2026 à 16:21, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> a écrit :
> >
> > Hi Nicolas,
> >
> > On Wed, Sep 2, 2026 at 10:19 AM Nicolas Thibert <nithibert@gmail.com> 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 htobs()/htobl() conversion used everywhere else in this
> > > codebase for the same purpose. This is a no-op on little-endian hosts
> > > (where htobs()/htobl() 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.
> > > ---
> > > src/adapter.c | 5 +++--
> > > src/advertising.c | 6 +++---
> > > 2 files changed, 6 insertions(+), 5 deletions(-)
> > >
> > > --- a/src/adapter.c
> > > +++ b/src/adapter.c
> > > @@ -9938,10 +9938,11 @@ static bool set_blocked_keys(struct btd_
> > > 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 = htobs(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));
> > > --- a/src/advertising.c
> > > +++ b/src/advertising.c
> > > @@ -995,7 +995,7 @@ static int refresh_legacy_adv(struct btd
> > >
> > > cp->flags = htobl(flags);
> > > cp->instance = client->instance;
> > > - cp->duration = client->duration;
> > > + cp->duration = htobs(client->duration);
> > > cp->adv_data_len = adv_data_len;
> > > cp->scan_rsp_len = scan_rsp_len;
> > > memcpy(cp->data, adv_data, adv_data_len);
> > > @@ -1046,13 +1046,13 @@ static int refresh_extended_adv(struct b
> > > */
> > > if (client->duration) {
> > > - cp.duration = client->duration;
> > > + cp.duration = htobs(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 = htobl(client->min_interval);
> > > + cp.max_interval = htobl(client->max_interval);
> >
> > Use cpu_to_le** instead of htobl.
> >
> > > flags |= MGMT_ADV_PARAM_INTERVALS;
> > > }
> > >
> > > --
> > > 2.34.1
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH BlueZ] adapter/advertising: fix mgmt endian bug
2026-09-03 7:27 ` Nicolas Thibert
@ 2026-09-03 8:14 ` Bastien Nocera
0 siblings, 0 replies; 6+ messages in thread
From: Bastien Nocera @ 2026-09-03 8:14 UTC (permalink / raw)
To: Nicolas Thibert, Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hello Nicolas,
I think you'll want to double-check your setup for sending bluez
patches, as there were multiple problems with it.
The canonical documentation for sending patches to kernel mailing-lists
is here:
https://www.kernel.org/doc/html/v7.0/process/submitting-patches.html
1) The subject prefix mentions BlueZ (good) but not v2
2) The "---" in your message will make git stop trying to process the
rest of the mail, so your commit message and patch will be dropped
3) version changelog should go below the "---" that's below the commit
message, this will be dropped by git so that the changelog only appears
on the mailing-list, not in the git repo.
4) your mail client (or website) mangled the patch
Best solution is either to figure out how to set up "git-send-email" or
use a desktop mail client that can take patches without modifying them
(I know that in Evolution, you can drag'n'drop .patch files on the
drafts folder, and set the whole text to "preformatted")
Cheers
On Thu, 2026-09-03 at 09:27 +0200, Nicolas Thibert wrote:
> Hi Luiz it seems the patch didnt apply correctly, here is a corrected
> version
> Regards
> Nicolas
>
> ---
> 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
>
> Le mer. 2 sept. 2026 à 18:07, Nicolas Thibert <nithibert@gmail.com> a
> écrit :
> >
> > Hi Luiz,
> > Ty for your answer, here is a correction
> > Regards,
> > Nicolas
> >
> > 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 | 6 +++---
> > 2 files changed, 6 insertions(+), 5 deletions(-)
> >
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> > @@ -9938,10 +9938,11 @@ static bool set_blocked_keys(struct btd_
> > 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));
> > + 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));
> > --- a/src/advertising.c
> > +++ b/src/advertising.c
> > memcpy(cp->keys[i].val, blocked_keys[i].val,
> > sizeof(cp->keys[i].val));
> > --- a/src/advertising.c
> > +++ b/src/advertising.c
> > @@ -995,7 +995,7 @@ static int refresh_legacy_adv(struct btd
> >
> > 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);
> > @@ -1046,13 +1046,13 @@ static int refresh_extended_adv(struct b
> > */
> >
> > 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
> >
> > Le mer. 2 sept. 2026 à 16:21, Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> a écrit :
> > >
> > > Hi Nicolas,
> > >
> > > On Wed, Sep 2, 2026 at 10:19 AM Nicolas Thibert
> > > <nithibert@gmail.com> 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 htobs()/htobl() conversion used everywhere else in
> > > > this
> > > > codebase for the same purpose. This is a no-op on little-endian
> > > > hosts
> > > > (where htobs()/htobl() 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.
> > > > ---
> > > > src/adapter.c | 5 +++--
> > > > src/advertising.c | 6 +++---
> > > > 2 files changed, 6 insertions(+), 5 deletions(-)
> > > >
> > > > --- a/src/adapter.c
> > > > +++ b/src/adapter.c
> > > > @@ -9938,10 +9938,11 @@ static bool set_blocked_keys(struct
> > > > btd_
> > > > 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 = htobs(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));
> > > > --- a/src/advertising.c
> > > > +++ b/src/advertising.c
> > > > @@ -995,7 +995,7 @@ static int refresh_legacy_adv(struct btd
> > > >
> > > > cp->flags = htobl(flags);
> > > > cp->instance = client->instance;
> > > > - cp->duration = client->duration;
> > > > + cp->duration = htobs(client->duration);
> > > > cp->adv_data_len = adv_data_len;
> > > > cp->scan_rsp_len = scan_rsp_len;
> > > > memcpy(cp->data, adv_data, adv_data_len);
> > > > @@ -1046,13 +1046,13 @@ static int refresh_extended_adv(struct
> > > > b
> > > > */
> > > > if (client->duration) {
> > > > - cp.duration = client->duration;
> > > > + cp.duration = htobs(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 = htobl(client->min_interval);
> > > > + cp.max_interval = htobl(client->max_interval);
> > >
> > > Use cpu_to_le** instead of htobl.
> > >
> > > > flags |= MGMT_ADV_PARAM_INTERVALS;
> > > > }
> > > >
> > > > --
> > > > 2.34.1
> > > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-03 8:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02 14:00 [PATCH BlueZ] adapter/advertising: fix mgmt endian bug Nicolas Thibert
2026-09-02 14:21 ` Luiz Augusto von Dentz
2026-09-02 16:07 ` Nicolas Thibert
2026-09-03 7:27 ` Nicolas Thibert
2026-09-03 8:14 ` Bastien Nocera
2026-09-02 17:16 ` [BlueZ] " bluez.test.bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox