2020-03-24 19:50:38

by Alain Michaud

Subject: [PATCH v2] bluetooth: don't assume key size is 16 when the command fails.

With this change, the encryption key size is not assumed to be 16 if the
read_encryption_key_size command fails for any reason. This ensures
that if the controller fails the command for any reason that the
encryption key size isn't implicitely set to 16 and instead take a more
concervative posture to assume it is 0.

Signed-off-by: Alain Michaud <[email protected]>

---

net/bluetooth/hci_event.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index cd3d7d90029b..8a7a94e6f956 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2963,14 +2963,10 @@ static void read_enc_key_size_complete(struct hci_dev *hdev, u8 status,
if (!conn)
goto unlock;

- /* If we fail to read the encryption key size, assume maximum
- * (which is the same we do also when this HCI command isn't
- * supported.
- */
if (rp->status) {
bt_dev_err(hdev, "failed to read key size for handle %u",
handle);
- conn->enc_key_size = HCI_LINK_KEY_SIZE;
+ conn->enc_key_size = 0;
} else {
conn->enc_key_size = rp->key_size;
}
--
2.25.1.696.g5e7596f4ac-goog

2020-03-25 08:32:13

by Marcel Holtmann

Subject: Re: [PATCH v2] bluetooth: don't assume key size is 16 when the command fails.

Hi Alain,

can please prefix the subject with “Bluetooth: “ then I have to stop fixing it up after applying ;)

> With this change, the encryption key size is not assumed to be 16 if the
> read_encryption_key_size command fails for any reason. This ensures
> that if the controller fails the command for any reason that the
> encryption key size isn't implicitely set to 16 and instead take a more
> concervative posture to assume it is 0.
>
> Signed-off-by: Alain Michaud <[email protected]>
>
> ---
>
> net/bluetooth/hci_event.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index cd3d7d90029b..8a7a94e6f956 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2963,14 +2963,10 @@ static void read_enc_key_size_complete(struct hci_dev *hdev, u8 status,
> if (!conn)
> goto unlock;
>
> - /* If we fail to read the encryption key size, assume maximum
> - * (which is the same we do also when this HCI command isn't
> - * supported.
> - */

/* When the command to read the encryption key size fails, then this
* normally indicates a bad handle or an invalid handle. However in
* this case we are pretty certain that the parameters provided have
* been correct. This means that if the command fails, something
* serious went wrong between the controller and the host. The best
* approach is to fail the connection now and for that the key size
* is set to 0 to force a disconnect.
*/

> if (rp->status) {
> bt_dev_err(hdev, "failed to read key size for handle %u",
> handle);
> - conn->enc_key_size = HCI_LINK_KEY_SIZE;
> + conn->enc_key_size = 0;
> } else {
> conn->enc_key_size = rp->key_size;
> }

So I wouldn’t scrap the comment. I would extend it to something like above. Feel free to copy it or reword or use your own words. I just want a comment there that when I read the code in 6 month from now, I remember why this is done.

Regards

Marcel