lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