Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-4332-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: <20240506022035.663102-1-iam@sung-woo.kim>
In-Reply-To: <20240506022035.663102-1-iam@sung-woo.kim>
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: Mon, 6 May 2024 13:25:19 -0400
Message-ID: <CABBYNZJPA_a0LmAArNcV4pyskhyeXzSRpe98Ksf=BSZ56ZA0zA@mail.gmail.com>
Subject: Re: [PATCH] Bluetooth: HCI: fix divide error in __get_blocks()
To: Sungwoo Kim <iam@sung-woo.kim>
Cc: daveti@purdue.edu, benquike@gmail.com, 
	Marcel Holtmann <marcel@holtmann.org>, Johan Hedberg <johan.hedberg@gmail.com>, 
	linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Sun, May 5, 2024 at 10:21=E2=80=AFPM Sungwoo Kim <iam@sung-woo.kim> wrot=
e:
>
> hdev->block_len could be 0. Fix this by adding a check.
>
> divide error: 0000 [#1] PREEMPT SMP KASAN NOPTI
> CPU: 0 PID: 9622 Comm: kworker/u5:4 Tainted: G        W          6.9.0-rc=
6-00001-g38e1170f515d-dirty #32
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/0=
1/2014
> Workqueue: hci11 hci_tx_work
> RIP: 0010:__get_blocks net/bluetooth/hci_core.c:3618 [inline]
> RIP: 0010:hci_sched_acl_blk net/bluetooth/hci_core.c:3766 [inline]
> RIP: 0010:hci_sched_acl net/bluetooth/hci_core.c:3806 [inline]
> RIP: 0010:hci_tx_work+0x73e/0x1d10 net/bluetooth/hci_core.c:3901
>
> Fixes: b71d385a18cd ("Bluetooth: Recalculate sched HCI blk/pkt flow ctrl"=
)
> Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>
> ---
>  net/bluetooth/hci_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 0efd59760..20b1cd7f3 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3762,7 +3762,7 @@ static void hci_sched_acl_blk(struct hci_dev *hdev)
>
>         __check_timeout(hdev, cnt, type);
>
> -       while (hdev->block_cnt > 0 &&
> +       while (hdev->block_len > 0 && hdev->block_cnt > 0 &&
>                (chan =3D hci_chan_sent(hdev, type, &quote))) {
>                 u32 priority =3D (skb_peek(&chan->data_q))->priority;
>                 while (quote > 0 && (skb =3D skb_peek(&chan->data_q))) {
> --
> 2.34.1

Hmm, this code shall probably be removed as well since
HCI_FLOW_CTL_MODE_BLOCK_BASED was sort of tight to AMP support which
we have removed support for, anyway this is failing late actually
since we might have to check this during hci_conn_add with:

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 171a667bc991..73b9d08438fe 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -907,8 +907,16 @@ struct hci_conn *hci_conn_add(struct hci_dev
*hdev, int type, bdaddr_t *dst,

        switch (type) {
        case ACL_LINK:
-               if (!hdev->acl_mtu)
-                       return ERR_PTR(-ECONNREFUSED);
+               switch (hdev->flow_ctl_mode) {
+               case HCI_FLOW_CTL_MODE_PACKET_BASED:
+                       if (!hdev->acl_mtu)
+                               return ERR_PTR(-ECONNREFUSED);
+                       break;
+               case HCI_FLOW_CTL_MODE_BLOCK_BASED:
+                       if (!hdev->block_mtu)
+                               return ERR_PTR(-ECONNREFUSED);
+                       break;
+               }
                break;
        case ISO_LINK:
                if (hdev->iso_mtu)
@@ -966,7 +974,14 @@ struct hci_conn *hci_conn_add(struct hci_dev
*hdev, int type, bdaddr_t *dst,
        switch (type) {
        case ACL_LINK:
                conn->pkt_type =3D hdev->pkt_type & ACL_PTYPE_MASK;
-               conn->mtu =3D hdev->acl_mtu;
+               switch (hdev->flow_ctl_mode) {
+               case HCI_FLOW_CTL_MODE_PACKET_BASED:
+                       conn->mtu =3D hdev->acl_mtu;
+                       break;
+               case HCI_FLOW_CTL_MODE_BLOCK_BASED:
+                       conn->mtu =3D hdev->block_mtu;
+                       break;
+               }
                break;
        case LE_LINK:
                /* conn->src should reflect the local identity address */

--=20
Luiz Augusto von Dentz