[PATCH] Bluetooth: hci_event: Mask data status from LE ext adv reports

[PATCH] Bluetooth: hci_event: Mask data status from LE ext adv reports

[Chris Down]

The Event_Type field in an LE Extended Advertising Report uses bits 5
and 6 for data status (e.g. fragmentation), not the PDU type itself.

The ext_evt_type_to_legacy() function fails to mask these status bits
before evaluation. This causes valid advertisements with status bits set
(e.g. a fragmented non-connectable advertisement, which ends up showing
as PDU type 0x40) to be misclassified as unknown and subsequently
dropped. This is okay for most checks which use bitwise AND on the
relevant event type bits, but it doesn't work for non-connectable types,
which are checked with '== LE_EXT_ADV_NON_CONN_IND' (that is, zero).

This patch introduces a PDU type mask to ensure only the relevant bits
are evaluated, allowing for the correct translation of all valid
extended advertising packets.

Signed-off-by: Chris Down <chris@chrisdown.name>
Cc: linux-bluetooth@vger.kernel.org
---
 net/bluetooth/hci_event.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index c0eb03e5cbf8..077c93b5fae0 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -6237,10 +6237,14 @@ static void hci_le_adv_report_evt(struct hci_dev *hdev, void *data,
 	hci_dev_unlock(hdev);
 }
 
