Subject: Re: [syzbot] BUG: sleeping function called from invalid context in
 hci_cmd_sync_cancel
To:     syzbot <syzbot+485cc00ea7cf41dfdbf1@syzkaller.appspotmail.com>,
        Thinh.Nguyen@synopsys.com, bberg@redhat.com, changbin.du@intel.com,
        christian.brauner@ubuntu.com, davem@davemloft.net,
        edumazet@google.com, gregkh@linuxfoundation.org,
        johan.hedberg@gmail.com, kuba@kernel.org,
        linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-usb@vger.kernel.org, luiz.dentz@gmail.com,
        luiz.von.dentz@intel.com, marcel@holtmann.org,
        mathias.nyman@linux.intel.com, netdev@vger.kernel.org,
        stern@rowland.harvard.edu, syzkaller-bugs@googlegroups.com,
        yajun.deng@linux.dev
References: <00000000000098464c05d2acf3ba@google.com>
From:   Oliver Neukum <oneukum@suse.com>
Message-ID: <3e8cba55-5d34-eab3-0625-687b66bb9449@suse.com>
Date:   Thu, 9 Dec 2021 11:06:50 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.12.0
In-Reply-To: <00000000000098464c05d2acf3ba@google.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Content-Language: en-US
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?lMaabn51fazXATP3ZMzkMXLd9s8AaET2wm3SUn49Cqf2PFPeCzo5O2gXpIR5?=
 =?us-ascii?Q?DuLCa/r6SU++Tde592FDrVBhXVcin+Cda+7wZn+eANdpa6Ser+sPK6hxY7Vm?=
 =?us-ascii?Q?YSM5K7vMoPDIzp47FRYmsQ6dsO3lg9/P0QV0samMYVX68BpsoqavZGoREOfG?=
 =?us-ascii?Q?kzLKM97Dj9kVt1rJrDmluJKP3ubXNd7CwxeErNhvWf9m2mKrb7C+B+BL4b9J?=
 =?us-ascii?Q?1Gif4NZJFNy69OCpJXjLxvbGtxPceTbgcDV8QJdbP2TANRFpby+/37Mqvpaa?=
 =?us-ascii?Q?O0NBCZ5CA64U3Ceh2TcAoFxZ0JKPu5kK+4lLQAhJjq6U8UdxbaGQtbAFYA9q?=
 =?us-ascii?Q?7xd9/CXPxBLF9IQbMNusf8GI2iy+AToOemLAnDWtB9crCtn/YbUW0mbfpQKJ?=
 =?us-ascii?Q?5GNGMwN6ngyaLOjLTFR/evdwVD3DQKemyaIqW9fLewjmE/wPQobN62O8TO9A?=
 =?us-ascii?Q?0smzSkEF9pjJdKgZ6ZN/AxIIkecStpsC3ybY37c/OnEierFq9ViyWRvH0IaB?=
 =?us-ascii?Q?cLI2rFtKsU27zOIXnQ+yokepRquQ9m54ZSYENXqdTyartgoFQ/WsRZk6zjzB?=
 =?us-ascii?Q?F7COS6J0iUdRYNZNMa0DiSlQ5O6Wn9VWEEa/uZaAtF3o7L4cIoRfP6769tLw?=
 =?us-ascii?Q?jJcvLCDlDb1Ts5uNqOHFnQw/mZuEf50m42PtQJtqha6JFa5UbqFcbA9hdH+l?=
 =?us-ascii?Q?LiKvmrKhX3k3rSwDFANnASzqSZco/F6tbQwEpEEuF7LOUOIt2rZchJ6cMNsS?=
 =?us-ascii?Q?ynbEO91Ysm0ibnC2IYm0qAebOs2TLWQqlNBsbGOCUd4OojaEhlqeH6Ud1HNs?=
 =?us-ascii?Q?KsNW3xxFXVH/ahI7iu8+sG1Q50ALxragA4OeEnZ+i3gN1MsXzXSwMzi/ACaf?=
 =?us-ascii?Q?p+b20a2RvsfFwkdBQFCGPgCTQL32EDo3YxJohtgI13+MxvQEFgHBYmRLd9/o?=
 =?us-ascii?Q?PyEpSqzpCwiOQ9cyHifPUYz4ZmcyLtZdrlTyBYpSJcJv9wf+hi/vk/mLnayu?=
 =?us-ascii?Q?+/zVrxpTLPi8LXyrf+MRQ2RWBmEITOYFxRX05DPxIt/YfrCApb/VibuwNXUD?=
 =?us-ascii?Q?2rJ5gTpYjThrVRWzxXpYrlj1JLJ7OF4OR8J9/sa5JMY3wERRzlnInO5Gx/Ly?=
 =?us-ascii?Q?Zkyt6doKatXB2ZfEOVXnbl4NRt3qB3Mh84W1X+x7UQ7WmViCydLwkviJ6R1f?=
 =?us-ascii?Q?vXfj/TdrgjRQd9eN6E8mj8859k588Kza0MYCOAaCGtp1HfnxWDqoaaRb1yb1?=
 =?us-ascii?Q?7p587b5MA+QepFvYAs/vJKS52HDYsaRQWf3SHpgMS1SIhj1qqHsy74chUn2C?=
 =?us-ascii?Q?EchP2lVYKxCXXV1DErsro42gdbXNkD/WPgChinZMY3NEns0ecp+fnJmYBCvB?=
 =?us-ascii?Q?OYrEE9t14qWsFyOVjMVnottUDMYCKToi3cS0WL1Gh8x+ofhdzwPw9lXHOPy5?=
 =?us-ascii?Q?cH9+GRF6NDVK7UPWRa+HLUDNv7NLLe7MlHf0DedaZNV8bRyGQkqhEgrpH1CT?=
 =?us-ascii?Q?4kyB7wFR0a1qvHSBe4OMpgpnW2QBc4AmHExKmzMT3yvLCKCl/VSTaDUXtgND?=
 =?us-ascii?Q?MoOU65dvFkLZinFBo8SGvsDsYoRrrEiUKjO7gurjWY9RdySRmkxUAJWoTbzJ?=
 =?us-ascii?Q?FkRh2ec3TrXRlxJk8kl0OvQ98TEWsnGKbTpNVm/iI8FRC7a1nhDlX62qswT0?=
 =?us-ascii?Q?2hrQ4Q6AbTybiu2yRdNtKVBsI3s=3D?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a79e8efd-4dda-40eb-5a8c-08d9bafba02f
