diff mbox series

[v1] Bluetooth: Tolerate valid pre 4.2 conn params

Message ID 20210217142511.3221605-1-alainm@chromium.org
State New
Headers show
Series [v1] Bluetooth: Tolerate valid pre 4.2 conn params | expand

Commit Message

Alain Michaud Feb. 17, 2021, 2:25 p.m. UTC
This patch tolerates connection parameters that were valid before
ESR08_v1.0.0 which was also incoporated into the 4.2 core specification.

In particular, this addresses compatibility issues with the Lenovo Yoga
Mouse that sees its connection parameters rejected by the max_latency
computation in hci_conn_check_params, but was perfectly valid at the
time this mouse was qualified.

Before the patch, the mouse was extremely laggy, after the patch, the
mouse worked as expected.

Signed-off-by: Alain Michaud <alainm@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Tested-by: Harry Cutts <hcutts@chromium.org>

---

 include/net/bluetooth/hci_core.h | 30 ++++++++++++++++++++++++++++++
 net/bluetooth/l2cap_core.c       | 12 ++++++++++++
 2 files changed, 42 insertions(+)

Comments

bluez.test.bot@gmail.com Feb. 17, 2021, 3:11 p.m. UTC | #1
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=434523

---Test result---

##############################
    Test: CheckPatch - PASS
    

    ##############################
    Test: CheckGitLint - PASS
    

    ##############################
    Test: CheckBuildK - PASS
    

    ##############################
    Test: CheckTestRunner: Setup - PASS
    

    ##############################
    Test: CheckTestRunner: l2cap-tester - PASS
    Total: 40, Passed: 34 (85.0%), Failed: 0, Not Run: 6

    ##############################
    Test: CheckTestRunner: bnep-tester - PASS
    Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

    ##############################
    Test: CheckTestRunner: mgmt-tester - PASS
    Total: 416, Passed: 402 (96.6%), Failed: 0, Not Run: 14

    ##############################
    Test: CheckTestRunner: rfcomm-tester - PASS
    Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

    ##############################
    Test: CheckTestRunner: sco-tester - PASS
    Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

    ##############################
    Test: CheckTestRunner: smp-tester - PASS
    Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

    ##############################
    Test: CheckTestRunner: userchan-tester - PASS
    Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0

    

---
Regards,
Linux Bluetooth
Alain Michaud Feb. 18, 2021, 8:06 p.m. UTC | #2
+Marcel for viz


On Wed, Feb 17, 2021 at 10:11 AM <bluez.test.bot@gmail.com> wrote:
>

> This is automated email and please do not reply to this email!

>

> Dear submitter,

>

> Thank you for submitting the patches to the linux bluetooth mailing list.

> This is a CI test results with your patch series:

> PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=434523

>

> ---Test result---

>

> ##############################

>     Test: CheckPatch - PASS

>

>

>     ##############################

>     Test: CheckGitLint - PASS

>

>

>     ##############################

>     Test: CheckBuildK - PASS

>

>

>     ##############################

>     Test: CheckTestRunner: Setup - PASS

>

>

>     ##############################

>     Test: CheckTestRunner: l2cap-tester - PASS

>     Total: 40, Passed: 34 (85.0%), Failed: 0, Not Run: 6

>

>     ##############################

>     Test: CheckTestRunner: bnep-tester - PASS

>     Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

>

>     ##############################

>     Test: CheckTestRunner: mgmt-tester - PASS

>     Total: 416, Passed: 402 (96.6%), Failed: 0, Not Run: 14

>

>     ##############################

>     Test: CheckTestRunner: rfcomm-tester - PASS

>     Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

>

>     ##############################

>     Test: CheckTestRunner: sco-tester - PASS

>     Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

>

>     ##############################

>     Test: CheckTestRunner: smp-tester - PASS

>     Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

>

>     ##############################

>     Test: CheckTestRunner: userchan-tester - PASS

>     Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0

>

>

>

> ---

> Regards,

> Linux Bluetooth

>
Alain Michaud Feb. 24, 2021, 6:57 p.m. UTC | #3
Friendly ping.


