linux-kernel - Re: [syzbot] BUG: sleeping function called from invalid context in hci_cmd_sync

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3e8cba55-5d34-eab3-0625-687b66bb9449@suse.com>
Date:   Thu, 9 Dec 2021 11:06:50 +0100
From:   Oliver Neukum <oneukum@...e.com>
To:     syzbot <syzbot+485cc00ea7cf41dfdbf1@...kaller.appspotmail.com>,
        Thinh.Nguyen@...opsys.com, bberg@...hat.com, changbin.du@...el.com,
        christian.brauner@...ntu.com, davem@...emloft.net,
        edumazet@...gle.com, gregkh@...uxfoundation.org,
        johan.hedberg@...il.com, kuba@...nel.org,
        linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org, luiz.dentz@...il.com,
        luiz.von.dentz@...el.com, marcel@...tmann.org,
        mathias.nyman@...ux.intel.com, netdev@...r.kernel.org,
        stern@...land.harvard.edu, syzkaller-bugs@...glegroups.com,
        yajun.deng@...ux.dev
Subject: Re: [syzbot] BUG: sleeping function called from invalid context in
 hci_cmd_sync_cancel


On 09.12.21 02:59, syzbot wrote:
> syzbot has bisected this issue to:
>
> commit c97a747efc93f94a4ad6c707972dfbf8d774edf9
> Author: Benjamin Berg <bberg@...hat.com>
> Date:   Fri Dec 3 14:59:02 2021 +0000
>
>     Bluetooth: btusb: Cancel sync commands for certain URB errors

Hi,

looking at the patch, it sleeps in an interrupt handler (or equivalent)
in two places:

@@ -933,6 +933,8 @@ static void btusb_intr_complete(struct urb *urb)
                if (err != -EPERM && err != -ENODEV)
                        bt_dev_err(hdev, "urb %p failed to resubmit (%d)",
                                   urb, -err);
+               if (err != -EPERM)
+                       hci_cmd_sync_cancel(hdev, -err);


@@ -1331,10 +1335,13 @@ static void btusb_tx_complete(struct urb *urb)
        if (!test_bit(HCI_RUNNING, &hdev->flags))
                goto done;
 
-       if (!urb->status)
+       if (!urb->status) {
                hdev->stat.byte_tx += urb->transfer_buffer_length;
-       else
+       } else {
+               if (hci_skb_pkt_type(skb) == HCI_COMMAND_PKT)
+                       hci_cmd_sync_cancel(hdev, -urb->status);

As __cancel_work_timer can be called from hci_cmd_sync_cancel() this is
just not
an approach you can take. It looks like asynchronously canceling the
scheduled work
would result in a race, so I would for now just revert.

What issue exactly is this trying to fix or improve?

    Regards
        Oliver