Re: WARNING: possible circular locking dependency detected (hci_sock_dev_event+0x17d/0x1f0)

From: "Anand K. Mistry" <amistry@google.com>
To: Ammar Faizi <ammarfaizi2@gnuweeb.org>
Cc: Hillf Danton <hdanton@sina.com>, Lin Ma <linma@zju.edu.cn>,
	Marcel Holtmann <marcel@holtmann.org>,
	Ammar Faizi <ammarfaizi2@gmail.com>,
	linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: WARNING: possible circular locking dependency detected (hci_sock_dev_event+0x17d/0x1f0)
Date: Fri, 6 Aug 2021 17:18:25 +1000	[thread overview]
Message-ID: <CAATStaMjeyzU3Gj7D-Q1zGMdioC5ENoXbf4QB9Rmw8TCWRiagg@mail.gmail.com> (raw)
In-Reply-To: <80e24ed7-a34a-7fac-c709-06ccad6d2612@gnuweeb.org>

On Wed, 4 Aug 2021 at 21:43, Ammar Faizi <ammarfaizi2@gnuweeb.org> wrote:
>
> See the original report here:
> https://lore.kernel.org/lkml/2c40741c-8c8f-a105-1846-aa1ed15a6c7e@gnuweeb.org/
>
> On 8/4/21 3:56 PM, Hillf Danton wrote:
> > On Mon, 2 Aug 2021 20:17:02 +0700 Ammar Faizi wrote:
> >> Hi everyone,
> >>
> >> I found a potential deadlock at hci_sock_dev_event.
> >
> > Thanks for your report.
> >
> > Can you try and see if reverting e305509e678b3a4af2b3cfd410f409f7cdaabb52
> > ("Bluetooth: use correct lock to prevent UAF of hdev object") is a cure.
> >
>
> Hi Hillf,
>
> Thank you for your response.
>
> Here is what I have tried:
> 1. Reverted e305509e678b3a4af2b3cfd410f409f7cdaabb52.
> 2. Compile and install reverted version.
> 3. Rebooted 5.14.0-rc3 (c7d102232649, before revert) and try to
> reproduce again. At this point I am able to reproduce it.
> 4. Booted 5.14.0-rc3 (the reverted version). The deadlock warning is
> gone after reverting it. No more deadlock warning.

Can you try patching in
https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=58ce6d5b271ab25fb2056f84a8e5546945eb5fc9
and see if that fixes it.

>
> ------------------------------------------------------------
> [ 51207ee38ab65db86554655300a912e8c661525e is my local revert. ]
> Revert diff:
> ammarfaizi2@integral:~/project/now/linux$ git diff
> c7d102232649226a69dddd58a4942cf13cff4f7c
> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> index b04a5a02ecf3..590467345c5f 100644
> --- a/net/bluetooth/hci_sock.c
> +++ b/net/bluetooth/hci_sock.c
> @@ -762,7 +762,7 @@ void hci_sock_dev_event(struct hci_dev *hdev, int event)
>                 /* Detach sockets from device */
>                 read_lock(&hci_sk_list.lock);
>                 sk_for_each(sk, &hci_sk_list.head) {
> -                       lock_sock(sk);
> +                       bh_lock_sock_nested(sk);
>                         if (hci_pi(sk)->hdev == hdev) {
>                                 hci_pi(sk)->hdev = NULL;
>                                 sk->sk_err = EPIPE;
> @@ -771,7 +771,7 @@ void hci_sock_dev_event(struct hci_dev *hdev, int event)
>
>                                 hci_dev_put(hdev);
>                         }
> -                       release_sock(sk);
> +                       bh_unlock_sock(sk);
>                 }
>                 read_unlock(&hci_sk_list.lock);
>         }
> ammarfaizi2@integral:~/project/now/linux$
> ------------------------------------------------------------
>
> Summary:
> Reverting e305509e678b3a4af2b3cfd410f409f7cdaabb52 is a cure.
>
> Although it fixes the deadlock, there is probably something still goes
> wrong regarding "prevent UAF" as the commit message says.
>
> Will anyone create a patch for this?
> I want to be the tester, I am new at kernel development and want to
> start contributing, so please guide me :)
>
> I also attached dmesg log before and after revert if you want to see the
> full log.
>
> Adding Lin as the author of e305509e678b3a4af2b3cfd410f409f7cdaabb52 to
> CC list...
>
>
> --
> Ammar
>

-- 
Anand K. Mistry
Software Engineer
Google Australia