Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-680-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20231220173052.1617172-1-xiaokeqinhealth@126.com>
In-Reply-To: <20231220173052.1617172-1-xiaokeqinhealth@126.com>
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: Wed, 20 Dec 2023 12:34:21 -0500
Message-ID: <CABBYNZKU+iHedypune81X597ReL66Gn3genntas9u3t92Px2_w@mail.gmail.com>
Subject: Re: [PATCH v2] adapter: Fix link key address type for old kernels
To: Xiao Yao <xiaokeqinhealth@126.com>
Cc: linux-bluetooth@vger.kernel.org, antiz@archlinux.org, 
	Xiao Yao <xiaoyao@rock-chips.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Xiao,

On Wed, Dec 20, 2023 at 12:31=E2=80=AFPM Xiao Yao <xiaokeqinhealth@126.com>=
 wrote:
>
> From: Xiao Yao <xiaoyao@rock-chips.com>
>
> According to the Bluetooth specification, the address
> type of the link key is not fixed. However, the
> load_link_keys function in the old kernel code requires
> that the address type must be BDADDR_BREDR, so attempt
> it when the first load fails.
>
> Fixes: https://github.com/bluez/bluez/issues/686
>
> Signed-off-by: Xiao Yao <xiaoyao@rock-chips.com>
> ---
> v1 -> v2
> Prioritize loading keys with standard address types,
> and switch to BREDR address types upon failure, as
> suggested by the maintainer.
> ---
>  src/adapter.c | 45 ++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 36 insertions(+), 9 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index ee70b00d2..e1b704ecc 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -168,6 +168,9 @@ static GSList *adapter_drivers =3D NULL;
>  static GSList *disconnect_list =3D NULL;
>  static GSList *conn_fail_list =3D NULL;
>
> +static GSList *link_keys =3D NULL;
> +static bool link_keys_brder =3D false;
> +
>  struct link_key_info {
>         bdaddr_t bdaddr;
>         uint8_t bdaddr_type;
> @@ -4284,23 +4287,45 @@ static int set_privacy(struct btd_adapter *adapte=
r, uint8_t privacy)
>         return -1;
>  }
>
> +static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
> +                          bool debug_keys, bool link_key_bredr);
> +
>  static void load_link_keys_complete(uint8_t status, uint16_t length,
>                                         const void *param, void *user_dat=
a)
>  {
>         struct btd_adapter *adapter =3D user_data;
>
>         if (status !=3D MGMT_STATUS_SUCCESS) {
> +               /*
> +                * According to the Bluetooth specification, the address
> +                * type of the link key is not fixed. However, the
> +                * load_link_keys function in the old kernel code require=
s
> +                * that the address type must be BDADDR_BREDR, so attempt=
 it.
> +                */
> +               if (link_keys_brder =3D=3D false && status =3D=3D 0x0d) {
> +                       link_keys_brder =3D true;
> +                       load_link_keys(adapter, link_keys, btd_opts.debug=
_keys,
> +                                      link_keys_brder);
> +                       return;
> +               }
> +
>                 btd_error(adapter->dev_id,
>                         "Failed to load link keys for hci%u: %s (0x%02x)"=
,
>                                 adapter->dev_id, mgmt_errstr(status), sta=
tus);
> -               return;
> +
> +               goto free;
>         }
>
>         DBG("link keys loaded for hci%u", adapter->dev_id);
> +
> +free:
> +       g_slist_free_full(link_keys, g_free);
> +       link_keys =3D NULL;
> +       link_keys_brder =3D false;
>  }
>
>  static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
> -                                                       bool debug_keys)
> +                          bool debug_keys, bool link_key_bredr)
>  {
>         struct mgmt_cp_load_link_keys *cp;
>         struct mgmt_link_key_info *key;
> @@ -4320,8 +4345,8 @@ static void load_link_keys(struct btd_adapter *adap=
ter, GSList *keys,
>
>         key_count =3D g_slist_length(keys);
>
> -       DBG("hci%u keys %zu debug_keys %d", adapter->dev_id, key_count,
> -                                                               debug_key=
s);
> +       DBG("hci%u keys %zu debug_keys %d (%s)", adapter->dev_id, key_cou=
nt,
> +                       debug_keys, link_key_bredr ? "force bredr" : "nor=
mal");
>
>         cp_size =3D sizeof(*cp) + (key_count * sizeof(*key));
>
> @@ -4347,7 +4372,10 @@ static void load_link_keys(struct btd_adapter *ada=
pter, GSList *keys,
>                 struct link_key_info *info =3D l->data;
>
>                 bacpy(&key->addr.bdaddr, &info->bdaddr);
> -               key->addr.type =3D info->bdaddr_type;
> +               if (link_key_bredr)
> +                       key->addr.type =3D BDADDR_BREDR;
> +               else
> +                       key->addr.type =3D info->bdaddr_type;
>                 key->type =3D info->type;
>                 memcpy(key->val, info->key, 16);
>                 key->pin_len =3D info->pin_len;
> @@ -4873,7 +4901,6 @@ done:
>  static void load_devices(struct btd_adapter *adapter)
>  {
>         char dirname[PATH_MAX];
> -       GSList *keys =3D NULL;
>         GSList *ltks =3D NULL;
>         GSList *irks =3D NULL;
>         GSList *params =3D NULL;
> @@ -4964,7 +4991,7 @@ static void load_devices(struct btd_adapter *adapte=
r)
>                 }
>
>                 if (key_info)
> -                       keys =3D g_slist_append(keys, key_info);
> +                       link_keys =3D g_slist_append(link_keys, key_info)=
;
>
>                 if (ltk_info)
>                         ltks =3D g_slist_append(ltks, ltk_info);
> @@ -5013,8 +5040,8 @@ free:
>
>         closedir(dir);
>
> -       load_link_keys(adapter, keys, btd_opts.debug_keys);
> -       g_slist_free_full(keys, g_free);
> +       load_link_keys(adapter, link_keys, btd_opts.debug_keys,
> +                       link_keys_brder);
>
>         load_ltks(adapter, ltks);
>         g_slist_free_full(ltks, g_free);
> --
> 2.34.1

Ive just sent a similar fix:

https://patchwork.kernel.org/project/bluetooth/patch/20231220172222.2333064=
-1-luiz.dentz@gmail.com/

>


--=20
Luiz Augusto von Dentz