X-MS-Exchange-CrossTenant-AuthSource: DB7PR04MB5050.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Dec 2021 10:06:54.4256
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: l3g/GFncj65zO8d1IbpHILEwVU2blMcP9f4H+AQ1PdCcQGMcDySCkFAzimWmZ0dQltEpF1Bc6BDiirdgxzuF0w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR04MB4140
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 09.12.21 02:59, syzbot wrote:
> syzbot has bisected this issue to:
>
> commit c97a747efc93f94a4ad6c707972dfbf8d774edf9
> Author: Benjamin Berg <bberg@redhat.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)
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0 if (err !=3D -EPERM && err !=3D -ENODEV)
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 bt_dev_err(=
hdev, "urb %p failed to resubmit (%d)",
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 urb, -err);
+=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0 if (err !=3D -EPERM)
+=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 hci_cmd_sync_c=
ancel(hdev, -err);


@@ -1331,10 +1335,13 @@ static void btusb_tx_complete(struct urb *urb)
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (!test_bit(HCI_RUNNING, &hdev=
->flags))
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0 goto done;
=C2=A0
-=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (!urb->status)
+=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (!urb->status) {
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0 hdev->stat.byte_tx +=3D urb->transfer_buffer_length;
-=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 else
+=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } else {
+=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0 if (hci_skb_pkt_type(skb) =3D=3D HCI_COMMAND_PKT)
+=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 hci_cmd_sync_c=
ancel(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?

=C2=A0=C2=A0=C2=A0 Regards
=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 Oliver