linux-kernel - Re: [PATCH] Bluetooth: Honor name resolve evt regardless of discov state

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 10 Aug 2022 12:58:36 -0700
From:   Luiz Augusto von Dentz <luiz.dentz@...il.com>
To:     Archie Pusaka <apusaka@...gle.com>
Cc:     linux-bluetooth <linux-bluetooth@...r.kernel.org>,
        Marcel Holtmann <marcel@...tmann.org>,
        CrosBT Upstreaming <chromeos-bluetooth-upstreaming@...omium.org>,
        Archie Pusaka <apusaka@...omium.org>,
        Ying Hsu <yinghsu@...omium.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "open list:NETWORKING [GENERAL]" <netdev@...r.kernel.org>
Subject: Re: [PATCH] Bluetooth: Honor name resolve evt regardless of discov state

Hi Archie,

On Wed, Aug 10, 2022 at 1:47 AM Archie Pusaka <apusaka@...gle.com> wrote:
>
> From: Archie Pusaka <apusaka@...omium.org>
>
> Currently, we don't update the name resolving cache when receiving
> a name resolve event if the discovery phase is not in the resolving
> stage.
>
> However, if the user connect to a device while we are still resolving
> remote name for another device, discovery will be stopped, and because
> we are no longer in the discovery resolving phase, the corresponding
> remote name event will be ignored, and thus the device being resolved
> will stuck in NAME_PENDING state.
>
> If discovery is then restarted and then stopped, this will cause us to
> try cancelling the name resolve of the same device again, which is
> incorrect and might upset the controller.

Please add the Fixes tag.

> Signed-off-by: Archie Pusaka <apusaka@...omium.org>
> Reviewed-by: Ying Hsu <yinghsu@...omium.org>
>
> ---
> The following steps are performed:
>     (1) Prepare 2 classic peer devices that needs RNR. Put device A
>         closer to DUT and device B (much) farther from DUT.
>     (2) Remove all cache and previous connection from DUT
>     (3) Put both peers into pairing mode, then start scanning on DUT
>     (4) After ~8 sec, turn off peer B.
>     *This is done so DUT can discover peer B (discovery time is 10s),
>     but it hasn't started RNR. Peer is turned off to buy us the max
>     time in the RNR phase (5s).
>     (5) Immediately as device A is shown on UI, click to connect.
>     *We thus know that the DUT is in the RNR phase and trying to
>     resolve the name of peer B when we initiate connection to peer A.
>     (6) Forget peer A.
>     (7) Restart scan and stop scan.
>     *Before the CL, stop scan is broken because we will try to cancel
>     a nonexistent RNR
>     (8) Restart scan again. Observe DUT can scan normally.
>
>
>  net/bluetooth/hci_event.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 395c6479456f..95e145e278c9 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2453,6 +2453,16 @@ static void hci_check_pending_name(struct hci_dev *hdev, struct hci_conn *conn,
>             !test_and_set_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags))
>                 mgmt_device_connected(hdev, conn, name, name_len);
>
> +       e = hci_inquiry_cache_lookup_resolve(hdev, bdaddr, NAME_PENDING);
> +
> +       if (e) {
> +               list_del(&e->list);
> +
> +               e->name_state = name ? NAME_KNOWN : NAME_NOT_KNOWN;
> +               mgmt_remote_name(hdev, bdaddr, ACL_LINK, 0x00, e->data.rssi,
> +                                name, name_len);
> +       }
> +
>         if (discov->state == DISCOVERY_STOPPED)
>                 return;
>
> @@ -2462,7 +2472,6 @@ static void hci_check_pending_name(struct hci_dev *hdev, struct hci_conn *conn,
>         if (discov->state != DISCOVERY_RESOLVING)
>                 return;
>
> -       e = hci_inquiry_cache_lookup_resolve(hdev, bdaddr, NAME_PENDING);
>         /* If the device was not found in a list of found devices names of which
>          * are pending. there is no need to continue resolving a next name as it
>          * will be done upon receiving another Remote Name Request Complete
> @@ -2470,12 +2479,6 @@ static void hci_check_pending_name(struct hci_dev *hdev, struct hci_conn *conn,
>         if (!e)
>                 return;
>
> -       list_del(&e->list);
> -
> -       e->name_state = name ? NAME_KNOWN : NAME_NOT_KNOWN;
> -       mgmt_remote_name(hdev, bdaddr, ACL_LINK, 0x00, e->data.rssi,
> -                        name, name_len);
> -
>         if (hci_resolve_next_name(hdev))
>                 return;
>
> --
> 2.37.1.595.g718a3a8f04-goog
>


-- 
Luiz Augusto von Dentz