+#define LE_EXT_ADV_DATA_STATUS_MASK GENMASK(6, 5)
+
 static u8 ext_evt_type_to_legacy(struct hci_dev *hdev, u16 evt_type)
 {
-	if (evt_type & LE_EXT_ADV_LEGACY_PDU) {
-		switch (evt_type) {
+	u16 pdu_type = evt_type & ~LE_EXT_ADV_DATA_STATUS_MASK;
+
+	if (pdu_type & LE_EXT_ADV_LEGACY_PDU) {
+		switch (pdu_type) {
 		case LE_LEGACY_ADV_IND:
 			return LE_ADV_IND;
 		case LE_LEGACY_ADV_DIRECT_IND:
@@ -6257,21 +6261,21 @@ static u8 ext_evt_type_to_legacy(struct hci_dev *hdev, u16 evt_type)
 		goto invalid;
 	}
 
-	if (evt_type & LE_EXT_ADV_CONN_IND) {
-		if (evt_type & LE_EXT_ADV_DIRECT_IND)
+	if (pdu_type & LE_EXT_ADV_CONN_IND) {
+		if (pdu_type & LE_EXT_ADV_DIRECT_IND)
 			return LE_ADV_DIRECT_IND;
 
 		return LE_ADV_IND;
 	}
 
-	if (evt_type & LE_EXT_ADV_SCAN_RSP)
+	if (pdu_type & LE_EXT_ADV_SCAN_RSP)
 		return LE_ADV_SCAN_RSP;
 
-	if (evt_type & LE_EXT_ADV_SCAN_IND)
+	if (pdu_type & LE_EXT_ADV_SCAN_IND)
 		return LE_ADV_SCAN_IND;
 
-	if (evt_type == LE_EXT_ADV_NON_CONN_IND ||
-	    evt_type & LE_EXT_ADV_DIRECT_IND)
+	if (pdu_type == LE_EXT_ADV_NON_CONN_IND ||
+	    pdu_type & LE_EXT_ADV_DIRECT_IND)
 		return LE_ADV_NONCONN_IND;
 
 invalid:
-- 
2.49.0

RE: Bluetooth: hci_event: Mask data status from LE ext adv reports

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PENDING   0.27 seconds
GitLint                       PENDING   0.19 seconds
SubjectPrefix                 PASS      0.12 seconds
BuildKernel                   PASS      24.17 seconds
CheckAllWarning               PASS      26.64 seconds
CheckSparse                   WARNING   30.08 seconds
BuildKernel32                 PASS      23.90 seconds
TestRunnerSetup               PASS      471.53 seconds
TestRunner_l2cap-tester       PASS      25.45 seconds
TestRunner_iso-tester         PASS      40.15 seconds
TestRunner_bnep-tester        PASS      6.20 seconds
TestRunner_mgmt-tester        FAIL      132.04 seconds
TestRunner_rfcomm-tester      PASS      9.54 seconds
TestRunner_sco-tester         PASS      14.95 seconds
TestRunner_ioctl-tester       PASS      10.34 seconds
TestRunner_mesh-tester        FAIL      11.75 seconds
TestRunner_smp-tester         PASS      8.82 seconds
TestRunner_userchan-tester    PASS      6.44 seconds
IncrementalBuild              PENDING   0.78 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
##############################
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
LL Privacy - Add Device 3 (AL is full)               Failed       0.216 seconds
LL Privacy - Start Discovery 2 (Disable RL)          Failed       0.198 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.276 seconds
Mesh - Send cancel - 2                               Timed out    2.000 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

Re: [PATCH] Bluetooth: hci_event: Mask data status from LE ext adv reports

[Luiz Augusto von Dentz]

Hi Chris,

On Wed, Jul 16, 2025 at 1:14 PM Chris Down <chris@chrisdown.name> wrote:
>
> The Event_Type field in an LE Extended Advertising Report uses bits 5
> and 6 for data status (e.g. fragmentation), not the PDU type itself.
>
> The ext_evt_type_to_legacy() function fails to mask these status bits
> before evaluation. This causes valid advertisements with status bits set
> (e.g. a fragmented non-connectable advertisement, which ends up showing
> as PDU type 0x40) to be misclassified as unknown and subsequently
> dropped. This is okay for most checks which use bitwise AND on the
> relevant event type bits, but it doesn't work for non-connectable types,
> which are checked with '== LE_EXT_ADV_NON_CONN_IND' (that is, zero).

Can you include a sample trace of the above? Also it would be great to
have a mgmt-tester for example that attempts to generate an
advertisement like that to exercise such change.

> This patch introduces a PDU type mask to ensure only the relevant bits
> are evaluated, allowing for the correct translation of all valid
> extended advertising packets.
>
> Signed-off-by: Chris Down <chris@chrisdown.name>
> Cc: linux-bluetooth@vger.kernel.org
> ---
>  net/bluetooth/hci_event.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index c0eb03e5cbf8..077c93b5fae0 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -6237,10 +6237,14 @@ static void hci_le_adv_report_evt(struct hci_dev *hdev, void *data,
>         hci_dev_unlock(hdev);
>  }
>
> +#define LE_EXT_ADV_DATA_STATUS_MASK GENMASK(6, 5)



>  static u8 ext_evt_type_to_legacy(struct hci_dev *hdev, u16 evt_type)
>  {
> -       if (evt_type & LE_EXT_ADV_LEGACY_PDU) {
> -               switch (evt_type) {
> +       u16 pdu_type = evt_type & ~LE_EXT_ADV_DATA_STATUS_MASK;
> +
> +       if (pdu_type & LE_EXT_ADV_LEGACY_PDU) {
> +               switch (pdu_type) {
>                 case LE_LEGACY_ADV_IND:
>                         return LE_ADV_IND;
>                 case LE_LEGACY_ADV_DIRECT_IND:
> @@ -6257,21 +6261,21 @@ static u8 ext_evt_type_to_legacy(struct hci_dev *hdev, u16 evt_type)
>                 goto invalid;
>         }
>
> -       if (evt_type & LE_EXT_ADV_CONN_IND) {
> -               if (evt_type & LE_EXT_ADV_DIRECT_IND)
> +       if (pdu_type & LE_EXT_ADV_CONN_IND) {
> +               if (pdu_type & LE_EXT_ADV_DIRECT_IND)
>                         return LE_ADV_DIRECT_IND;
>
>                 return LE_ADV_IND;
>         }
>
> -       if (evt_type & LE_EXT_ADV_SCAN_RSP)
> +       if (pdu_type & LE_EXT_ADV_SCAN_RSP)
>                 return LE_ADV_SCAN_RSP;
>
> -       if (evt_type & LE_EXT_ADV_SCAN_IND)
> +       if (pdu_type & LE_EXT_ADV_SCAN_IND)
>                 return LE_ADV_SCAN_IND;
>
> -       if (evt_type == LE_EXT_ADV_NON_CONN_IND ||
> -           evt_type & LE_EXT_ADV_DIRECT_IND)
> +       if (pdu_type == LE_EXT_ADV_NON_CONN_IND ||

I'm not sure I would keep checking for  LE_EXT_ADV_NON_CONN_IND, maybe
just return LE_ADV_NONCONN_IND, LE_EXT_ADV_NON_CONN_IND is not
actually a bit it is the absence of any bits being set, so I guess the
only invalid adv are the ones for legacy which seem to require a bit
to be set.

> +           pdu_type & LE_EXT_ADV_DIRECT_IND)
>                 return LE_ADV_NONCONN_IND;
>
>  invalid:
> --
> 2.49.0
>
>


-- 
Luiz Augusto von Dentz

Re: [PATCH] Bluetooth: hci_event: Mask data status from LE ext adv reports

[Chris Down]

Hi Luiz,

Thanks for the review!

Luiz Augusto von Dentz writes:
>Can you include a sample trace of the above?

Is that with btmon or similar? Sorry, I'm not a regular contributor to this 
subsystem :-)

I mostly have a personal desire to get this merged because it's a particularly 
noisy case where I happen to live :-) These are all with 0x40:

   % dmesg | wc -l
   3815
   % dmesg | grep -c 'Unknown advertising'
   3227

>Also it would be great to  have a mgmt-tester for example that attempts to 
>generate an advertisement like that to exercise such change.

Looks like that's in Bluez userspace code right, so what's the order of doing 
these things?

>> -       if (evt_type == LE_EXT_ADV_NON_CONN_IND ||
>> -           evt_type & LE_EXT_ADV_DIRECT_IND)
>> +       if (pdu_type == LE_EXT_ADV_NON_CONN_IND ||
>
>I'm not sure I would keep checking for  LE_EXT_ADV_NON_CONN_IND, maybe
>just return LE_ADV_NONCONN_IND, LE_EXT_ADV_NON_CONN_IND is not
>actually a bit it is the absence of any bits being set, so I guess the
>only invalid adv are the ones for legacy which seem to require a bit
>to be set.

So are you thinking of doing this?

   if (!(pdu_type & ~(LE_EXT_ADV_DIRECT_IND)))
           return LE_ADV_NONCONN_IND;

Thanks for your help!

Chris

Re: [PATCH] Bluetooth: hci_event: Mask data status from LE ext adv reports

[Luiz Augusto von Dentz]

Hi Chris,

On Fri, Jul 18, 2025 at 4:13 AM Chris Down <chris@chrisdown.name> wrote:
>
> Hi Luiz,
>
> Thanks for the review!
>
> Luiz Augusto von Dentz writes:
> >Can you include a sample trace of the above?
>
> Is that with btmon or similar? Sorry, I'm not a regular contributor to this
> subsystem :-)
>
> I mostly have a personal desire to get this merged because it's a particularly
> noisy case where I happen to live :-) These are all with 0x40:
>
>    % dmesg | wc -l
>    3815
>    % dmesg | grep -c 'Unknown advertising'
>    3227

Try to capture one of them using btmon and then add to the patch description.

> >Also it would be great to  have a mgmt-tester for example that attempts to
> >generate an advertisement like that to exercise such change.
>
> Looks like that's in Bluez userspace code right, so what's the order of doing
> these things?
>
> >> -       if (evt_type == LE_EXT_ADV_NON_CONN_IND ||
> >> -           evt_type & LE_EXT_ADV_DIRECT_IND)
> >> +       if (pdu_type == LE_EXT_ADV_NON_CONN_IND ||
> >
> >I'm not sure I would keep checking for  LE_EXT_ADV_NON_CONN_IND, maybe
> >just return LE_ADV_NONCONN_IND, LE_EXT_ADV_NON_CONN_IND is not
> >actually a bit it is the absence of any bits being set, so I guess the
> >only invalid adv are the ones for legacy which seem to require a bit
> >to be set.
>
> So are you thinking of doing this?
>
>    if (!(pdu_type & ~(LE_EXT_ADV_DIRECT_IND)))
>            return LE_ADV_NONCONN_IND;

We can probably return early on if (!evt_type) return
LE_ADV_NONCONN_IND since there is no point in evaluating it if it is
zero.

> Thanks for your help!
>
> Chris



-- 
Luiz Augusto von Dentz

Re: [PATCH] Bluetooth: hci_event: Mask data status from LE ext adv reports

[Chris Down]

Hi Luiz,

>Try to capture one of them using btmon and then add to the patch description.

Thanks, I have one now and will add for v2.

>> >> -       if (evt_type == LE_EXT_ADV_NON_CONN_IND ||
>> >> -           evt_type & LE_EXT_ADV_DIRECT_IND)
>> >> +       if (pdu_type == LE_EXT_ADV_NON_CONN_IND ||
>> >
>> >I'm not sure I would keep checking for  LE_EXT_ADV_NON_CONN_IND, maybe
>> >just return LE_ADV_NONCONN_IND, LE_EXT_ADV_NON_CONN_IND is not
>> >actually a bit it is the absence of any bits being set, so I guess the
>> >only invalid adv are the ones for legacy which seem to require a bit
>> >to be set.
>>
>> So are you thinking of doing this?
>>
>>    if (!(pdu_type & ~(LE_EXT_ADV_DIRECT_IND)))
>>            return LE_ADV_NONCONN_IND;
>
>We can probably return early on if (!evt_type) return
>LE_ADV_NONCONN_IND since there is no point in evaluating it if it is
>zero.

I guess you meant `if (!pdu_type)`? That correctly handles the 0x40 case (where 
pdu_type becomes 0), but it would miss non-connectable directed advertisements 
(PDU type 0x04), right? Or maybe you meant something else?

Re: [PATCH] Bluetooth: hci_event: Mask data status from LE ext adv reports

[Luiz Augusto von Dentz]

Hi Chris,

On Sat, Jul 19, 2025 at 12:04 PM Chris Down <chris@chrisdown.name> wrote:
>
> Hi Luiz,
>
> >Try to capture one of them using btmon and then add to the patch description.
>
> Thanks, I have one now and will add for v2.
>
> >> >> -       if (evt_type == LE_EXT_ADV_NON_CONN_IND ||
> >> >> -           evt_type & LE_EXT_ADV_DIRECT_IND)
> >> >> +       if (pdu_type == LE_EXT_ADV_NON_CONN_IND ||
> >> >
> >> >I'm not sure I would keep checking for  LE_EXT_ADV_NON_CONN_IND, maybe
> >> >just return LE_ADV_NONCONN_IND, LE_EXT_ADV_NON_CONN_IND is not
> >> >actually a bit it is the absence of any bits being set, so I guess the
> >> >only invalid adv are the ones for legacy which seem to require a bit
> >> >to be set.
> >>
> >> So are you thinking of doing this?
> >>
> >>    if (!(pdu_type & ~(LE_EXT_ADV_DIRECT_IND)))
> >>            return LE_ADV_NONCONN_IND;
> >
> >We can probably return early on if (!evt_type) return
> >LE_ADV_NONCONN_IND since there is no point in evaluating it if it is
> >zero.
>
> I guess you meant `if (!pdu_type)`? That correctly handles the 0x40 case (where
> pdu_type becomes 0), but it would miss non-connectable directed advertisements
> (PDU type 0x04), right? Or maybe you meant something else?

Yes, we can just test for !pdu_type and return LE_ADV_NONCONN_IND
skipping any testing of bits.

-- 
Luiz Augusto von Dentz