public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] Bluetooth: RFCOMM: FIX possible deadlock in rfcomm_sk_state_change
@ 2024-09-30 19:30 Luiz Augusto von Dentz
  2024-09-30 20:36 ` Luiz Augusto von Dentz
  2024-10-01 16:10 ` [PATCH v1] Bluetooth: RFCOMM: FIX " patchwork-bot+bluetooth
  0 siblings, 2 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2024-09-30 19:30 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

rfcomm_sk_state_change attempts to use sock_lock so it must never be
called with it locked but rfcomm_sock_ioctl always attempt to lock it
causing the following trace:

======================================================
WARNING: possible circular locking dependency detected
6.8.0-syzkaller-08951-gfe46a7dd189e #0 Not tainted
------------------------------------------------------
syz-executor386/5093 is trying to acquire lock:
ffff88807c396258 (sk_lock-AF_BLUETOOTH-BTPROTO_RFCOMM){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1671 [inline]
ffff88807c396258 (sk_lock-AF_BLUETOOTH-BTPROTO_RFCOMM){+.+.}-{0:0}, at: rfcomm_sk_state_change+0x5b/0x310 net/bluetooth/rfcomm/sock.c:73

but task is already holding lock:
ffff88807badfd28 (&d->lock){+.+.}-{3:3}, at: __rfcomm_dlc_close+0x226/0x6a0 net/bluetooth/rfcomm/core.c:491

Reported-by: syzbot+d7ce59b06b3eb14fd218@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=d7ce59b06b3eb14fd218
Fixes: 3241ad820dbb ("[Bluetooth] Add timestamp support to L2CAP, RFCOMM and SCO")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/rfcomm/sock.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 37d63d768afb..f48250e3f2e1 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -865,9 +865,7 @@ static int rfcomm_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned lon
 
 	if (err == -ENOIOCTLCMD) {
 #ifdef CONFIG_BT_RFCOMM_TTY
-		lock_sock(sk);
 		err = rfcomm_dev_ioctl(sk, cmd, (void __user *) arg);
-		release_sock(sk);
 #else
 		err = -EOPNOTSUPP;
 #endif
-- 
2.46.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] Bluetooth: RFCOMM: FIX possible deadlock in rfcomm_sk_state_change
  2024-09-30 19:30 [PATCH v1] Bluetooth: RFCOMM: FIX possible deadlock in rfcomm_sk_state_change Luiz Augusto von Dentz
@ 2024-09-30 20:36 ` Luiz Augusto von Dentz
  2024-10-01 14:58   ` Luiz Augusto von Dentz
  2024-10-01 16:10 ` [PATCH v1] Bluetooth: RFCOMM: FIX " patchwork-bot+bluetooth
  1 sibling, 1 reply; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2024-09-30 20:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: syzbot+d7ce59b06b3eb14fd218

On Mon, Sep 30, 2024 at 3:30 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> rfcomm_sk_state_change attempts to use sock_lock so it must never be
> called with it locked but rfcomm_sock_ioctl always attempt to lock it
> causing the following trace:
>
> ======================================================
> WARNING: possible circular locking dependency detected
> 6.8.0-syzkaller-08951-gfe46a7dd189e #0 Not tainted
> ------------------------------------------------------
> syz-executor386/5093 is trying to acquire lock:
> ffff88807c396258 (sk_lock-AF_BLUETOOTH-BTPROTO_RFCOMM){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1671 [inline]
> ffff88807c396258 (sk_lock-AF_BLUETOOTH-BTPROTO_RFCOMM){+.+.}-{0:0}, at: rfcomm_sk_state_change+0x5b/0x310 net/bluetooth/rfcomm/sock.c:73
>
> but task is already holding lock:
> ffff88807badfd28 (&d->lock){+.+.}-{3:3}, at: __rfcomm_dlc_close+0x226/0x6a0 net/bluetooth/rfcomm/core.c:491
>
> Reported-by: syzbot+d7ce59b06b3eb14fd218@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=d7ce59b06b3eb14fd218
> Fixes: 3241ad820dbb ("[Bluetooth] Add timestamp support to L2CAP, RFCOMM and SCO")
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
>  net/bluetooth/rfcomm/sock.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
> index 37d63d768afb..f48250e3f2e1 100644
> --- a/net/bluetooth/rfcomm/sock.c
> +++ b/net/bluetooth/rfcomm/sock.c
> @@ -865,9 +865,7 @@ static int rfcomm_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned lon
>
>         if (err == -ENOIOCTLCMD) {
>  #ifdef CONFIG_BT_RFCOMM_TTY
> -               lock_sock(sk);
>                 err = rfcomm_dev_ioctl(sk, cmd, (void __user *) arg);
> -               release_sock(sk);
>  #else
>                 err = -EOPNOTSUPP;
>  #endif
> --
> 2.46.1
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] Bluetooth: RFCOMM: FIX possible deadlock in rfcomm_sk_state_change
  2024-09-30 20:36 ` Luiz Augusto von Dentz
@ 2024-10-01 14:58   ` Luiz Augusto von Dentz
  2024-10-01 15:52     ` [syzbot] [bluetooth?] " syzbot
  0 siblings, 1 reply; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-01 14:58 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: syzbot+d7ce59b06b3eb14fd218

[-- Attachment #1: Type: text/plain, Size: 2263 bytes --]

#syz test

On Mon, Sep 30, 2024 at 4:36 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> On Mon, Sep 30, 2024 at 3:30 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >
> > rfcomm_sk_state_change attempts to use sock_lock so it must never be
> > called with it locked but rfcomm_sock_ioctl always attempt to lock it
> > causing the following trace:
> >
> > ======================================================
> > WARNING: possible circular locking dependency detected
> > 6.8.0-syzkaller-08951-gfe46a7dd189e #0 Not tainted
> > ------------------------------------------------------
> > syz-executor386/5093 is trying to acquire lock:
> > ffff88807c396258 (sk_lock-AF_BLUETOOTH-BTPROTO_RFCOMM){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1671 [inline]
> > ffff88807c396258 (sk_lock-AF_BLUETOOTH-BTPROTO_RFCOMM){+.+.}-{0:0}, at: rfcomm_sk_state_change+0x5b/0x310 net/bluetooth/rfcomm/sock.c:73
> >
> > but task is already holding lock:
> > ffff88807badfd28 (&d->lock){+.+.}-{3:3}, at: __rfcomm_dlc_close+0x226/0x6a0 net/bluetooth/rfcomm/core.c:491
> >
> > Reported-by: syzbot+d7ce59b06b3eb14fd218@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=d7ce59b06b3eb14fd218
> > Fixes: 3241ad820dbb ("[Bluetooth] Add timestamp support to L2CAP, RFCOMM and SCO")
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > ---
> >  net/bluetooth/rfcomm/sock.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
> > index 37d63d768afb..f48250e3f2e1 100644
> > --- a/net/bluetooth/rfcomm/sock.c
> > +++ b/net/bluetooth/rfcomm/sock.c
> > @@ -865,9 +865,7 @@ static int rfcomm_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned lon
> >
> >         if (err == -ENOIOCTLCMD) {
> >  #ifdef CONFIG_BT_RFCOMM_TTY
> > -               lock_sock(sk);
> >                 err = rfcomm_dev_ioctl(sk, cmd, (void __user *) arg);
> > -               release_sock(sk);
> >  #else
> >                 err = -EOPNOTSUPP;
> >  #endif
> > --
> > 2.46.1
> >
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

[-- Attachment #2: v1-0001-Bluetooth-RFCOMM-FIX-possible-deadlock-in-rfcomm_.patch --]
[-- Type: text/x-patch, Size: 1925 bytes --]

From 9d651555ea532485cc38e759134e0c231c890eab Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Mon, 30 Sep 2024 13:26:21 -0400
Subject: [PATCH v1] Bluetooth: RFCOMM: FIX possible deadlock in
 rfcomm_sk_state_change

rfcomm_sk_state_change attempts to use sock_lock so it must never be
called with it locked but rfcomm_sock_ioctl always attempt to lock it
causing the following trace:

======================================================
WARNING: possible circular locking dependency detected
6.8.0-syzkaller-08951-gfe46a7dd189e #0 Not tainted
------------------------------------------------------
syz-executor386/5093 is trying to acquire lock:
ffff88807c396258 (sk_lock-AF_BLUETOOTH-BTPROTO_RFCOMM){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1671 [inline]
ffff88807c396258 (sk_lock-AF_BLUETOOTH-BTPROTO_RFCOMM){+.+.}-{0:0}, at: rfcomm_sk_state_change+0x5b/0x310 net/bluetooth/rfcomm/sock.c:73

but task is already holding lock:
ffff88807badfd28 (&d->lock){+.+.}-{3:3}, at: __rfcomm_dlc_close+0x226/0x6a0 net/bluetooth/rfcomm/core.c:491

Reported-by: syzbot+d7ce59b06b3eb14fd218@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=d7ce59b06b3eb14fd218
Fixes: 3241ad820dbb ("[Bluetooth] Add timestamp support to L2CAP, RFCOMM and SCO")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/rfcomm/sock.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 37d63d768afb..f48250e3f2e1 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -865,9 +865,7 @@ static int rfcomm_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned lon
 
 	if (err == -ENOIOCTLCMD) {
 #ifdef CONFIG_BT_RFCOMM_TTY
-		lock_sock(sk);
 		err = rfcomm_dev_ioctl(sk, cmd, (void __user *) arg);
-		release_sock(sk);
 #else
 		err = -EOPNOTSUPP;
 #endif
-- 
2.46.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [syzbot] [bluetooth?] possible deadlock in rfcomm_sk_state_change
  2024-10-01 14:58   ` Luiz Augusto von Dentz
@ 2024-10-01 15:52     ` syzbot
  0 siblings, 0 replies; 5+ messages in thread
From: syzbot @ 2024-10-01 15:52 UTC (permalink / raw)
  To: linux-bluetooth, linux-kernel, luiz.dentz, syzkaller-bugs

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+d7ce59b06b3eb14fd218@syzkaller.appspotmail.com
Tested-by: syzbot+d7ce59b06b3eb14fd218@syzkaller.appspotmail.com

Tested on:

commit:         e32cde8d Merge tag 'sched_ext-for-6.12-rc1-fixes-1' of..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12a33dd0580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=c9c87051d13eb9da
dashboard link: https://syzkaller.appspot.com/bug?extid=d7ce59b06b3eb14fd218
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch:          https://syzkaller.appspot.com/x/patch.diff?x=139c139f980000

Note: testing is done by a robot and is best-effort only.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] Bluetooth: RFCOMM: FIX possible deadlock in rfcomm_sk_state_change
  2024-09-30 19:30 [PATCH v1] Bluetooth: RFCOMM: FIX possible deadlock in rfcomm_sk_state_change Luiz Augusto von Dentz
  2024-09-30 20:36 ` Luiz Augusto von Dentz
@ 2024-10-01 16:10 ` patchwork-bot+bluetooth
  1 sibling, 0 replies; 5+ messages in thread
From: patchwork-bot+bluetooth @ 2024-10-01 16:10 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Mon, 30 Sep 2024 15:30:44 -0400 you wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> rfcomm_sk_state_change attempts to use sock_lock so it must never be
> called with it locked but rfcomm_sock_ioctl always attempt to lock it
> causing the following trace:
> 
> ======================================================
> WARNING: possible circular locking dependency detected
> 6.8.0-syzkaller-08951-gfe46a7dd189e #0 Not tainted
> 
> [...]

Here is the summary with links:
  - [v1] Bluetooth: RFCOMM: FIX possible deadlock in rfcomm_sk_state_change
    https://git.kernel.org/bluetooth/bluetooth-next/c/ffb3f98d4dae

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-10-01 16:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-30 19:30 [PATCH v1] Bluetooth: RFCOMM: FIX possible deadlock in rfcomm_sk_state_change Luiz Augusto von Dentz
2024-09-30 20:36 ` Luiz Augusto von Dentz
2024-10-01 14:58   ` Luiz Augusto von Dentz
2024-10-01 15:52     ` [syzbot] [bluetooth?] " syzbot
2024-10-01 16:10 ` [PATCH v1] Bluetooth: RFCOMM: FIX " patchwork-bot+bluetooth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox