| Message ID | 20240930193044.2907716-1-luiz.dentz@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ffb3f98d4dae82b95d2c80e4bdd990798263ff14 |
| Headers | show |
| Series | [v1] Bluetooth: RFCOMM: FIX possible deadlock in rfcomm_sk_state_change | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | warning | WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #105: ffff88807badfd28 (&d->lock){+.+.}-{3:3}, at: __rfcomm_dlc_close+0x226/0x6a0 net/bluetooth/rfcomm/core.c:491 total: 0 errors, 1 warnings, 0 checks, 9 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/src/13816897.patch has style problems, please review. NOTE: Ignored message types: UNKNOWN_COMMIT_ID NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. |
| tedd_an/GitLint | fail | WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 14: B1 Line exceeds max length (114>80): "ffff88807c396258 (sk_lock-AF_BLUETOOTH-BTPROTO_RFCOMM){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1671 [inline]" 15: B1 Line exceeds max length (136>80): "ffff88807c396258 (sk_lock-AF_BLUETOOTH-BTPROTO_RFCOMM){+.+.}-{0:0}, at: rfcomm_sk_state_change+0x5b/0x310 net/bluetooth/rfcomm/sock.c:73" 18: B1 Line exceeds max length (107>80): "ffff88807badfd28 (&d->lock){+.+.}-{3:3}, at: __rfcomm_dlc_close+0x226/0x6a0 net/bluetooth/rfcomm/core.c:491" |
| tedd_an/SubjectPrefix | success | Gitlint PASS |
| tedd_an/BuildKernel | success | BuildKernel PASS |
| tedd_an/CheckAllWarning | success | CheckAllWarning PASS |
| tedd_an/CheckSparse | success | CheckSparse PASS |
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 >
#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
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.
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!
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