On Thu, Feb 18, 2021 at 3:06 PM Alain Michaud <alainmichaud@google.com> wrote:
>

> +Marcel for viz

>

>

> On Wed, Feb 17, 2021 at 10:11 AM <bluez.test.bot@gmail.com> wrote:

> >

> > This is automated email and please do not reply to this email!

> >

> > Dear submitter,

> >

> > Thank you for submitting the patches to the linux bluetooth mailing list.

> > This is a CI test results with your patch series:

> > PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=434523

> >

> > ---Test result---

> >

> > ##############################

> >     Test: CheckPatch - PASS

> >

> >

> >     ##############################

> >     Test: CheckGitLint - PASS

> >

> >

> >     ##############################

> >     Test: CheckBuildK - PASS

> >

> >

> >     ##############################

> >     Test: CheckTestRunner: Setup - PASS

> >

> >

> >     ##############################

> >     Test: CheckTestRunner: l2cap-tester - PASS

> >     Total: 40, Passed: 34 (85.0%), Failed: 0, Not Run: 6

> >

> >     ##############################

> >     Test: CheckTestRunner: bnep-tester - PASS

> >     Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

> >

> >     ##############################

> >     Test: CheckTestRunner: mgmt-tester - PASS

> >     Total: 416, Passed: 402 (96.6%), Failed: 0, Not Run: 14

> >

> >     ##############################

> >     Test: CheckTestRunner: rfcomm-tester - PASS

> >     Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

> >

> >     ##############################

> >     Test: CheckTestRunner: sco-tester - PASS

> >     Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

> >

> >     ##############################

> >     Test: CheckTestRunner: smp-tester - PASS

> >     Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

> >

> >     ##############################

> >     Test: CheckTestRunner: userchan-tester - PASS

> >     Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0

> >

> >

> >

> > ---

> > Regards,

> > Linux Bluetooth

> >
Marcel Holtmann Feb. 26, 2021, 8:19 p.m. UTC | #4
Hi Alain,

> This patch tolerates connection parameters that were valid before

> ESR08_v1.0.0 which was also incoporated into the 4.2 core specification.

> 

> In particular, this addresses compatibility issues with the Lenovo Yoga

> Mouse that sees its connection parameters rejected by the max_latency

> computation in hci_conn_check_params, but was perfectly valid at the

> time this mouse was qualified.

> 

> Before the patch, the mouse was extremely laggy, after the patch, the

> mouse worked as expected.

> 

> Signed-off-by: Alain Michaud <alainm@chromium.org>

> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>

> Tested-by: Harry Cutts <hcutts@chromium.org>

> 

> ---

> 

> include/net/bluetooth/hci_core.h | 30 ++++++++++++++++++++++++++++++

> net/bluetooth/l2cap_core.c       | 12 ++++++++++++

> 2 files changed, 42 insertions(+)

> 

> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h

> index ebdd4afe30d2..67b75077c82e 100644

> --- a/include/net/bluetooth/hci_core.h

> +++ b/include/net/bluetooth/hci_core.h

> @@ -1646,6 +1646,36 @@ static inline struct smp_irk *hci_get_irk(struct hci_dev *hdev,

> 	return hci_find_irk_by_rpa(hdev, bdaddr);

> }

> 

> +/* Erratum 5412 which has been fixed in 4.2 changed the validation of

> + * connection parameters.  For backwards compatibility reasons, the old

> + * calculation must be tolerated.

> + * For further details :

> + * https://www.bluetooth.org/errata/errata_view.cfm?errata_id=5419

> + */

> +static inline int hci_check_conn_params_legacy(u16 min, u16 max, u16 latency,

> +					u16 to_multiplier)


I would not bother with “inline” here. Let the compiler decide.

