[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABBYNZJo48983SWhxcB7UzWXPeUofRCMhQ8mJjih-rJoTET3_Q@mail.gmail.com>
Date: Thu, 17 Jul 2025 15:12:06 -0400
From: Luiz Augusto von Dentz <luiz.dentz@...il.com>
To: Chris Down <chris@...isdown.name>
Cc: linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org,
kernel-team@...com, Jaganath Kanakkassery <jaganath.k.os@...il.com>
Subject: Re: [PATCH] Bluetooth: hci_event: Mask data status from LE ext adv reports
Hi Chris,
On Wed, Jul 16, 2025 at 1:14 PM Chris Down <chris@...isdown.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@...isdown.name>
> Cc: linux-bluetooth@...r.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
Powered by blists - more mailing lists