* [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2)
@ 2025-02-06 13:10 syzbot
2025-02-06 22:05 ` [syzbot] Re: [PATCH v1] Bluetooth: L2CAP: Fix corrupted list in hci_chan_del syzbot
` (7 more replies)
0 siblings, 8 replies; 10+ messages in thread
From: syzbot @ 2025-02-06 13:10 UTC (permalink / raw)
To: eadavis, johan.hedberg, linux-bluetooth, linux-kernel,
luiz.dentz, luiz.von.dentz, marcel, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: 40b8e93e17bf Add linux-next specific files for 20250204
git tree: linux-next
console+strace: https://syzkaller.appspot.com/x/log.txt?x=16241eb0580000
kernel config: https://syzkaller.appspot.com/x/.config?x=ec880188a87c6aad
dashboard link: https://syzkaller.appspot.com/bug?extid=10bd8fe6741eedd2be2e
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1078cf64580000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=138f5d18580000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/ccdfef06f59f/disk-40b8e93e.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/b339eaf8dcfd/vmlinux-40b8e93e.xz
kernel image: https://storage.googleapis.com/syzbot-assets/ae1a0f1c3c80/bzImage-40b8e93e.xz
The issue was bisected to:
commit 6ab54a7171894394fa07f28f835d714967b39797
Author: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Thu Jan 16 15:35:03 2025 +0000
Bluetooth: L2CAP: Fix slab-use-after-free Read in l2cap_send_cmd
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1273d3df980000
final oops: https://syzkaller.appspot.com/x/report.txt?x=1173d3df980000
console output: https://syzkaller.appspot.com/x/log.txt?x=1673d3df980000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+10bd8fe6741eedd2be2e@syzkaller.appspotmail.com
Fixes: 6ab54a717189 ("Bluetooth: L2CAP: Fix slab-use-after-free Read in l2cap_send_cmd")
non-paged memory
list_del corruption, ffff888021297e00->prev is LIST_POISON2 (dead000000000122)
------------[ cut here ]------------
kernel BUG at lib/list_debug.c:61!
Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN PTI
CPU: 1 UID: 0 PID: 5896 Comm: syz-executor213 Not tainted 6.14.0-rc1-next-20250204-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 12/27/2024
RIP: 0010:__list_del_entry_valid_or_report+0x12c/0x190 lib/list_debug.c:59
Code: 8c 4c 89 fe 48 89 da e8 32 8c 37 fc 90 0f 0b 48 89 df e8 27 9f 14 fd 48 c7 c7 a0 c0 60 8c 4c 89 fe 48 89 da e8 15 8c 37 fc 90 <0f> 0b 4c 89 e7 e8 0a 9f 14 fd 42 80 3c 2b 00 74 08 4c 89 e7 e8 cb
RSP: 0018:ffffc90003f6f998 EFLAGS: 00010246
RAX: 000000000000004e RBX: dead000000000122 RCX: 01454d423f7fbf00
RDX: 0000000000000000 RSI: 0000000080000000 RDI: 0000000000000000
RBP: dffffc0000000000 R08: ffffffff819f077c R09: 1ffff920007eded0
R10: dffffc0000000000 R11: fffff520007eded1 R12: dead000000000122
R13: dffffc0000000000 R14: ffff8880352248d8 R15: ffff888021297e00
FS: 00007f7ace6686c0(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f7aceeeb1d0 CR3: 000000003527c000 CR4: 00000000003526f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
__list_del_entry_valid include/linux/list.h:124 [inline]
__list_del_entry include/linux/list.h:215 [inline]
list_del_rcu include/linux/rculist.h:168 [inline]
hci_chan_del+0x70/0x1b0 net/bluetooth/hci_conn.c:2858
l2cap_conn_free net/bluetooth/l2cap_core.c:1816 [inline]
kref_put include/linux/kref.h:65 [inline]
l2cap_conn_put+0x70/0xe0 net/bluetooth/l2cap_core.c:1830
l2cap_sock_shutdown+0xa8a/0x1020 net/bluetooth/l2cap_sock.c:1377
l2cap_sock_release+0x79/0x1d0 net/bluetooth/l2cap_sock.c:1416
__sock_release net/socket.c:642 [inline]
sock_close+0xbc/0x240 net/socket.c:1393
__fput+0x3e9/0x9f0 fs/file_table.c:448
task_work_run+0x24f/0x310 kernel/task_work.c:227
ptrace_notify+0x2d2/0x380 kernel/signal.c:2522
ptrace_report_syscall include/linux/ptrace.h:415 [inline]
ptrace_report_syscall_exit include/linux/ptrace.h:477 [inline]
syscall_exit_work+0xc7/0x1d0 kernel/entry/common.c:173
syscall_exit_to_user_mode_prepare kernel/entry/common.c:200 [inline]
__syscall_exit_to_user_mode_work kernel/entry/common.c:205 [inline]
syscall_exit_to_user_mode+0x24a/0x340 kernel/entry/common.c:218
do_syscall_64+0x100/0x230 arch/x86/entry/common.c:89
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f7aceeaf449
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 41 19 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f7ace668218 EFLAGS: 00000246 ORIG_RAX: 000000000000002a
RAX: fffffffffffffffc RBX: 00007f7acef39328 RCX: 00007f7aceeaf449
RDX: 000000000000000e RSI: 0000000020000100 RDI: 0000000000000004
RBP: 00007f7acef39320 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000003
R13: 0000000000000004 R14: 00007f7ace668670 R15: 000000000000000b
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:__list_del_entry_valid_or_report+0x12c/0x190 lib/list_debug.c:59
Code: 8c 4c 89 fe 48 89 da e8 32 8c 37 fc 90 0f 0b 48 89 df e8 27 9f 14 fd 48 c7 c7 a0 c0 60 8c 4c 89 fe 48 89 da e8 15 8c 37 fc 90 <0f> 0b 4c 89 e7 e8 0a 9f 14 fd 42 80 3c 2b 00 74 08 4c 89 e7 e8 cb
RSP: 0018:ffffc90003f6f998 EFLAGS: 00010246
RAX: 000000000000004e RBX: dead000000000122 RCX: 01454d423f7fbf00
RDX: 0000000000000000 RSI: 0000000080000000 RDI: 0000000000000000
RBP: dffffc0000000000 R08: ffffffff819f077c R09: 1ffff920007eded0
R10: dffffc0000000000 R11: fffff520007eded1 R12: dead000000000122
R13: dffffc0000000000 R14: ffff8880352248d8 R15: ffff888021297e00
FS: 00007f7ace6686c0(0000) GS:ffff8880b8600000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f7acef05b08 CR3: 000000003527c000 CR4: 00000000003526f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [syzbot] Re: [PATCH v1] Bluetooth: L2CAP: Fix corrupted list in hci_chan_del
2025-02-06 13:10 [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2) syzbot
@ 2025-02-06 22:05 ` syzbot
2025-02-07 2:20 ` [syzbot] Re: [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2) syzbot
` (6 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: syzbot @ 2025-02-06 22:05 UTC (permalink / raw)
To: linux-kernel, syzkaller-bugs
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.
***
Subject: Re: [PATCH v1] Bluetooth: L2CAP: Fix corrupted list in hci_chan_del
Author: luiz.dentz@gmail.com
#syz test
On Thu, Feb 6, 2025 at 4:01 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This fixes the following trace by reworking the locking of l2cap_conn
> so instead of only locking when changing the chan_l list this promotes
> chan_lock to a general lock of l2cap_conn so whenever it is being held
> it would prevents the likes of l2cap_conn_del to run:
>
> list_del corruption, ffff888021297e00->prev is LIST_POISON2 (dead000000000122)
> ------------[ cut here ]------------
> kernel BUG at lib/list_debug.c:61!
> Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN PTI
> CPU: 1 UID: 0 PID: 5896 Comm: syz-executor213 Not tainted 6.14.0-rc1-next-20250204-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 12/27/2024
> RIP: 0010:__list_del_entry_valid_or_report+0x12c/0x190 lib/list_debug.c:59
> Code: 8c 4c 89 fe 48 89 da e8 32 8c 37 fc 90 0f 0b 48 89 df e8 27 9f 14 fd 48 c7 c7 a0 c0 60 8c 4c 89 fe 48 89 da e8 15 8c 37 fc 90 <0f> 0b 4c 89 e7 e8 0a 9f 14 fd 42 80 3c 2b 00 74 08 4c 89 e7 e8 cb
> RSP: 0018:ffffc90003f6f998 EFLAGS: 00010246
> RAX: 000000000000004e RBX: dead000000000122 RCX: 01454d423f7fbf00
> RDX: 0000000000000000 RSI: 0000000080000000 RDI: 0000000000000000
> RBP: dffffc0000000000 R08: ffffffff819f077c R09: 1ffff920007eded0
> R10: dffffc0000000000 R11: fffff520007eded1 R12: dead000000000122
> R13: dffffc0000000000 R14: ffff8880352248d8 R15: ffff888021297e00
> FS: 00007f7ace6686c0(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f7aceeeb1d0 CR3: 000000003527c000 CR4: 00000000003526f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> <TASK>
> __list_del_entry_valid include/linux/list.h:124 [inline]
> __list_del_entry include/linux/list.h:215 [inline]
> list_del_rcu include/linux/rculist.h:168 [inline]
> hci_chan_del+0x70/0x1b0 net/bluetooth/hci_conn.c:2858
> l2cap_conn_free net/bluetooth/l2cap_core.c:1816 [inline]
> kref_put include/linux/kref.h:65 [inline]
> l2cap_conn_put+0x70/0xe0 net/bluetooth/l2cap_core.c:1830
> l2cap_sock_shutdown+0xa8a/0x1020 net/bluetooth/l2cap_sock.c:1377
> l2cap_sock_release+0x79/0x1d0 net/bluetooth/l2cap_sock.c:1416
> __sock_release net/socket.c:642 [inline]
> sock_close+0xbc/0x240 net/socket.c:1393
> __fput+0x3e9/0x9f0 fs/file_table.c:448
> task_work_run+0x24f/0x310 kernel/task_work.c:227
> ptrace_notify+0x2d2/0x380 kernel/signal.c:2522
> ptrace_report_syscall include/linux/ptrace.h:415 [inline]
> ptrace_report_syscall_exit include/linux/ptrace.h:477 [inline]
> syscall_exit_work+0xc7/0x1d0 kernel/entry/common.c:173
> syscall_exit_to_user_mode_prepare kernel/entry/common.c:200 [inline]
> __syscall_exit_to_user_mode_work kernel/entry/common.c:205 [inline]
> syscall_exit_to_user_mode+0x24a/0x340 kernel/entry/common.c:218
> do_syscall_64+0x100/0x230 arch/x86/entry/common.c:89
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7f7aceeaf449
> Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 41 19 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007f7ace668218 EFLAGS: 00000246 ORIG_RAX: 000000000000002a
> RAX: fffffffffffffffc RBX: 00007f7acef39328 RCX: 00007f7aceeaf449
> RDX: 000000000000000e RSI: 0000000020000100 RDI: 0000000000000004
> RBP: 00007f7acef39320 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000003
> R13: 0000000000000004 R14: 00007f7ace668670 R15: 000000000000000b
> </TASK>
> Modules linked in:
> ---[ end trace 0000000000000000 ]---
> RIP: 0010:__list_del_entry_valid_or_report+0x12c/0x190 lib/list_debug.c:59
> Code: 8c 4c 89 fe 48 89 da e8 32 8c 37 fc 90 0f 0b 48 89 df e8 27 9f 14 fd 48 c7 c7 a0 c0 60 8c 4c 89 fe 48 89 da e8 15 8c 37 fc 90 <0f> 0b 4c 89 e7 e8 0a 9f 14 fd 42 80 3c 2b 00 74 08 4c 89 e7 e8 cb
> RSP: 0018:ffffc90003f6f998 EFLAGS: 00010246
> RAX: 000000000000004e RBX: dead000000000122 RCX: 01454d423f7fbf00
> RDX: 0000000000000000 RSI: 0000000080000000 RDI: 0000000000000000
> RBP: dffffc0000000000 R08: ffffffff819f077c R09: 1ffff920007eded0
> R10: dffffc0000000000 R11: fffff520007eded1 R12: dead000000000122
> R13: dffffc0000000000 R14: ffff8880352248d8 R15: ffff888021297e00
> FS: 00007f7ace6686c0(0000) GS:ffff8880b8600000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f7acef05b08 CR3: 000000003527c000 CR4: 00000000003526f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>
> Reported-by: syzbot+10bd8fe6741eedd2be2e@syzkaller.appspotmail.com
> Fixes: 6ab54a717189 ("Bluetooth: L2CAP: Fix slab-use-after-free Read in l2cap_send_cmd")
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> include/net/bluetooth/l2cap.h | 2 +-
> net/bluetooth/l2cap_core.c | 123 +++++++++++-----------------------
> net/bluetooth/l2cap_sock.c | 4 +-
> 3 files changed, 41 insertions(+), 88 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index d9c767cf773d..6b90cfa15d88 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -668,7 +668,7 @@ struct l2cap_conn {
> struct l2cap_chan *smp;
>
> struct list_head chan_l;
> - struct mutex chan_lock;
> + struct mutex lock;
> struct kref ref;
> struct list_head users;
> };
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index adb8c33ac595..5eedd6611f6c 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -119,7 +119,6 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn,
> {
> struct l2cap_chan *c;
>
> - mutex_lock(&conn->chan_lock);
> c = __l2cap_get_chan_by_scid(conn, cid);
> if (c) {
> /* Only lock if chan reference is not 0 */
> @@ -127,7 +126,6 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn,
> if (c)
> l2cap_chan_lock(c);
> }
> - mutex_unlock(&conn->chan_lock);
>
> return c;
> }
> @@ -140,7 +138,6 @@ static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
> {
> struct l2cap_chan *c;
>
> - mutex_lock(&conn->chan_lock);
> c = __l2cap_get_chan_by_dcid(conn, cid);
> if (c) {
> /* Only lock if chan reference is not 0 */
> @@ -148,7 +145,6 @@ static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
> if (c)
> l2cap_chan_lock(c);
> }
> - mutex_unlock(&conn->chan_lock);
>
> return c;
> }
> @@ -418,7 +414,7 @@ static void l2cap_chan_timeout(struct work_struct *work)
> if (!conn)
> return;
>
> - mutex_lock(&conn->chan_lock);
> + mutex_lock(&conn->lock);
> /* __set_chan_timer() calls l2cap_chan_hold(chan) while scheduling
> * this work. No need to call l2cap_chan_hold(chan) here again.
> */
> @@ -439,7 +435,7 @@ static void l2cap_chan_timeout(struct work_struct *work)
> l2cap_chan_unlock(chan);
> l2cap_chan_put(chan);
>
> - mutex_unlock(&conn->chan_lock);
> + mutex_unlock(&conn->lock);
> }
>
> struct l2cap_chan *l2cap_chan_create(void)
> @@ -641,9 +637,9 @@ void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
>
> void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
> {
> - mutex_lock(&conn->chan_lock);
> + mutex_lock(&conn->lock);
> __l2cap_chan_add(conn, chan);
> - mutex_unlock(&conn->chan_lock);
> + mutex_unlock(&conn->lock);
> }
>
> void l2cap_chan_del(struct l2cap_chan *chan, int err)
> @@ -731,9 +727,9 @@ void l2cap_chan_list(struct l2cap_conn *conn, l2cap_chan_func_t func,
> if (!conn)
> return;
>
> - mutex_lock(&conn->chan_lock);
> + mutex_lock(&conn->lock);
> __l2cap_chan_list(conn, func, data);
> - mutex_unlock(&conn->chan_lock);
> + mutex_unlock(&conn->lock);
> }
>
> EXPORT_SYMBOL_GPL(l2cap_chan_list);
> @@ -745,7 +741,7 @@ static void l2cap_conn_update_id_addr(struct work_struct *work)
> struct hci_conn *hcon = conn->hcon;
> struct l2cap_chan *chan;
>
> - mutex_lock(&conn->chan_lock);
> + mutex_lock(&conn->lock);
>
> list_for_each_entry(chan, &conn->chan_l, list) {
> l2cap_chan_lock(chan);
> @@ -754,7 +750,7 @@ static void l2cap_conn_update_id_addr(struct work_struct *work)
> l2cap_chan_unlock(chan);
> }
>
> - mutex_unlock(&conn->chan_lock);
> + mutex_unlock(&conn->lock);
> }
>
> static void l2cap_chan_le_connect_reject(struct l2cap_chan *chan)
> @@ -1507,8 +1503,6 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
>
> BT_DBG("conn %p", conn);
>
> - mutex_lock(&conn->chan_lock);
> -
> list_for_each_entry_safe(chan, tmp, &conn->chan_l, list) {
> l2cap_chan_lock(chan);
>
> @@ -1577,8 +1571,6 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
>
> l2cap_chan_unlock(chan);
> }
> -
> - mutex_unlock(&conn->chan_lock);
> }
>
> static void l2cap_le_conn_ready(struct l2cap_conn *conn)
> @@ -1624,7 +1616,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
> if (hcon->type == ACL_LINK)
> l2cap_request_info(conn);
>
> - mutex_lock(&conn->chan_lock);
> + mutex_lock(&conn->lock);
>
> list_for_each_entry(chan, &conn->chan_l, list) {
>
> @@ -1642,7 +1634,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
> l2cap_chan_unlock(chan);
> }
>
> - mutex_unlock(&conn->chan_lock);
> + mutex_unlock(&conn->lock);
>
> if (hcon->type == LE_LINK)
> l2cap_le_conn_ready(conn);
> @@ -1657,14 +1649,10 @@ static void l2cap_conn_unreliable(struct l2cap_conn *conn, int err)
>
> BT_DBG("conn %p", conn);
>
> - mutex_lock(&conn->chan_lock);
> -
> list_for_each_entry(chan, &conn->chan_l, list) {
> if (test_bit(FLAG_FORCE_RELIABLE, &chan->flags))
> l2cap_chan_set_err(chan, err);
> }
> -
> - mutex_unlock(&conn->chan_lock);
> }
>
> static void l2cap_info_timeout(struct work_struct *work)
> @@ -1675,7 +1663,9 @@ static void l2cap_info_timeout(struct work_struct *work)
> conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
> conn->info_ident = 0;
>
> + mutex_lock(&conn->lock);
> l2cap_conn_start(conn);
> + mutex_unlock(&conn->lock);
> }
>
> /*
> @@ -1767,6 +1757,8 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
>
> BT_DBG("hcon %p conn %p, err %d", hcon, conn, err);
>
> + mutex_lock(&conn->lock);
> +
> kfree_skb(conn->rx_skb);
>
> skb_queue_purge(&conn->pending_rx);
> @@ -1785,8 +1777,6 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
> /* Force the connection to be immediately dropped */
> hcon->disc_timeout = 0;
>
> - mutex_lock(&conn->chan_lock);
> -
> /* Kill channels */
> list_for_each_entry_safe(chan, l, &conn->chan_l, list) {
> l2cap_chan_hold(chan);
> @@ -1800,12 +1790,11 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
> l2cap_chan_put(chan);
> }
>
> - mutex_unlock(&conn->chan_lock);
> -
> if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT)
> cancel_delayed_work_sync(&conn->info_timer);
>
> hcon->l2cap_data = NULL;
> + mutex_unlock(&conn->lock);
> l2cap_conn_put(conn);
> }
>
> @@ -2924,8 +2913,6 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb)
>
> BT_DBG("conn %p", conn);
>
> - mutex_lock(&conn->chan_lock);
> -
> list_for_each_entry(chan, &conn->chan_l, list) {
> if (chan->chan_type != L2CAP_CHAN_RAW)
> continue;
> @@ -2940,8 +2927,6 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb)
> if (chan->ops->recv(chan, nskb))
> kfree_skb(nskb);
> }
> -
> - mutex_unlock(&conn->chan_lock);
> }
>
> /* ---- L2CAP signalling commands ---- */
> @@ -3960,7 +3945,6 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
> goto response;
> }
>
> - mutex_lock(&conn->chan_lock);
> l2cap_chan_lock(pchan);
>
> /* Check if the ACL is secure enough (if not SDP) */
> @@ -4067,7 +4051,6 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
> }
>
> l2cap_chan_unlock(pchan);
> - mutex_unlock(&conn->chan_lock);
> l2cap_chan_put(pchan);
> }
>
> @@ -4106,27 +4089,19 @@ static int l2cap_connect_create_rsp(struct l2cap_conn *conn,
> BT_DBG("dcid 0x%4.4x scid 0x%4.4x result 0x%2.2x status 0x%2.2x",
> dcid, scid, result, status);
>
> - mutex_lock(&conn->chan_lock);
> -
> if (scid) {
> chan = __l2cap_get_chan_by_scid(conn, scid);
> - if (!chan) {
> - err = -EBADSLT;
> - goto unlock;
> - }
> + if (!chan)
> + return -EBADSLT;
> } else {
> chan = __l2cap_get_chan_by_ident(conn, cmd->ident);
> - if (!chan) {
> - err = -EBADSLT;
> - goto unlock;
> - }
> + if (!chan)
> + return -EBADSLT;
> }
>
> chan = l2cap_chan_hold_unless_zero(chan);
> - if (!chan) {
> - err = -EBADSLT;
> - goto unlock;
> - }
> + if (!chan)
> + return -EBADSLT;
>
> err = 0;
>
> @@ -4164,9 +4139,6 @@ static int l2cap_connect_create_rsp(struct l2cap_conn *conn,
> l2cap_chan_unlock(chan);
> l2cap_chan_put(chan);
>
> -unlock:
> - mutex_unlock(&conn->chan_lock);
> -
> return err;
> }
>
> @@ -4454,11 +4426,7 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn,
>
> chan->ops->set_shutdown(chan);
>
> - l2cap_chan_unlock(chan);
> - mutex_lock(&conn->chan_lock);
> - l2cap_chan_lock(chan);
> l2cap_chan_del(chan, ECONNRESET);
> - mutex_unlock(&conn->chan_lock);
>
> chan->ops->close(chan);
>
> @@ -4495,11 +4463,7 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn,
> return 0;
> }
>
> - l2cap_chan_unlock(chan);
> - mutex_lock(&conn->chan_lock);
> - l2cap_chan_lock(chan);
> l2cap_chan_del(chan, 0);
> - mutex_unlock(&conn->chan_lock);
>
> chan->ops->close(chan);
>
> @@ -4697,13 +4661,9 @@ static int l2cap_le_connect_rsp(struct l2cap_conn *conn,
> BT_DBG("dcid 0x%4.4x mtu %u mps %u credits %u result 0x%2.2x",
> dcid, mtu, mps, credits, result);
>
> - mutex_lock(&conn->chan_lock);
> -
> chan = __l2cap_get_chan_by_ident(conn, cmd->ident);
> - if (!chan) {
> - err = -EBADSLT;
> - goto unlock;
> - }
> + if (!chan)
> + return -EBADSLT;
>
> err = 0;
>
> @@ -4751,9 +4711,6 @@ static int l2cap_le_connect_rsp(struct l2cap_conn *conn,
>
> l2cap_chan_unlock(chan);
>
> -unlock:
> - mutex_unlock(&conn->chan_lock);
> -
> return err;
> }
>
> @@ -4865,7 +4822,6 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
> goto response;
> }
>
> - mutex_lock(&conn->chan_lock);
> l2cap_chan_lock(pchan);
>
> if (!smp_sufficient_security(conn->hcon, pchan->sec_level,
> @@ -4931,7 +4887,6 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
>
> response_unlock:
> l2cap_chan_unlock(pchan);
> - mutex_unlock(&conn->chan_lock);
> l2cap_chan_put(pchan);
>
> if (result == L2CAP_CR_PEND)
> @@ -5065,7 +5020,6 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
> goto response;
> }
>
> - mutex_lock(&conn->chan_lock);
> l2cap_chan_lock(pchan);
>
> if (!smp_sufficient_security(conn->hcon, pchan->sec_level,
> @@ -5140,7 +5094,6 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
>
> unlock:
> l2cap_chan_unlock(pchan);
> - mutex_unlock(&conn->chan_lock);
> l2cap_chan_put(pchan);
>
> response:
> @@ -5177,8 +5130,6 @@ static inline int l2cap_ecred_conn_rsp(struct l2cap_conn *conn,
> BT_DBG("mtu %u mps %u credits %u result 0x%4.4x", mtu, mps, credits,
> result);
>
> - mutex_lock(&conn->chan_lock);
> -
> cmd_len -= sizeof(*rsp);
>
> list_for_each_entry_safe(chan, tmp, &conn->chan_l, list) {
> @@ -5264,8 +5215,6 @@ static inline int l2cap_ecred_conn_rsp(struct l2cap_conn *conn,
> l2cap_chan_unlock(chan);
> }
>
> - mutex_unlock(&conn->chan_lock);
> -
> return err;
> }
>
> @@ -5378,8 +5327,6 @@ static inline int l2cap_le_command_rej(struct l2cap_conn *conn,
> if (cmd_len < sizeof(*rej))
> return -EPROTO;
>
> - mutex_lock(&conn->chan_lock);
> -
> chan = __l2cap_get_chan_by_ident(conn, cmd->ident);
> if (!chan)
> goto done;
> @@ -5394,7 +5341,6 @@ static inline int l2cap_le_command_rej(struct l2cap_conn *conn,
> l2cap_chan_put(chan);
>
> done:
> - mutex_unlock(&conn->chan_lock);
> return 0;
> }
>
> @@ -6849,8 +6795,12 @@ static void process_pending_rx(struct work_struct *work)
>
> BT_DBG("");
>
> + mutex_lock(&conn->lock);
> +
> while ((skb = skb_dequeue(&conn->pending_rx)))
> l2cap_recv_frame(conn, skb);
> +
> + mutex_unlock(&conn->lock);
> }
>
> static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
> @@ -6889,7 +6839,7 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
> conn->local_fixed_chan |= L2CAP_FC_SMP_BREDR;
>
> mutex_init(&conn->ident_lock);
> - mutex_init(&conn->chan_lock);
> + mutex_init(&conn->lock);
>
> INIT_LIST_HEAD(&conn->chan_l);
> INIT_LIST_HEAD(&conn->users);
> @@ -7080,7 +7030,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
> }
> }
>
> - mutex_lock(&conn->chan_lock);
> + mutex_lock(&conn->lock);
> l2cap_chan_lock(chan);
>
> if (cid && __l2cap_get_chan_by_dcid(conn, cid)) {
> @@ -7121,7 +7071,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
>
> chan_unlock:
> l2cap_chan_unlock(chan);
> - mutex_unlock(&conn->chan_lock);
> + mutex_unlock(&conn->lock);
> done:
> hci_dev_unlock(hdev);
> hci_dev_put(hdev);
> @@ -7333,7 +7283,7 @@ static void l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
>
> BT_DBG("conn %p status 0x%2.2x encrypt %u", conn, status, encrypt);
>
> - mutex_lock(&conn->chan_lock);
> + mutex_lock(&conn->lock);
>
> list_for_each_entry(chan, &conn->chan_l, list) {
> l2cap_chan_lock(chan);
> @@ -7407,7 +7357,7 @@ static void l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> l2cap_chan_unlock(chan);
> }
>
> - mutex_unlock(&conn->chan_lock);
> + mutex_unlock(&conn->lock);
> }
>
> /* Append fragment into frame respecting the maximum len of rx_skb */
> @@ -7506,6 +7456,8 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
>
> BT_DBG("conn %p len %u flags 0x%x", conn, skb->len, flags);
>
> + mutex_lock(&conn->lock);
> +
> switch (flags) {
> case ACL_START:
> case ACL_START_NO_FLUSH:
> @@ -7530,7 +7482,7 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
> if (len == skb->len) {
> /* Complete frame received */
> l2cap_recv_frame(conn, skb);
> - return;
> + goto unlock;
> }
>
> BT_DBG("Start: total len %d, frag len %u", len, skb->len);
> @@ -7592,10 +7544,11 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
> break;
> }
>
> - l2cap_conn_put(conn);
> -
> drop:
> kfree_skb(skb);
> +unlock:
> + mutex_unlock(&conn->lock);
> + l2cap_conn_put(conn);
> }
>
> static struct hci_cb l2cap_cb = {
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 46ea0bee2259..831ab9b97001 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -1366,14 +1366,14 @@ static int l2cap_sock_shutdown(struct socket *sock, int how)
>
> if (conn)
> /* mutex lock must be taken before l2cap_chan_lock() */
> - mutex_lock(&conn->chan_lock);
> + mutex_lock(&conn->lock);
>
> l2cap_chan_lock(chan);
> l2cap_chan_close(chan, 0);
> l2cap_chan_unlock(chan);
>
> if (conn) {
> - mutex_unlock(&conn->chan_lock);
> + mutex_unlock(&conn->lock);
> l2cap_conn_put(conn);
> }
>
> --
> 2.48.1
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [syzbot] Re: [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2)
2025-02-06 13:10 [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2) syzbot
2025-02-06 22:05 ` [syzbot] Re: [PATCH v1] Bluetooth: L2CAP: Fix corrupted list in hci_chan_del syzbot
@ 2025-02-07 2:20 ` syzbot
2025-02-07 4:09 ` syzbot
` (5 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: syzbot @ 2025-02-07 2:20 UTC (permalink / raw)
To: linux-kernel
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org.
***
Subject: Re: [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2)
Author: lizhi.xu@windriver.com
old logical will make get/put unbalance in l2cap_recv_acldata.
so remote get/put conn.
#syz test
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index adb8c33ac595..503626f70be5 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -7497,8 +7497,6 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
if (!conn)
conn = l2cap_conn_add(hcon);
- conn = l2cap_conn_hold_unless_zero(conn);
-
hci_dev_unlock(hcon->hdev);
if (!conn)
@@ -7592,8 +7590,6 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
break;
}
- l2cap_conn_put(conn);
-
drop:
kfree_skb(skb);
}
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [syzbot] Re: [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2)
2025-02-06 13:10 [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2) syzbot
2025-02-06 22:05 ` [syzbot] Re: [PATCH v1] Bluetooth: L2CAP: Fix corrupted list in hci_chan_del syzbot
2025-02-07 2:20 ` [syzbot] Re: [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2) syzbot
@ 2025-02-07 4:09 ` syzbot
2025-02-07 6:37 ` syzbot
` (4 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: syzbot @ 2025-02-07 4:09 UTC (permalink / raw)
To: linux-kernel
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org.
***
Subject: Re: [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2)
Author: lizhi.xu@windriver.com
old logical will make get/put unbalance in l2cap_recv_acldata.
so remote get/put conn.
move chan_del from l2cap_conn_free to conn_del, avoid up level l2cap_sock_release double call it.
#syz test
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index adb8c33ac595..bbaf2141578b 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1801,6 +1801,7 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
}
mutex_unlock(&conn->chan_lock);
+ hci_chan_del(conn->hchan);
if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT)
cancel_delayed_work_sync(&conn->info_timer);
@@ -1813,7 +1814,6 @@ static void l2cap_conn_free(struct kref *ref)
{
struct l2cap_conn *conn = container_of(ref, struct l2cap_conn, ref);
- hci_chan_del(conn->hchan);
hci_conn_put(conn->hcon);
kfree(conn);
}
@@ -7497,8 +7497,6 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
if (!conn)
conn = l2cap_conn_add(hcon);
- conn = l2cap_conn_hold_unless_zero(conn);
-
hci_dev_unlock(hcon->hdev);
if (!conn)
@@ -7592,8 +7590,6 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
break;
}
- l2cap_conn_put(conn);
-
drop:
kfree_skb(skb);
}
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [syzbot] Re: [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2)
2025-02-06 13:10 [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2) syzbot
` (2 preceding siblings ...)
2025-02-07 4:09 ` syzbot
@ 2025-02-07 6:37 ` syzbot
2025-02-07 12:25 ` syzbot
` (3 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: syzbot @ 2025-02-07 6:37 UTC (permalink / raw)
To: linux-kernel
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org.
***
Subject: Re: [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2)
Author: lizhi.xu@windriver.com
old logical will make get/put unbalance in l2cap_recv_acldata.
so remote get/put conn.
protect conn refcnt under hci dev lock.
#syz test
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index adb8c33ac595..503626f70be5 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -7497,8 +7497,6 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
if (!conn)
conn = l2cap_conn_add(hcon);
- conn = l2cap_conn_hold_unless_zero(conn);
-
hci_dev_unlock(hcon->hdev);
if (!conn)
@@ -7592,8 +7590,6 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
break;
}
- l2cap_conn_put(conn);
-
drop:
kfree_skb(skb);
}
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 46ea0bee2259..697e3b56f119 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1359,9 +1359,11 @@ static int l2cap_sock_shutdown(struct socket *sock, int how)
l2cap_chan_lock(chan);
conn = chan->conn;
- if (conn)
+ if (conn) {
+ hci_dev_lock(conn->hcon->hdev);
/* prevent conn structure from being freed */
l2cap_conn_get(conn);
+ }
l2cap_chan_unlock(chan);
if (conn)
@@ -1375,6 +1377,7 @@ static int l2cap_sock_shutdown(struct socket *sock, int how)
if (conn) {
mutex_unlock(&conn->chan_lock);
l2cap_conn_put(conn);
+ hci_dev_unlock(conn->hcon->hdev);
}
lock_sock(sk);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [syzbot] Re: [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2)
2025-02-06 13:10 [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2) syzbot
` (3 preceding siblings ...)
2025-02-07 6:37 ` syzbot
@ 2025-02-07 12:25 ` syzbot
2025-02-07 14:02 ` [PATCH next] Bluetooth: l2cap: protect conn refcnt under hci dev lock Lizhi Xu
` (2 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: syzbot @ 2025-02-07 12:25 UTC (permalink / raw)
To: linux-kernel
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org.
***
Subject: Re: [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2)
Author: lizhi.xu@windriver.com
old logical will make get/put unbalance in l2cap_recv_acldata.
so remote get/put conn.
protect conn refcnt under hci dev lock.
#syz test
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index adb8c33ac595..503626f70be5 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -7497,8 +7497,6 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
if (!conn)
conn = l2cap_conn_add(hcon);
- conn = l2cap_conn_hold_unless_zero(conn);
-
hci_dev_unlock(hcon->hdev);
if (!conn)
@@ -7592,8 +7590,6 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
break;
}
- l2cap_conn_put(conn);
-
drop:
kfree_skb(skb);
}
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 46ea0bee2259..2a99394925a5 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1359,10 +1359,12 @@ static int l2cap_sock_shutdown(struct socket *sock, int how)
l2cap_chan_lock(chan);
conn = chan->conn;
- if (conn)
+ l2cap_chan_unlock(chan);
+ if (conn) {
+ hci_dev_lock(conn->hcon->hdev);
/* prevent conn structure from being freed */
l2cap_conn_get(conn);
- l2cap_chan_unlock(chan);
+ }
if (conn)
/* mutex lock must be taken before l2cap_chan_lock() */
@@ -1375,6 +1377,7 @@ static int l2cap_sock_shutdown(struct socket *sock, int how)
if (conn) {
mutex_unlock(&conn->chan_lock);
l2cap_conn_put(conn);
+ hci_dev_unlock(conn->hcon->hdev);
}
lock_sock(sk);
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH next] Bluetooth: l2cap: protect conn refcnt under hci dev lock
2025-02-06 13:10 [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2) syzbot
` (4 preceding siblings ...)
2025-02-07 12:25 ` syzbot
@ 2025-02-07 14:02 ` Lizhi Xu
2025-02-08 8:20 ` kernel test robot
2025-02-07 16:13 ` [syzbot] Re: [PATCH v1] Bluetooth: L2CAP: Fix corrupted list in hci_chan_del syzbot
2025-02-07 16:54 ` syzbot
7 siblings, 1 reply; 10+ messages in thread
From: Lizhi Xu @ 2025-02-07 14:02 UTC (permalink / raw)
To: syzbot+10bd8fe6741eedd2be2e
Cc: eadavis, johan.hedberg, linux-bluetooth, linux-kernel,
luiz.dentz, luiz.von.dentz, marcel, syzkaller-bugs
syzbot reported a corrupted list in hci_chan_del. [1]
Use hci dev lock to protect the reference count of conn to avoid race
conditions on l2cap_sock_shutdown and l2cap_connect_cfm paths.
The get/put actions of conn are unbalanced due to abnormal exit, so remove
the redundant get/put actions of conn in l2cap_recv_acldata().
[1]
kernel BUG at lib/list_debug.c:61!
Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN PTI
CPU: 1 UID: 0 PID: 5896 Comm: syz-executor213 Not tainted 6.14.0-rc1-next-20250204-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 12/27/2024
RIP: 0010:__list_del_entry_valid_or_report+0x12c/0x190 lib/list_debug.c:59
Code: 8c 4c 89 fe 48 89 da e8 32 8c 37 fc 90 0f 0b 48 89 df e8 27 9f 14 fd 48 c7 c7 a0 c0 60 8c 4c 89 fe 48 89 da e8 15 8c 37 fc 90 <0f> 0b 4c 89 e7 e8 0a 9f 14 fd 42 80 3c 2b 00 74 08 4c 89 e7 e8 cb
RSP: 0018:ffffc90003f6f998 EFLAGS: 00010246
RAX: 000000000000004e RBX: dead000000000122 RCX: 01454d423f7fbf00
RDX: 0000000000000000 RSI: 0000000080000000 RDI: 0000000000000000
RBP: dffffc0000000000 R08: ffffffff819f077c R09: 1ffff920007eded0
R10: dffffc0000000000 R11: fffff520007eded1 R12: dead000000000122
R13: dffffc0000000000 R14: ffff8880352248d8 R15: ffff888021297e00
FS: 00007f7ace6686c0(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f7aceeeb1d0 CR3: 000000003527c000 CR4: 00000000003526f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
__list_del_entry_valid include/linux/list.h:124 [inline]
__list_del_entry include/linux/list.h:215 [inline]
list_del_rcu include/linux/rculist.h:168 [inline]
hci_chan_del+0x70/0x1b0 net/bluetooth/hci_conn.c:2858
l2cap_conn_free net/bluetooth/l2cap_core.c:1816 [inline]
kref_put include/linux/kref.h:65 [inline]
l2cap_conn_put+0x70/0xe0 net/bluetooth/l2cap_core.c:1830
l2cap_sock_shutdown+0xa8a/0x1020 net/bluetooth/l2cap_sock.c:1377
l2cap_sock_release+0x79/0x1d0 net/bluetooth/l2cap_sock.c:1416
__sock_release net/socket.c:642 [inline]
sock_close+0xbc/0x240 net/socket.c:1393
__fput+0x3e9/0x9f0 fs/file_table.c:448
task_work_run+0x24f/0x310 kernel/task_work.c:227
ptrace_notify+0x2d2/0x380 kernel/signal.c:2522
ptrace_report_syscall include/linux/ptrace.h:415 [inline]
ptrace_report_syscall_exit include/linux/ptrace.h:477 [inline]
syscall_exit_work+0xc7/0x1d0 kernel/entry/common.c:173
syscall_exit_to_user_mode_prepare kernel/entry/common.c:200 [inline]
__syscall_exit_to_user_mode_work kernel/entry/common.c:205 [inline]
syscall_exit_to_user_mode+0x24a/0x340 kernel/entry/common.c:218
do_syscall_64+0x100/0x230 arch/x86/entry/common.c:89
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Reported-by: syzbot+10bd8fe6741eedd2be2e@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=10bd8fe6741eedd2be2e
Tested-by: syzbot+10bd8fe6741eedd2be2e@syzkaller.appspotmail.com
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
net/bluetooth/l2cap_core.c | 4 ----
net/bluetooth/l2cap_sock.c | 7 +++++--
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index adb8c33ac595..503626f70be5 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -7497,8 +7497,6 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
if (!conn)
conn = l2cap_conn_add(hcon);
- conn = l2cap_conn_hold_unless_zero(conn);
-
hci_dev_unlock(hcon->hdev);
if (!conn)
@@ -7592,8 +7590,6 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
break;
}
- l2cap_conn_put(conn);
-
drop:
kfree_skb(skb);
}
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 46ea0bee2259..2a99394925a5 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1359,10 +1359,12 @@ static int l2cap_sock_shutdown(struct socket *sock, int how)
l2cap_chan_lock(chan);
conn = chan->conn;
- if (conn)
+ l2cap_chan_unlock(chan);
+ if (conn) {
+ hci_dev_lock(conn->hcon->hdev);
/* prevent conn structure from being freed */
l2cap_conn_get(conn);
- l2cap_chan_unlock(chan);
+ }
if (conn)
/* mutex lock must be taken before l2cap_chan_lock() */
@@ -1375,6 +1377,7 @@ static int l2cap_sock_shutdown(struct socket *sock, int how)
if (conn) {
mutex_unlock(&conn->chan_lock);
l2cap_conn_put(conn);
+ hci_dev_unlock(conn->hcon->hdev);
}
lock_sock(sk);
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [syzbot] Re: [PATCH v1] Bluetooth: L2CAP: Fix corrupted list in hci_chan_del
2025-02-06 13:10 [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2) syzbot
` (5 preceding siblings ...)
2025-02-07 14:02 ` [PATCH next] Bluetooth: l2cap: protect conn refcnt under hci dev lock Lizhi Xu
@ 2025-02-07 16:13 ` syzbot
2025-02-07 16:54 ` syzbot
7 siblings, 0 replies; 10+ messages in thread
From: syzbot @ 2025-02-07 16:13 UTC (permalink / raw)
To: linux-kernel, syzkaller-bugs
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.
***
Subject: Re: [PATCH v1] Bluetooth: L2CAP: Fix corrupted list in hci_chan_del
Author: luiz.dentz@gmail.com
#syz test
On Thu, Feb 6, 2025 at 5:05 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> #syz test
>
> On Thu, Feb 6, 2025 at 4:01 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >
> > This fixes the following trace by reworking the locking of l2cap_conn
> > so instead of only locking when changing the chan_l list this promotes
> > chan_lock to a general lock of l2cap_conn so whenever it is being held
> > it would prevents the likes of l2cap_conn_del to run:
> >
> > list_del corruption, ffff888021297e00->prev is LIST_POISON2 (dead000000000122)
> > ------------[ cut here ]------------
> > kernel BUG at lib/list_debug.c:61!
> > Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN PTI
> > CPU: 1 UID: 0 PID: 5896 Comm: syz-executor213 Not tainted 6.14.0-rc1-next-20250204-syzkaller #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 12/27/2024
> > RIP: 0010:__list_del_entry_valid_or_report+0x12c/0x190 lib/list_debug.c:59
> > Code: 8c 4c 89 fe 48 89 da e8 32 8c 37 fc 90 0f 0b 48 89 df e8 27 9f 14 fd 48 c7 c7 a0 c0 60 8c 4c 89 fe 48 89 da e8 15 8c 37 fc 90 <0f> 0b 4c 89 e7 e8 0a 9f 14 fd 42 80 3c 2b 00 74 08 4c 89 e7 e8 cb
> > RSP: 0018:ffffc90003f6f998 EFLAGS: 00010246
> > RAX: 000000000000004e RBX: dead000000000122 RCX: 01454d423f7fbf00
> > RDX: 0000000000000000 RSI: 0000000080000000 RDI: 0000000000000000
> > RBP: dffffc0000000000 R08: ffffffff819f077c R09: 1ffff920007eded0
> > R10: dffffc0000000000 R11: fffff520007eded1 R12: dead000000000122
> > R13: dffffc0000000000 R14: ffff8880352248d8 R15: ffff888021297e00
> > FS: 00007f7ace6686c0(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 00007f7aceeeb1d0 CR3: 000000003527c000 CR4: 00000000003526f0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > Call Trace:
> > <TASK>
> > __list_del_entry_valid include/linux/list.h:124 [inline]
> > __list_del_entry include/linux/list.h:215 [inline]
> > list_del_rcu include/linux/rculist.h:168 [inline]
> > hci_chan_del+0x70/0x1b0 net/bluetooth/hci_conn.c:2858
> > l2cap_conn_free net/bluetooth/l2cap_core.c:1816 [inline]
> > kref_put include/linux/kref.h:65 [inline]
> > l2cap_conn_put+0x70/0xe0 net/bluetooth/l2cap_core.c:1830
> > l2cap_sock_shutdown+0xa8a/0x1020 net/bluetooth/l2cap_sock.c:1377
> > l2cap_sock_release+0x79/0x1d0 net/bluetooth/l2cap_sock.c:1416
> > __sock_release net/socket.c:642 [inline]
> > sock_close+0xbc/0x240 net/socket.c:1393
> > __fput+0x3e9/0x9f0 fs/file_table.c:448
> > task_work_run+0x24f/0x310 kernel/task_work.c:227
> > ptrace_notify+0x2d2/0x380 kernel/signal.c:2522
> > ptrace_report_syscall include/linux/ptrace.h:415 [inline]
> > ptrace_report_syscall_exit include/linux/ptrace.h:477 [inline]
> > syscall_exit_work+0xc7/0x1d0 kernel/entry/common.c:173
> > syscall_exit_to_user_mode_prepare kernel/entry/common.c:200 [inline]
> > __syscall_exit_to_user_mode_work kernel/entry/common.c:205 [inline]
> > syscall_exit_to_user_mode+0x24a/0x340 kernel/entry/common.c:218
> > do_syscall_64+0x100/0x230 arch/x86/entry/common.c:89
> > entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > RIP: 0033:0x7f7aceeaf449
> > Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 41 19 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
> > RSP: 002b:00007f7ace668218 EFLAGS: 00000246 ORIG_RAX: 000000000000002a
> > RAX: fffffffffffffffc RBX: 00007f7acef39328 RCX: 00007f7aceeaf449
> > RDX: 000000000000000e RSI: 0000000020000100 RDI: 0000000000000004
> > RBP: 00007f7acef39320 R08: 0000000000000000 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000003
> > R13: 0000000000000004 R14: 00007f7ace668670 R15: 000000000000000b
> > </TASK>
> > Modules linked in:
> > ---[ end trace 0000000000000000 ]---
> > RIP: 0010:__list_del_entry_valid_or_report+0x12c/0x190 lib/list_debug.c:59
> > Code: 8c 4c 89 fe 48 89 da e8 32 8c 37 fc 90 0f 0b 48 89 df e8 27 9f 14 fd 48 c7 c7 a0 c0 60 8c 4c 89 fe 48 89 da e8 15 8c 37 fc 90 <0f> 0b 4c 89 e7 e8 0a 9f 14 fd 42 80 3c 2b 00 74 08 4c 89 e7 e8 cb
> > RSP: 0018:ffffc90003f6f998 EFLAGS: 00010246
> > RAX: 000000000000004e RBX: dead000000000122 RCX: 01454d423f7fbf00
> > RDX: 0000000000000000 RSI: 0000000080000000 RDI: 0000000000000000
> > RBP: dffffc0000000000 R08: ffffffff819f077c R09: 1ffff920007eded0
> > R10: dffffc0000000000 R11: fffff520007eded1 R12: dead000000000122
> > R13: dffffc0000000000 R14: ffff8880352248d8 R15: ffff888021297e00
> > FS: 00007f7ace6686c0(0000) GS:ffff8880b8600000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 00007f7acef05b08 CR3: 000000003527c000 CR4: 00000000003526f0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> >
> > Reported-by: syzbot+10bd8fe6741eedd2be2e@syzkaller.appspotmail.com
> > Fixes: 6ab54a717189 ("Bluetooth: L2CAP: Fix slab-use-after-free Read in l2cap_send_cmd")
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > ---
> > include/net/bluetooth/l2cap.h | 2 +-
> > net/bluetooth/l2cap_core.c | 123 +++++++++++-----------------------
> > net/bluetooth/l2cap_sock.c | 4 +-
> > 3 files changed, 41 insertions(+), 88 deletions(-)
> >
> > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> > index d9c767cf773d..6b90cfa15d88 100644
> > --- a/include/net/bluetooth/l2cap.h
> > +++ b/include/net/bluetooth/l2cap.h
> > @@ -668,7 +668,7 @@ struct l2cap_conn {
> > struct l2cap_chan *smp;
> >
> > struct list_head chan_l;
> > - struct mutex chan_lock;
> > + struct mutex lock;
> > struct kref ref;
> > struct list_head users;
> > };
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index adb8c33ac595..5eedd6611f6c 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -119,7 +119,6 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn,
> > {
> > struct l2cap_chan *c;
> >
> > - mutex_lock(&conn->chan_lock);
> > c = __l2cap_get_chan_by_scid(conn, cid);
> > if (c) {
> > /* Only lock if chan reference is not 0 */
> > @@ -127,7 +126,6 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn,
> > if (c)
> > l2cap_chan_lock(c);
> > }
> > - mutex_unlock(&conn->chan_lock);
> >
> > return c;
> > }
> > @@ -140,7 +138,6 @@ static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
> > {
> > struct l2cap_chan *c;
> >
> > - mutex_lock(&conn->chan_lock);
> > c = __l2cap_get_chan_by_dcid(conn, cid);
> > if (c) {
> > /* Only lock if chan reference is not 0 */
> > @@ -148,7 +145,6 @@ static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
> > if (c)
> > l2cap_chan_lock(c);
> > }
> > - mutex_unlock(&conn->chan_lock);
> >
> > return c;
> > }
> > @@ -418,7 +414,7 @@ static void l2cap_chan_timeout(struct work_struct *work)
> > if (!conn)
> > return;
> >
> > - mutex_lock(&conn->chan_lock);
> > + mutex_lock(&conn->lock);
> > /* __set_chan_timer() calls l2cap_chan_hold(chan) while scheduling
> > * this work. No need to call l2cap_chan_hold(chan) here again.
> > */
> > @@ -439,7 +435,7 @@ static void l2cap_chan_timeout(struct work_struct *work)
> > l2cap_chan_unlock(chan);
> > l2cap_chan_put(chan);
> >
> > - mutex_unlock(&conn->chan_lock);
> > + mutex_unlock(&conn->lock);
> > }
> >
> > struct l2cap_chan *l2cap_chan_create(void)
> > @@ -641,9 +637,9 @@ void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
> >
> > void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
> > {
> > - mutex_lock(&conn->chan_lock);
> > + mutex_lock(&conn->lock);
> > __l2cap_chan_add(conn, chan);
> > - mutex_unlock(&conn->chan_lock);
> > + mutex_unlock(&conn->lock);
> > }
> >
> > void l2cap_chan_del(struct l2cap_chan *chan, int err)
> > @@ -731,9 +727,9 @@ void l2cap_chan_list(struct l2cap_conn *conn, l2cap_chan_func_t func,
> > if (!conn)
> > return;
> >
> > - mutex_lock(&conn->chan_lock);
> > + mutex_lock(&conn->lock);
> > __l2cap_chan_list(conn, func, data);
> > - mutex_unlock(&conn->chan_lock);
> > + mutex_unlock(&conn->lock);
> > }
> >
> > EXPORT_SYMBOL_GPL(l2cap_chan_list);
> > @@ -745,7 +741,7 @@ static void l2cap_conn_update_id_addr(struct work_struct *work)
> > struct hci_conn *hcon = conn->hcon;
> > struct l2cap_chan *chan;
> >
> > - mutex_lock(&conn->chan_lock);
> > + mutex_lock(&conn->lock);
> >
> > list_for_each_entry(chan, &conn->chan_l, list) {
> > l2cap_chan_lock(chan);
> > @@ -754,7 +750,7 @@ static void l2cap_conn_update_id_addr(struct work_struct *work)
> > l2cap_chan_unlock(chan);
> > }
> >
> > - mutex_unlock(&conn->chan_lock);
> > + mutex_unlock(&conn->lock);
> > }
> >
> > static void l2cap_chan_le_connect_reject(struct l2cap_chan *chan)
> > @@ -1507,8 +1503,6 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
> >
> > BT_DBG("conn %p", conn);
> >
> > - mutex_lock(&conn->chan_lock);
> > -
> > list_for_each_entry_safe(chan, tmp, &conn->chan_l, list) {
> > l2cap_chan_lock(chan);
> >
> > @@ -1577,8 +1571,6 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
> >
> > l2cap_chan_unlock(chan);
> > }
> > -
> > - mutex_unlock(&conn->chan_lock);
> > }
> >
> > static void l2cap_le_conn_ready(struct l2cap_conn *conn)
> > @@ -1624,7 +1616,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
> > if (hcon->type == ACL_LINK)
> > l2cap_request_info(conn);
> >
> > - mutex_lock(&conn->chan_lock);
> > + mutex_lock(&conn->lock);
> >
> > list_for_each_entry(chan, &conn->chan_l, list) {
> >
> > @@ -1642,7 +1634,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
> > l2cap_chan_unlock(chan);
> > }
> >
> > - mutex_unlock(&conn->chan_lock);
> > + mutex_unlock(&conn->lock);
> >
> > if (hcon->type == LE_LINK)
> > l2cap_le_conn_ready(conn);
> > @@ -1657,14 +1649,10 @@ static void l2cap_conn_unreliable(struct l2cap_conn *conn, int err)
> >
> > BT_DBG("conn %p", conn);
> >
> > - mutex_lock(&conn->chan_lock);
> > -
> > list_for_each_entry(chan, &conn->chan_l, list) {
> > if (test_bit(FLAG_FORCE_RELIABLE, &chan->flags))
> > l2cap_chan_set_err(chan, err);
> > }
> > -
> > - mutex_unlock(&conn->chan_lock);
> > }
> >
> > static void l2cap_info_timeout(struct work_struct *work)
> > @@ -1675,7 +1663,9 @@ static void l2cap_info_timeout(struct work_struct *work)
> > conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
> > conn->info_ident = 0;
> >
> > + mutex_lock(&conn->lock);
> > l2cap_conn_start(conn);
> > + mutex_unlock(&conn->lock);
> > }
> >
> > /*
> > @@ -1767,6 +1757,8 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
> >
> > BT_DBG("hcon %p conn %p, err %d", hcon, conn, err);
> >
> > + mutex_lock(&conn->lock);
> > +
> > kfree_skb(conn->rx_skb);
> >
> > skb_queue_purge(&conn->pending_rx);
> > @@ -1785,8 +1777,6 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
> > /* Force the connection to be immediately dropped */
> > hcon->disc_timeout = 0;
> >
> > - mutex_lock(&conn->chan_lock);
> > -
> > /* Kill channels */
> > list_for_each_entry_safe(chan, l, &conn->chan_l, list) {
> > l2cap_chan_hold(chan);
> > @@ -1800,12 +1790,11 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
> > l2cap_chan_put(chan);
> > }
> >
> > - mutex_unlock(&conn->chan_lock);
> > -
> > if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT)
> > cancel_delayed_work_sync(&conn->info_timer);
> >
> > hcon->l2cap_data = NULL;
> > + mutex_unlock(&conn->lock);
> > l2cap_conn_put(conn);
> > }
> >
> > @@ -2924,8 +2913,6 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb)
> >
> > BT_DBG("conn %p", conn);
> >
> > - mutex_lock(&conn->chan_lock);
> > -
> > list_for_each_entry(chan, &conn->chan_l, list) {
> > if (chan->chan_type != L2CAP_CHAN_RAW)
> > continue;
> > @@ -2940,8 +2927,6 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb)
> > if (chan->ops->recv(chan, nskb))
> > kfree_skb(nskb);
> > }
> > -
> > - mutex_unlock(&conn->chan_lock);
> > }
> >
> > /* ---- L2CAP signalling commands ---- */
> > @@ -3960,7 +3945,6 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
> > goto response;
> > }
> >
> > - mutex_lock(&conn->chan_lock);
> > l2cap_chan_lock(pchan);
> >
> > /* Check if the ACL is secure enough (if not SDP) */
> > @@ -4067,7 +4051,6 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
> > }
> >
> > l2cap_chan_unlock(pchan);
> > - mutex_unlock(&conn->chan_lock);
> > l2cap_chan_put(pchan);
> > }
> >
> > @@ -4106,27 +4089,19 @@ static int l2cap_connect_create_rsp(struct l2cap_conn *conn,
> > BT_DBG("dcid 0x%4.4x scid 0x%4.4x result 0x%2.2x status 0x%2.2x",
> > dcid, scid, result, status);
> >
> > - mutex_lock(&conn->chan_lock);
> > -
> > if (scid) {
> > chan = __l2cap_get_chan_by_scid(conn, scid);
> > - if (!chan) {
> > - err = -EBADSLT;
> > - goto unlock;
> > - }
> > + if (!chan)
> > + return -EBADSLT;
> > } else {
> > chan = __l2cap_get_chan_by_ident(conn, cmd->ident);
> > - if (!chan) {
> > - err = -EBADSLT;
> > - goto unlock;
> > - }
> > + if (!chan)
> > + return -EBADSLT;
> > }
> >
> > chan = l2cap_chan_hold_unless_zero(chan);
> > - if (!chan) {
> > - err = -EBADSLT;
> > - goto unlock;
> > - }
> > + if (!chan)
> > + return -EBADSLT;
> >
> > err = 0;
> >
> > @@ -4164,9 +4139,6 @@ static int l2cap_connect_create_rsp(struct l2cap_conn *conn,
> > l2cap_chan_unlock(chan);
> > l2cap_chan_put(chan);
> >
> > -unlock:
> > - mutex_unlock(&conn->chan_lock);
> > -
> > return err;
> > }
> >
> > @@ -4454,11 +4426,7 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn,
> >
> > chan->ops->set_shutdown(chan);
> >
> > - l2cap_chan_unlock(chan);
> > - mutex_lock(&conn->chan_lock);
> > - l2cap_chan_lock(chan);
> > l2cap_chan_del(chan, ECONNRESET);
> > - mutex_unlock(&conn->chan_lock);
> >
> > chan->ops->close(chan);
> >
> > @@ -4495,11 +4463,7 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn,
> > return 0;
> > }
> >
> > - l2cap_chan_unlock(chan);
> > - mutex_lock(&conn->chan_lock);
> > - l2cap_chan_lock(chan);
> > l2cap_chan_del(chan, 0);
> > - mutex_unlock(&conn->chan_lock);
> >
> > chan->ops->close(chan);
> >
> > @@ -4697,13 +4661,9 @@ static int l2cap_le_connect_rsp(struct l2cap_conn *conn,
> > BT_DBG("dcid 0x%4.4x mtu %u mps %u credits %u result 0x%2.2x",
> > dcid, mtu, mps, credits, result);
> >
> > - mutex_lock(&conn->chan_lock);
> > -
> > chan = __l2cap_get_chan_by_ident(conn, cmd->ident);
> > - if (!chan) {
> > - err = -EBADSLT;
> > - goto unlock;
> > - }
> > + if (!chan)
> > + return -EBADSLT;
> >
> > err = 0;
> >
> > @@ -4751,9 +4711,6 @@ static int l2cap_le_connect_rsp(struct l2cap_conn *conn,
> >
> > l2cap_chan_unlock(chan);
> >
> > -unlock:
> > - mutex_unlock(&conn->chan_lock);
> > -
> > return err;
> > }
> >
> > @@ -4865,7 +4822,6 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
> > goto response;
> > }
> >
> > - mutex_lock(&conn->chan_lock);
> > l2cap_chan_lock(pchan);
> >
> > if (!smp_sufficient_security(conn->hcon, pchan->sec_level,
> > @@ -4931,7 +4887,6 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
> >
> > response_unlock:
> > l2cap_chan_unlock(pchan);
> > - mutex_unlock(&conn->chan_lock);
> > l2cap_chan_put(pchan);
> >
> > if (result == L2CAP_CR_PEND)
> > @@ -5065,7 +5020,6 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
> > goto response;
> > }
> >
> > - mutex_lock(&conn->chan_lock);
> > l2cap_chan_lock(pchan);
> >
> > if (!smp_sufficient_security(conn->hcon, pchan->sec_level,
> > @@ -5140,7 +5094,6 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
> >
> > unlock:
> > l2cap_chan_unlock(pchan);
> > - mutex_unlock(&conn->chan_lock);
> > l2cap_chan_put(pchan);
> >
> > response:
> > @@ -5177,8 +5130,6 @@ static inline int l2cap_ecred_conn_rsp(struct l2cap_conn *conn,
> > BT_DBG("mtu %u mps %u credits %u result 0x%4.4x", mtu, mps, credits,
> > result);
> >
> > - mutex_lock(&conn->chan_lock);
> > -
> > cmd_len -= sizeof(*rsp);
> >
> > list_for_each_entry_safe(chan, tmp, &conn->chan_l, list) {
> > @@ -5264,8 +5215,6 @@ static inline int l2cap_ecred_conn_rsp(struct l2cap_conn *conn,
> > l2cap_chan_unlock(chan);
> > }
> >
> > - mutex_unlock(&conn->chan_lock);
> > -
> > return err;
> > }
> >
> > @@ -5378,8 +5327,6 @@ static inline int l2cap_le_command_rej(struct l2cap_conn *conn,
> > if (cmd_len < sizeof(*rej))
> > return -EPROTO;
> >
> > - mutex_lock(&conn->chan_lock);
> > -
> > chan = __l2cap_get_chan_by_ident(conn, cmd->ident);
> > if (!chan)
> > goto done;
> > @@ -5394,7 +5341,6 @@ static inline int l2cap_le_command_rej(struct l2cap_conn *conn,
> > l2cap_chan_put(chan);
> >
> > done:
> > - mutex_unlock(&conn->chan_lock);
> > return 0;
> > }
> >
> > @@ -6849,8 +6795,12 @@ static void process_pending_rx(struct work_struct *work)
> >
> > BT_DBG("");
> >
> > + mutex_lock(&conn->lock);
> > +
> > while ((skb = skb_dequeue(&conn->pending_rx)))
> > l2cap_recv_frame(conn, skb);
> > +
> > + mutex_unlock(&conn->lock);
> > }
> >
> > static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
> > @@ -6889,7 +6839,7 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
> > conn->local_fixed_chan |= L2CAP_FC_SMP_BREDR;
> >
> > mutex_init(&conn->ident_lock);
> > - mutex_init(&conn->chan_lock);
> > + mutex_init(&conn->lock);
> >
> > INIT_LIST_HEAD(&conn->chan_l);
> > INIT_LIST_HEAD(&conn->users);
> > @@ -7080,7 +7030,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
> > }
> > }
> >
> > - mutex_lock(&conn->chan_lock);
> > + mutex_lock(&conn->lock);
> > l2cap_chan_lock(chan);
> >
> > if (cid && __l2cap_get_chan_by_dcid(conn, cid)) {
> > @@ -7121,7 +7071,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
> >
> > chan_unlock:
> > l2cap_chan_unlock(chan);
> > - mutex_unlock(&conn->chan_lock);
> > + mutex_unlock(&conn->lock);
> > done:
> > hci_dev_unlock(hdev);
> > hci_dev_put(hdev);
> > @@ -7333,7 +7283,7 @@ static void l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> >
> > BT_DBG("conn %p status 0x%2.2x encrypt %u", conn, status, encrypt);
> >
> > - mutex_lock(&conn->chan_lock);
> > + mutex_lock(&conn->lock);
> >
> > list_for_each_entry(chan, &conn->chan_l, list) {
> > l2cap_chan_lock(chan);
> > @@ -7407,7 +7357,7 @@ static void l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> > l2cap_chan_unlock(chan);
> > }
> >
> > - mutex_unlock(&conn->chan_lock);
> > + mutex_unlock(&conn->lock);
> > }
> >
> > /* Append fragment into frame respecting the maximum len of rx_skb */
> > @@ -7506,6 +7456,8 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
> >
> > BT_DBG("conn %p len %u flags 0x%x", conn, skb->len, flags);
> >
> > + mutex_lock(&conn->lock);
> > +
> > switch (flags) {
> > case ACL_START:
> > case ACL_START_NO_FLUSH:
> > @@ -7530,7 +7482,7 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
> > if (len == skb->len) {
> > /* Complete frame received */
> > l2cap_recv_frame(conn, skb);
> > - return;
> > + goto unlock;
> > }
> >
> > BT_DBG("Start: total len %d, frag len %u", len, skb->len);
> > @@ -7592,10 +7544,11 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
> > break;
> > }
> >
> > - l2cap_conn_put(conn);
> > -
> > drop:
> > kfree_skb(skb);
> > +unlock:
> > + mutex_unlock(&conn->lock);
> > + l2cap_conn_put(conn);
> > }
> >
> > static struct hci_cb l2cap_cb = {
> > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > index 46ea0bee2259..831ab9b97001 100644
> > --- a/net/bluetooth/l2cap_sock.c
> > +++ b/net/bluetooth/l2cap_sock.c
> > @@ -1366,14 +1366,14 @@ static int l2cap_sock_shutdown(struct socket *sock, int how)
> >
> > if (conn)
> > /* mutex lock must be taken before l2cap_chan_lock() */
> > - mutex_lock(&conn->chan_lock);
> > + mutex_lock(&conn->lock);
> >
> > l2cap_chan_lock(chan);
> > l2cap_chan_close(chan, 0);
> > l2cap_chan_unlock(chan);
> >
> > if (conn) {
> > - mutex_unlock(&conn->chan_lock);
> > + mutex_unlock(&conn->lock);
> > l2cap_conn_put(conn);
> > }
> >
> > --
> > 2.48.1
> >
>
>
> --
> Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [syzbot] Re: [PATCH v1] Bluetooth: L2CAP: Fix corrupted list in hci_chan_del
2025-02-06 13:10 [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2) syzbot
` (6 preceding siblings ...)
2025-02-07 16:13 ` [syzbot] Re: [PATCH v1] Bluetooth: L2CAP: Fix corrupted list in hci_chan_del syzbot
@ 2025-02-07 16:54 ` syzbot
7 siblings, 0 replies; 10+ messages in thread
From: syzbot @ 2025-02-07 16:54 UTC (permalink / raw)
To: linux-kernel, syzkaller-bugs
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.
***
Subject: Re: [PATCH v1] Bluetooth: L2CAP: Fix corrupted list in hci_chan_del
Author: luiz.dentz@gmail.com
#syz test
On Fri, Feb 7, 2025 at 11:12 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> #syz test
>
> On Thu, Feb 6, 2025 at 5:05 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > #syz test
> >
> > On Thu, Feb 6, 2025 at 4:01 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > >
> > > This fixes the following trace by reworking the locking of l2cap_conn
> > > so instead of only locking when changing the chan_l list this promotes
> > > chan_lock to a general lock of l2cap_conn so whenever it is being held
> > > it would prevents the likes of l2cap_conn_del to run:
> > >
> > > list_del corruption, ffff888021297e00->prev is LIST_POISON2 (dead000000000122)
> > > ------------[ cut here ]------------
> > > kernel BUG at lib/list_debug.c:61!
> > > Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN PTI
> > > CPU: 1 UID: 0 PID: 5896 Comm: syz-executor213 Not tainted 6.14.0-rc1-next-20250204-syzkaller #0
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 12/27/2024
> > > RIP: 0010:__list_del_entry_valid_or_report+0x12c/0x190 lib/list_debug.c:59
> > > Code: 8c 4c 89 fe 48 89 da e8 32 8c 37 fc 90 0f 0b 48 89 df e8 27 9f 14 fd 48 c7 c7 a0 c0 60 8c 4c 89 fe 48 89 da e8 15 8c 37 fc 90 <0f> 0b 4c 89 e7 e8 0a 9f 14 fd 42 80 3c 2b 00 74 08 4c 89 e7 e8 cb
> > > RSP: 0018:ffffc90003f6f998 EFLAGS: 00010246
> > > RAX: 000000000000004e RBX: dead000000000122 RCX: 01454d423f7fbf00
> > > RDX: 0000000000000000 RSI: 0000000080000000 RDI: 0000000000000000
> > > RBP: dffffc0000000000 R08: ffffffff819f077c R09: 1ffff920007eded0
> > > R10: dffffc0000000000 R11: fffff520007eded1 R12: dead000000000122
> > > R13: dffffc0000000000 R14: ffff8880352248d8 R15: ffff888021297e00
> > > FS: 00007f7ace6686c0(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
> > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > CR2: 00007f7aceeeb1d0 CR3: 000000003527c000 CR4: 00000000003526f0
> > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > Call Trace:
> > > <TASK>
> > > __list_del_entry_valid include/linux/list.h:124 [inline]
> > > __list_del_entry include/linux/list.h:215 [inline]
> > > list_del_rcu include/linux/rculist.h:168 [inline]
> > > hci_chan_del+0x70/0x1b0 net/bluetooth/hci_conn.c:2858
> > > l2cap_conn_free net/bluetooth/l2cap_core.c:1816 [inline]
> > > kref_put include/linux/kref.h:65 [inline]
> > > l2cap_conn_put+0x70/0xe0 net/bluetooth/l2cap_core.c:1830
> > > l2cap_sock_shutdown+0xa8a/0x1020 net/bluetooth/l2cap_sock.c:1377
> > > l2cap_sock_release+0x79/0x1d0 net/bluetooth/l2cap_sock.c:1416
> > > __sock_release net/socket.c:642 [inline]
> > > sock_close+0xbc/0x240 net/socket.c:1393
> > > __fput+0x3e9/0x9f0 fs/file_table.c:448
> > > task_work_run+0x24f/0x310 kernel/task_work.c:227
> > > ptrace_notify+0x2d2/0x380 kernel/signal.c:2522
> > > ptrace_report_syscall include/linux/ptrace.h:415 [inline]
> > > ptrace_report_syscall_exit include/linux/ptrace.h:477 [inline]
> > > syscall_exit_work+0xc7/0x1d0 kernel/entry/common.c:173
> > > syscall_exit_to_user_mode_prepare kernel/entry/common.c:200 [inline]
> > > __syscall_exit_to_user_mode_work kernel/entry/common.c:205 [inline]
> > > syscall_exit_to_user_mode+0x24a/0x340 kernel/entry/common.c:218
> > > do_syscall_64+0x100/0x230 arch/x86/entry/common.c:89
> > > entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > > RIP: 0033:0x7f7aceeaf449
> > > Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 41 19 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
> > > RSP: 002b:00007f7ace668218 EFLAGS: 00000246 ORIG_RAX: 000000000000002a
> > > RAX: fffffffffffffffc RBX: 00007f7acef39328 RCX: 00007f7aceeaf449
> > > RDX: 000000000000000e RSI: 0000000020000100 RDI: 0000000000000004
> > > RBP: 00007f7acef39320 R08: 0000000000000000 R09: 0000000000000000
> > > R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000003
> > > R13: 0000000000000004 R14: 00007f7ace668670 R15: 000000000000000b
> > > </TASK>
> > > Modules linked in:
> > > ---[ end trace 0000000000000000 ]---
> > > RIP: 0010:__list_del_entry_valid_or_report+0x12c/0x190 lib/list_debug.c:59
> > > Code: 8c 4c 89 fe 48 89 da e8 32 8c 37 fc 90 0f 0b 48 89 df e8 27 9f 14 fd 48 c7 c7 a0 c0 60 8c 4c 89 fe 48 89 da e8 15 8c 37 fc 90 <0f> 0b 4c 89 e7 e8 0a 9f 14 fd 42 80 3c 2b 00 74 08 4c 89 e7 e8 cb
> > > RSP: 0018:ffffc90003f6f998 EFLAGS: 00010246
> > > RAX: 000000000000004e RBX: dead000000000122 RCX: 01454d423f7fbf00
> > > RDX: 0000000000000000 RSI: 0000000080000000 RDI: 0000000000000000
> > > RBP: dffffc0000000000 R08: ffffffff819f077c R09: 1ffff920007eded0
> > > R10: dffffc0000000000 R11: fffff520007eded1 R12: dead000000000122
> > > R13: dffffc0000000000 R14: ffff8880352248d8 R15: ffff888021297e00
> > > FS: 00007f7ace6686c0(0000) GS:ffff8880b8600000(0000) knlGS:0000000000000000
> > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > CR2: 00007f7acef05b08 CR3: 000000003527c000 CR4: 00000000003526f0
> > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > >
> > > Reported-by: syzbot+10bd8fe6741eedd2be2e@syzkaller.appspotmail.com
> > > Fixes: 6ab54a717189 ("Bluetooth: L2CAP: Fix slab-use-after-free Read in l2cap_send_cmd")
> > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > ---
> > > include/net/bluetooth/l2cap.h | 2 +-
> > > net/bluetooth/l2cap_core.c | 123 +++++++++++-----------------------
> > > net/bluetooth/l2cap_sock.c | 4 +-
> > > 3 files changed, 41 insertions(+), 88 deletions(-)
> > >
> > > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> > > index d9c767cf773d..6b90cfa15d88 100644
> > > --- a/include/net/bluetooth/l2cap.h
> > > +++ b/include/net/bluetooth/l2cap.h
> > > @@ -668,7 +668,7 @@ struct l2cap_conn {
> > > struct l2cap_chan *smp;
> > >
> > > struct list_head chan_l;
> > > - struct mutex chan_lock;
> > > + struct mutex lock;
> > > struct kref ref;
> > > struct list_head users;
> > > };
> > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > > index adb8c33ac595..5eedd6611f6c 100644
> > > --- a/net/bluetooth/l2cap_core.c
> > > +++ b/net/bluetooth/l2cap_core.c
> > > @@ -119,7 +119,6 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn,
> > > {
> > > struct l2cap_chan *c;
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > c = __l2cap_get_chan_by_scid(conn, cid);
> > > if (c) {
> > > /* Only lock if chan reference is not 0 */
> > > @@ -127,7 +126,6 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn,
> > > if (c)
> > > l2cap_chan_lock(c);
> > > }
> > > - mutex_unlock(&conn->chan_lock);
> > >
> > > return c;
> > > }
> > > @@ -140,7 +138,6 @@ static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
> > > {
> > > struct l2cap_chan *c;
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > c = __l2cap_get_chan_by_dcid(conn, cid);
> > > if (c) {
> > > /* Only lock if chan reference is not 0 */
> > > @@ -148,7 +145,6 @@ static struct l2cap_chan *l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
> > > if (c)
> > > l2cap_chan_lock(c);
> > > }
> > > - mutex_unlock(&conn->chan_lock);
> > >
> > > return c;
> > > }
> > > @@ -418,7 +414,7 @@ static void l2cap_chan_timeout(struct work_struct *work)
> > > if (!conn)
> > > return;
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > + mutex_lock(&conn->lock);
> > > /* __set_chan_timer() calls l2cap_chan_hold(chan) while scheduling
> > > * this work. No need to call l2cap_chan_hold(chan) here again.
> > > */
> > > @@ -439,7 +435,7 @@ static void l2cap_chan_timeout(struct work_struct *work)
> > > l2cap_chan_unlock(chan);
> > > l2cap_chan_put(chan);
> > >
> > > - mutex_unlock(&conn->chan_lock);
> > > + mutex_unlock(&conn->lock);
> > > }
> > >
> > > struct l2cap_chan *l2cap_chan_create(void)
> > > @@ -641,9 +637,9 @@ void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
> > >
> > > void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
> > > {
> > > - mutex_lock(&conn->chan_lock);
> > > + mutex_lock(&conn->lock);
> > > __l2cap_chan_add(conn, chan);
> > > - mutex_unlock(&conn->chan_lock);
> > > + mutex_unlock(&conn->lock);
> > > }
> > >
> > > void l2cap_chan_del(struct l2cap_chan *chan, int err)
> > > @@ -731,9 +727,9 @@ void l2cap_chan_list(struct l2cap_conn *conn, l2cap_chan_func_t func,
> > > if (!conn)
> > > return;
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > + mutex_lock(&conn->lock);
> > > __l2cap_chan_list(conn, func, data);
> > > - mutex_unlock(&conn->chan_lock);
> > > + mutex_unlock(&conn->lock);
> > > }
> > >
> > > EXPORT_SYMBOL_GPL(l2cap_chan_list);
> > > @@ -745,7 +741,7 @@ static void l2cap_conn_update_id_addr(struct work_struct *work)
> > > struct hci_conn *hcon = conn->hcon;
> > > struct l2cap_chan *chan;
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > + mutex_lock(&conn->lock);
> > >
> > > list_for_each_entry(chan, &conn->chan_l, list) {
> > > l2cap_chan_lock(chan);
> > > @@ -754,7 +750,7 @@ static void l2cap_conn_update_id_addr(struct work_struct *work)
> > > l2cap_chan_unlock(chan);
> > > }
> > >
> > > - mutex_unlock(&conn->chan_lock);
> > > + mutex_unlock(&conn->lock);
> > > }
> > >
> > > static void l2cap_chan_le_connect_reject(struct l2cap_chan *chan)
> > > @@ -1507,8 +1503,6 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
> > >
> > > BT_DBG("conn %p", conn);
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > -
> > > list_for_each_entry_safe(chan, tmp, &conn->chan_l, list) {
> > > l2cap_chan_lock(chan);
> > >
> > > @@ -1577,8 +1571,6 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
> > >
> > > l2cap_chan_unlock(chan);
> > > }
> > > -
> > > - mutex_unlock(&conn->chan_lock);
> > > }
> > >
> > > static void l2cap_le_conn_ready(struct l2cap_conn *conn)
> > > @@ -1624,7 +1616,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
> > > if (hcon->type == ACL_LINK)
> > > l2cap_request_info(conn);
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > + mutex_lock(&conn->lock);
> > >
> > > list_for_each_entry(chan, &conn->chan_l, list) {
> > >
> > > @@ -1642,7 +1634,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
> > > l2cap_chan_unlock(chan);
> > > }
> > >
> > > - mutex_unlock(&conn->chan_lock);
> > > + mutex_unlock(&conn->lock);
> > >
> > > if (hcon->type == LE_LINK)
> > > l2cap_le_conn_ready(conn);
> > > @@ -1657,14 +1649,10 @@ static void l2cap_conn_unreliable(struct l2cap_conn *conn, int err)
> > >
> > > BT_DBG("conn %p", conn);
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > -
> > > list_for_each_entry(chan, &conn->chan_l, list) {
> > > if (test_bit(FLAG_FORCE_RELIABLE, &chan->flags))
> > > l2cap_chan_set_err(chan, err);
> > > }
> > > -
> > > - mutex_unlock(&conn->chan_lock);
> > > }
> > >
> > > static void l2cap_info_timeout(struct work_struct *work)
> > > @@ -1675,7 +1663,9 @@ static void l2cap_info_timeout(struct work_struct *work)
> > > conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
> > > conn->info_ident = 0;
> > >
> > > + mutex_lock(&conn->lock);
> > > l2cap_conn_start(conn);
> > > + mutex_unlock(&conn->lock);
> > > }
> > >
> > > /*
> > > @@ -1767,6 +1757,8 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
> > >
> > > BT_DBG("hcon %p conn %p, err %d", hcon, conn, err);
> > >
> > > + mutex_lock(&conn->lock);
> > > +
> > > kfree_skb(conn->rx_skb);
> > >
> > > skb_queue_purge(&conn->pending_rx);
> > > @@ -1785,8 +1777,6 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
> > > /* Force the connection to be immediately dropped */
> > > hcon->disc_timeout = 0;
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > -
> > > /* Kill channels */
> > > list_for_each_entry_safe(chan, l, &conn->chan_l, list) {
> > > l2cap_chan_hold(chan);
> > > @@ -1800,12 +1790,11 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
> > > l2cap_chan_put(chan);
> > > }
> > >
> > > - mutex_unlock(&conn->chan_lock);
> > > -
> > > if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT)
> > > cancel_delayed_work_sync(&conn->info_timer);
> > >
> > > hcon->l2cap_data = NULL;
> > > + mutex_unlock(&conn->lock);
> > > l2cap_conn_put(conn);
> > > }
> > >
> > > @@ -2924,8 +2913,6 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb)
> > >
> > > BT_DBG("conn %p", conn);
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > -
> > > list_for_each_entry(chan, &conn->chan_l, list) {
> > > if (chan->chan_type != L2CAP_CHAN_RAW)
> > > continue;
> > > @@ -2940,8 +2927,6 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb)
> > > if (chan->ops->recv(chan, nskb))
> > > kfree_skb(nskb);
> > > }
> > > -
> > > - mutex_unlock(&conn->chan_lock);
> > > }
> > >
> > > /* ---- L2CAP signalling commands ---- */
> > > @@ -3960,7 +3945,6 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
> > > goto response;
> > > }
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > l2cap_chan_lock(pchan);
> > >
> > > /* Check if the ACL is secure enough (if not SDP) */
> > > @@ -4067,7 +4051,6 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
> > > }
> > >
> > > l2cap_chan_unlock(pchan);
> > > - mutex_unlock(&conn->chan_lock);
> > > l2cap_chan_put(pchan);
> > > }
> > >
> > > @@ -4106,27 +4089,19 @@ static int l2cap_connect_create_rsp(struct l2cap_conn *conn,
> > > BT_DBG("dcid 0x%4.4x scid 0x%4.4x result 0x%2.2x status 0x%2.2x",
> > > dcid, scid, result, status);
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > -
> > > if (scid) {
> > > chan = __l2cap_get_chan_by_scid(conn, scid);
> > > - if (!chan) {
> > > - err = -EBADSLT;
> > > - goto unlock;
> > > - }
> > > + if (!chan)
> > > + return -EBADSLT;
> > > } else {
> > > chan = __l2cap_get_chan_by_ident(conn, cmd->ident);
> > > - if (!chan) {
> > > - err = -EBADSLT;
> > > - goto unlock;
> > > - }
> > > + if (!chan)
> > > + return -EBADSLT;
> > > }
> > >
> > > chan = l2cap_chan_hold_unless_zero(chan);
> > > - if (!chan) {
> > > - err = -EBADSLT;
> > > - goto unlock;
> > > - }
> > > + if (!chan)
> > > + return -EBADSLT;
> > >
> > > err = 0;
> > >
> > > @@ -4164,9 +4139,6 @@ static int l2cap_connect_create_rsp(struct l2cap_conn *conn,
> > > l2cap_chan_unlock(chan);
> > > l2cap_chan_put(chan);
> > >
> > > -unlock:
> > > - mutex_unlock(&conn->chan_lock);
> > > -
> > > return err;
> > > }
> > >
> > > @@ -4454,11 +4426,7 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn,
> > >
> > > chan->ops->set_shutdown(chan);
> > >
> > > - l2cap_chan_unlock(chan);
> > > - mutex_lock(&conn->chan_lock);
> > > - l2cap_chan_lock(chan);
> > > l2cap_chan_del(chan, ECONNRESET);
> > > - mutex_unlock(&conn->chan_lock);
> > >
> > > chan->ops->close(chan);
> > >
> > > @@ -4495,11 +4463,7 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn,
> > > return 0;
> > > }
> > >
> > > - l2cap_chan_unlock(chan);
> > > - mutex_lock(&conn->chan_lock);
> > > - l2cap_chan_lock(chan);
> > > l2cap_chan_del(chan, 0);
> > > - mutex_unlock(&conn->chan_lock);
> > >
> > > chan->ops->close(chan);
> > >
> > > @@ -4697,13 +4661,9 @@ static int l2cap_le_connect_rsp(struct l2cap_conn *conn,
> > > BT_DBG("dcid 0x%4.4x mtu %u mps %u credits %u result 0x%2.2x",
> > > dcid, mtu, mps, credits, result);
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > -
> > > chan = __l2cap_get_chan_by_ident(conn, cmd->ident);
> > > - if (!chan) {
> > > - err = -EBADSLT;
> > > - goto unlock;
> > > - }
> > > + if (!chan)
> > > + return -EBADSLT;
> > >
> > > err = 0;
> > >
> > > @@ -4751,9 +4711,6 @@ static int l2cap_le_connect_rsp(struct l2cap_conn *conn,
> > >
> > > l2cap_chan_unlock(chan);
> > >
> > > -unlock:
> > > - mutex_unlock(&conn->chan_lock);
> > > -
> > > return err;
> > > }
> > >
> > > @@ -4865,7 +4822,6 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
> > > goto response;
> > > }
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > l2cap_chan_lock(pchan);
> > >
> > > if (!smp_sufficient_security(conn->hcon, pchan->sec_level,
> > > @@ -4931,7 +4887,6 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn,
> > >
> > > response_unlock:
> > > l2cap_chan_unlock(pchan);
> > > - mutex_unlock(&conn->chan_lock);
> > > l2cap_chan_put(pchan);
> > >
> > > if (result == L2CAP_CR_PEND)
> > > @@ -5065,7 +5020,6 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
> > > goto response;
> > > }
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > l2cap_chan_lock(pchan);
> > >
> > > if (!smp_sufficient_security(conn->hcon, pchan->sec_level,
> > > @@ -5140,7 +5094,6 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
> > >
> > > unlock:
> > > l2cap_chan_unlock(pchan);
> > > - mutex_unlock(&conn->chan_lock);
> > > l2cap_chan_put(pchan);
> > >
> > > response:
> > > @@ -5177,8 +5130,6 @@ static inline int l2cap_ecred_conn_rsp(struct l2cap_conn *conn,
> > > BT_DBG("mtu %u mps %u credits %u result 0x%4.4x", mtu, mps, credits,
> > > result);
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > -
> > > cmd_len -= sizeof(*rsp);
> > >
> > > list_for_each_entry_safe(chan, tmp, &conn->chan_l, list) {
> > > @@ -5264,8 +5215,6 @@ static inline int l2cap_ecred_conn_rsp(struct l2cap_conn *conn,
> > > l2cap_chan_unlock(chan);
> > > }
> > >
> > > - mutex_unlock(&conn->chan_lock);
> > > -
> > > return err;
> > > }
> > >
> > > @@ -5378,8 +5327,6 @@ static inline int l2cap_le_command_rej(struct l2cap_conn *conn,
> > > if (cmd_len < sizeof(*rej))
> > > return -EPROTO;
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > -
> > > chan = __l2cap_get_chan_by_ident(conn, cmd->ident);
> > > if (!chan)
> > > goto done;
> > > @@ -5394,7 +5341,6 @@ static inline int l2cap_le_command_rej(struct l2cap_conn *conn,
> > > l2cap_chan_put(chan);
> > >
> > > done:
> > > - mutex_unlock(&conn->chan_lock);
> > > return 0;
> > > }
> > >
> > > @@ -6849,8 +6795,12 @@ static void process_pending_rx(struct work_struct *work)
> > >
> > > BT_DBG("");
> > >
> > > + mutex_lock(&conn->lock);
> > > +
> > > while ((skb = skb_dequeue(&conn->pending_rx)))
> > > l2cap_recv_frame(conn, skb);
> > > +
> > > + mutex_unlock(&conn->lock);
> > > }
> > >
> > > static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
> > > @@ -6889,7 +6839,7 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
> > > conn->local_fixed_chan |= L2CAP_FC_SMP_BREDR;
> > >
> > > mutex_init(&conn->ident_lock);
> > > - mutex_init(&conn->chan_lock);
> > > + mutex_init(&conn->lock);
> > >
> > > INIT_LIST_HEAD(&conn->chan_l);
> > > INIT_LIST_HEAD(&conn->users);
> > > @@ -7080,7 +7030,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
> > > }
> > > }
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > + mutex_lock(&conn->lock);
> > > l2cap_chan_lock(chan);
> > >
> > > if (cid && __l2cap_get_chan_by_dcid(conn, cid)) {
> > > @@ -7121,7 +7071,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
> > >
> > > chan_unlock:
> > > l2cap_chan_unlock(chan);
> > > - mutex_unlock(&conn->chan_lock);
> > > + mutex_unlock(&conn->lock);
> > > done:
> > > hci_dev_unlock(hdev);
> > > hci_dev_put(hdev);
> > > @@ -7333,7 +7283,7 @@ static void l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> > >
> > > BT_DBG("conn %p status 0x%2.2x encrypt %u", conn, status, encrypt);
> > >
> > > - mutex_lock(&conn->chan_lock);
> > > + mutex_lock(&conn->lock);
> > >
> > > list_for_each_entry(chan, &conn->chan_l, list) {
> > > l2cap_chan_lock(chan);
> > > @@ -7407,7 +7357,7 @@ static void l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> > > l2cap_chan_unlock(chan);
> > > }
> > >
> > > - mutex_unlock(&conn->chan_lock);
> > > + mutex_unlock(&conn->lock);
> > > }
> > >
> > > /* Append fragment into frame respecting the maximum len of rx_skb */
> > > @@ -7506,6 +7456,8 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
> > >
> > > BT_DBG("conn %p len %u flags 0x%x", conn, skb->len, flags);
> > >
> > > + mutex_lock(&conn->lock);
> > > +
> > > switch (flags) {
> > > case ACL_START:
> > > case ACL_START_NO_FLUSH:
> > > @@ -7530,7 +7482,7 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
> > > if (len == skb->len) {
> > > /* Complete frame received */
> > > l2cap_recv_frame(conn, skb);
> > > - return;
> > > + goto unlock;
> > > }
> > >
> > > BT_DBG("Start: total len %d, frag len %u", len, skb->len);
> > > @@ -7592,10 +7544,11 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
> > > break;
> > > }
> > >
> > > - l2cap_conn_put(conn);
> > > -
> > > drop:
> > > kfree_skb(skb);
> > > +unlock:
> > > + mutex_unlock(&conn->lock);
> > > + l2cap_conn_put(conn);
> > > }
> > >
> > > static struct hci_cb l2cap_cb = {
> > > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > > index 46ea0bee2259..831ab9b97001 100644
> > > --- a/net/bluetooth/l2cap_sock.c
> > > +++ b/net/bluetooth/l2cap_sock.c
> > > @@ -1366,14 +1366,14 @@ static int l2cap_sock_shutdown(struct socket *sock, int how)
> > >
> > > if (conn)
> > > /* mutex lock must be taken before l2cap_chan_lock() */
> > > - mutex_lock(&conn->chan_lock);
> > > + mutex_lock(&conn->lock);
> > >
> > > l2cap_chan_lock(chan);
> > > l2cap_chan_close(chan, 0);
> > > l2cap_chan_unlock(chan);
> > >
> > > if (conn) {
> > > - mutex_unlock(&conn->chan_lock);
> > > + mutex_unlock(&conn->lock);
> > > l2cap_conn_put(conn);
> > > }
> > >
> > > --
> > > 2.48.1
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH next] Bluetooth: l2cap: protect conn refcnt under hci dev lock
2025-02-07 14:02 ` [PATCH next] Bluetooth: l2cap: protect conn refcnt under hci dev lock Lizhi Xu
@ 2025-02-08 8:20 ` kernel test robot
0 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2025-02-08 8:20 UTC (permalink / raw)
To: Lizhi Xu, syzbot+10bd8fe6741eedd2be2e
Cc: oe-kbuild-all, eadavis, johan.hedberg, linux-bluetooth,
linux-kernel, luiz.dentz, luiz.von.dentz, marcel, syzkaller-bugs
Hi Lizhi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on next-20250207]
url: https://github.com/intel-lab-lkp/linux/commits/Lizhi-Xu/Bluetooth-l2cap-protect-conn-refcnt-under-hci-dev-lock/20250207-221142
base: next-20250207
patch link: https://lore.kernel.org/r/20250207140216.3076952-1-lizhi.xu%40windriver.com
patch subject: [PATCH next] Bluetooth: l2cap: protect conn refcnt under hci dev lock
config: i386-buildonly-randconfig-001-20250208 (https://download.01.org/0day-ci/archive/20250208/202502081633.z7O9VFNy-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250208/202502081633.z7O9VFNy-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502081633.z7O9VFNy-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> net/bluetooth/l2cap_core.c:7477:27: warning: 'l2cap_conn_hold_unless_zero' defined but not used [-Wunused-function]
7477 | static struct l2cap_conn *l2cap_conn_hold_unless_zero(struct l2cap_conn *c)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/l2cap_conn_hold_unless_zero +7477 net/bluetooth/l2cap_core.c
4d7ea8ee90e42f Luiz Augusto von Dentz 2021-01-13 7476
6ab54a71718943 Luiz Augusto von Dentz 2025-01-16 @7477 static struct l2cap_conn *l2cap_conn_hold_unless_zero(struct l2cap_conn *c)
6ab54a71718943 Luiz Augusto von Dentz 2025-01-16 7478 {
6ab54a71718943 Luiz Augusto von Dentz 2025-01-16 7479 BT_DBG("conn %p orig refcnt %u", c, kref_read(&c->ref));
6ab54a71718943 Luiz Augusto von Dentz 2025-01-16 7480
6ab54a71718943 Luiz Augusto von Dentz 2025-01-16 7481 if (!kref_get_unless_zero(&c->ref))
6ab54a71718943 Luiz Augusto von Dentz 2025-01-16 7482 return NULL;
6ab54a71718943 Luiz Augusto von Dentz 2025-01-16 7483
6ab54a71718943 Luiz Augusto von Dentz 2025-01-16 7484 return c;
6ab54a71718943 Luiz Augusto von Dentz 2025-01-16 7485 }
6ab54a71718943 Luiz Augusto von Dentz 2025-01-16 7486
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-02-08 8:20 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-06 13:10 [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2) syzbot
2025-02-06 22:05 ` [syzbot] Re: [PATCH v1] Bluetooth: L2CAP: Fix corrupted list in hci_chan_del syzbot
2025-02-07 2:20 ` [syzbot] Re: [syzbot] [bluetooth?] BUG: corrupted list in hci_chan_del (2) syzbot
2025-02-07 4:09 ` syzbot
2025-02-07 6:37 ` syzbot
2025-02-07 12:25 ` syzbot
2025-02-07 14:02 ` [PATCH next] Bluetooth: l2cap: protect conn refcnt under hci dev lock Lizhi Xu
2025-02-08 8:20 ` kernel test robot
2025-02-07 16:13 ` [syzbot] Re: [PATCH v1] Bluetooth: L2CAP: Fix corrupted list in hci_chan_del syzbot
2025-02-07 16:54 ` syzbot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®