> +{

> +	u16 max_latency;

> +

> +	if (min > max || min < 6 || max > 3200)

> +		return -EINVAL;

> +

> +	if (to_multiplier < 10 || to_multiplier > 3200)

> +		return -EINVAL;

> +

> +	if (max >= to_multiplier * 8)

> +		return -EINVAL;

> +

> +	max_latency = (to_multiplier * 8 / max) - 1;

> +	if (latency > 499 || latency > max_latency)

> +		return -EINVAL;

> +

> +	return 0;

> +}

> +

> +/* Connection Parameter Validation Helper.

> + * See Vol 6, Part B, section 4.5.1.

> + */

> static inline int hci_check_conn_params(u16 min, u16 max, u16 latency,

> 					u16 to_multiplier)

> {

> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c

> index 72c2f5226d67..726e78bd85ff 100644

> --- a/net/bluetooth/l2cap_core.c

> +++ b/net/bluetooth/l2cap_core.c

> @@ -5553,6 +5553,18 @@ static inline int l2cap_conn_param_update_req(struct l2cap_conn *conn,

> 	memset(&rsp, 0, sizeof(rsp));

> 

> 	err = hci_check_conn_params(min, max, latency, to_multiplier);

> +	if (err) {

> +		BT_WARN("Invalid conn params min 0x%4.4x max 0x%4.4x latency: 0x%4.4x TO: 0x%4.4x",

> +			min, max, latency, to_multiplier);


Can we use bt_dev_warn() here?

> +

> +		err = hci_check_conn_params_legacy(min, max, latency,

> +						   to_multiplier);

> +		if (!err) {

> +			/* latency is invalid, cap it to the max allowed */

> +			latency = min(499, (to_multiplier * 4 / max) - 1);

> +		}

> +	}

> +

> 	if (err)

> 		rsp.result = cpu_to_le16(L2CAP_CONN_PARAM_REJECTED);

> 	else


Regards

Marcel
diff mbox series

Patch

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ebdd4afe30d2..67b75077c82e 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1646,6 +1646,36 @@  static inline struct smp_irk *hci_get_irk(struct hci_dev *hdev,
 	return hci_find_irk_by_rpa(hdev, bdaddr);
 }
 
+/* Erratum 5412 which has been fixed in 4.2 changed the validation of
+ * connection parameters.  For backwards compatibility reasons, the old
+ * calculation must be tolerated.
+ * For further details :
+ * https://www.bluetooth.org/errata/errata_view.cfm?errata_id=5419
+ */
+static inline int hci_check_conn_params_legacy(u16 min, u16 max, u16 latency,
+					u16 to_multiplier)
+{
+	u16 max_latency;
+
+	if (min > max || min < 6 || max > 3200)
+		return -EINVAL;
+
+	if (to_multiplier < 10 || to_multiplier > 3200)
+		return -EINVAL;
+
+	if (max >= to_multiplier * 8)
+		return -EINVAL;
+
+	max_latency = (to_multiplier * 8 / max) - 1;
+	if (latency > 499 || latency > max_latency)
+		return -EINVAL;
+
+	return 0;
+}
+
+/* Connection Parameter Validation Helper.
+ * See Vol 6, Part B, section 4.5.1.
+ */
 static inline int hci_check_conn_params(u16 min, u16 max, u16 latency,
 					u16 to_multiplier)
 {
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 72c2f5226d67..726e78bd85ff 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5553,6 +5553,18 @@  static inline int l2cap_conn_param_update_req(struct l2cap_conn *conn,
 	memset(&rsp, 0, sizeof(rsp));
 
 	err = hci_check_conn_params(min, max, latency, to_multiplier);
+	if (err) {
+		BT_WARN("Invalid conn params min 0x%4.4x max 0x%4.4x latency: 0x%4.4x TO: 0x%4.4x",
+			min, max, latency, to_multiplier);
+
+		err = hci_check_conn_params_legacy(min, max, latency,
+						   to_multiplier);
+		if (!err) {
+			/* latency is invalid, cap it to the max allowed */
+			latency = min(499, (to_multiplier * 4 / max) - 1);
+		}
+	}
+
 	if (err)
 		rsp.result = cpu_to_le16(L2CAP_CONN_PARAM_REJECTED);
 	else