Re: [PATCH] Bluetooth: HCI: fix divide error in __get_blocks()

[Luiz Augusto von Dentz]

Hi,

On Sun, May 5, 2024 at 10:21 PM Sungwoo Kim <iam@xxxxxxxxxxxx> wrote:
>
> 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-rc6-00001-g38e1170f515d-dirty #32
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/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@xxxxxxxxxxxx>
> ---
>  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 = hci_chan_sent(hdev, type, &quote))) {
>                 u32 priority = (skb_peek(&chan->data_q))->priority;
>                 while (quote > 0 && (skb = 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 = hdev->pkt_type & ACL_PTYPE_MASK;
-               conn->mtu = hdev->acl_mtu;
+               switch (hdev->flow_ctl_mode) {
+               case HCI_FLOW_CTL_MODE_PACKET_BASED:
+                       conn->mtu = hdev->acl_mtu;
+                       break;
+               case HCI_FLOW_CTL_MODE_BLOCK_BASED:
+                       conn->mtu = hdev->block_mtu;
+                       break;
+               }
                break;
        case LE_LINK:
                /* conn->src should reflect the local identity address */

-- 
Luiz Augusto von Dentz