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

From: Luiz Augusto von Dentz
Date: Mon May 06 2024 - 13:25:45 EST


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