* Re: [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb
2024-06-07 6:02 [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb syzbot
@ 2024-06-11 1:47 ` Edward Adam Davis
2024-06-11 2:15 ` syzbot
2024-06-11 2:28 ` Edward Adam Davis
` (9 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Edward Adam Davis @ 2024-06-11 1:47 UTC (permalink / raw)
To: syzbot+b7f6f8c9303466e16c8a; +Cc: linux-kernel, syzkaller-bugs
please test null ptr defref in l2cap_sock_recv_cb
#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git cc8ed4d0a848
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 6db60946c627..68b57ef01c7d 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1506,7 +1506,9 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
err = __sock_queue_rcv_skb(sk, skb);
+ sock_hold(sk);
l2cap_publish_rx_avail(chan);
+ sock_put(sk);
/* For ERTM and LE, handle a skb that doesn't fit into the recv
* buffer. This is important to do because the data frames
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb
2024-06-11 1:47 ` Edward Adam Davis
@ 2024-06-11 2:15 ` syzbot
0 siblings, 0 replies; 33+ messages in thread
From: syzbot @ 2024-06-11 2:15 UTC (permalink / raw)
To: eadavis, linux-kernel, syzkaller-bugs
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue:
WARNING: refcount bug in l2cap_sock_recv_cb
------------[ cut here ]------------
refcount_t: addition on 0; use-after-free.
WARNING: CPU: 1 PID: 4478 at lib/refcount.c:25 refcount_warn_saturate+0x13a/0x1d0 lib/refcount.c:25
Modules linked in:
CPU: 1 PID: 4478 Comm: kworker/u9:1 Not tainted 6.10.0-rc1-syzkaller-00267-gcc8ed4d0a848-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/02/2024
Workqueue: hci0 hci_rx_work
RIP: 0010:refcount_warn_saturate+0x13a/0x1d0 lib/refcount.c:25
Code: c0 cb 1e 8c e8 c7 ae b1 fc 90 0f 0b 90 90 eb b9 e8 1b 80 ef fc c6 05 9a 04 f0 0a 01 90 48 c7 c7 20 cc 1e 8c e8 a7 ae b1 fc 90 <0f> 0b 90 90 eb 99 e8 fb 7f ef fc c6 05 7b 04 f0 0a 01 90 48 c7 c7
RSP: 0018:ffffc9000d40f440 EFLAGS: 00010246
RAX: 8178418b52383400 RBX: ffff88807498d080 RCX: ffff88802fa51e00
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000000
RBP: 0000000000000002 R08: ffffffff815846c2 R09: 1ffff110172a519a
R10: dffffc0000000000 R11: ffffed10172a519b R12: ffff88807498e4a0
R13: ffff88807498e000 R14: dffffc0000000000 R15: 1ffff1100e931c94
FS: 0000000000000000(0000) GS:ffff8880b9500000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fbe77aafffc CR3: 000000000e132000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
__refcount_inc include/linux/refcount.h:241 [inline]
refcount_inc include/linux/refcount.h:258 [inline]
sock_hold include/net/sock.h:774 [inline]
l2cap_sock_recv_cb+0x53d/0x600 net/bluetooth/l2cap_sock.c:1509
l2cap_conless_channel net/bluetooth/l2cap_core.c:6780 [inline]
l2cap_recv_frame+0x8b6d/0x10670 net/bluetooth/l2cap_core.c:6833
hci_acldata_packet net/bluetooth/hci_core.c:3842 [inline]
hci_rx_work+0x50f/0xca0 net/bluetooth/hci_core.c:4079
process_one_work kernel/workqueue.c:3231 [inline]
process_scheduled_works+0xa2c/0x1830 kernel/workqueue.c:3312
worker_thread+0x86d/0xd70 kernel/workqueue.c:3393
kthread+0x2f0/0x390 kernel/kthread.c:389
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
Tested on:
commit: cc8ed4d0 Merge tag 'drm-fixes-2024-06-01' of https://g..
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=149a3bf6980000
kernel config: https://syzkaller.appspot.com/x/.config?x=47d282ddffae809f
dashboard link: https://syzkaller.appspot.com/bug?extid=b7f6f8c9303466e16c8a
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=106af454980000
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb
2024-06-07 6:02 [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb syzbot
2024-06-11 1:47 ` Edward Adam Davis
@ 2024-06-11 2:28 ` Edward Adam Davis
2024-06-11 2:47 ` syzbot
2024-06-11 3:00 ` Edward Adam Davis
` (8 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Edward Adam Davis @ 2024-06-11 2:28 UTC (permalink / raw)
To: syzbot+b7f6f8c9303466e16c8a; +Cc: linux-kernel, syzkaller-bugs
please test null ptr defref in l2cap_sock_recv_cb
#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git cc8ed4d0a848
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 6db60946c627..6f01920586e6 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1486,6 +1486,8 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
int err;
lock_sock(sk);
+ l2cap_chan_hold(chan);
+ l2cap_chan_lock(chan);
if (chan->mode == L2CAP_MODE_ERTM && !list_empty(&pi->rx_busy)) {
err = -ENOMEM;
@@ -1534,6 +1536,8 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
}
done:
+ l2cap_chan_unlock(chan);
+ l2cap_chan_put(chan);
release_sock(sk);
return err;
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb
2024-06-11 2:28 ` Edward Adam Davis
@ 2024-06-11 2:47 ` syzbot
0 siblings, 0 replies; 33+ messages in thread
From: syzbot @ 2024-06-11 2:47 UTC (permalink / raw)
To: eadavis, linux-kernel, syzkaller-bugs
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue:
KASAN: slab-use-after-free Read in smack_socket_sock_rcv_skb
==================================================================
BUG: KASAN: slab-use-after-free in smack_socket_sock_rcv_skb+0xec/0x13a0 security/smack/smack_lsm.c:4142
Read of size 8 at addr ffff88807ae86498 by task kworker/u9:2/5148
CPU: 1 PID: 5148 Comm: kworker/u9:2 Not tainted 6.10.0-rc1-syzkaller-00267-gcc8ed4d0a848-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/02/2024
Workqueue: hci0 hci_rx_work
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
print_address_description mm/kasan/report.c:377 [inline]
print_report+0x169/0x550 mm/kasan/report.c:488
kasan_report+0x143/0x180 mm/kasan/report.c:601
smack_socket_sock_rcv_skb+0xec/0x13a0 security/smack/smack_lsm.c:4142
security_sock_rcv_skb+0x6d/0xa0 security/security.c:4603
sk_filter_trim_cap+0x184/0xa80 net/core/filter.c:150
sk_filter include/linux/filter.h:945 [inline]
l2cap_sock_recv_cb+0x176/0x580 net/bluetooth/l2cap_sock.c:1504
l2cap_conless_channel net/bluetooth/l2cap_core.c:6780 [inline]
l2cap_recv_frame+0x8b6d/0x10670 net/bluetooth/l2cap_core.c:6833
hci_acldata_packet net/bluetooth/hci_core.c:3842 [inline]
hci_rx_work+0x50f/0xca0 net/bluetooth/hci_core.c:4079
process_one_work kernel/workqueue.c:3231 [inline]
process_scheduled_works+0xa2c/0x1830 kernel/workqueue.c:3312
worker_thread+0x86d/0xd70 kernel/workqueue.c:3393
kthread+0x2f0/0x390 kernel/kthread.c:389
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
Allocated by task 5966:
kasan_save_stack mm/kasan/common.c:47 [inline]
kasan_save_track+0x3f/0x80 mm/kasan/common.c:68
poison_kmalloc_redzone mm/kasan/common.c:370 [inline]
__kasan_kmalloc+0x98/0xb0 mm/kasan/common.c:387
kasan_kmalloc include/linux/kasan.h:211 [inline]
__do_kmalloc_node mm/slub.c:4121 [inline]
__kmalloc_noprof+0x1f9/0x400 mm/slub.c:4134
kmalloc_noprof include/linux/slab.h:664 [inline]
sk_prot_alloc+0xe0/0x210 net/core/sock.c:2080
sk_alloc+0x38/0x370 net/core/sock.c:2133
bt_sock_alloc+0x3c/0x340 net/bluetooth/af_bluetooth.c:148
l2cap_sock_alloc net/bluetooth/l2cap_sock.c:1873 [inline]
l2cap_sock_create+0x13f/0x2d0 net/bluetooth/l2cap_sock.c:1913
bt_sock_create+0x161/0x230 net/bluetooth/af_bluetooth.c:132
__sock_create+0x490/0x920 net/socket.c:1571
sock_create net/socket.c:1622 [inline]
__sys_socket_create net/socket.c:1659 [inline]
__sys_socket+0x150/0x3c0 net/socket.c:1706
__do_sys_socket net/socket.c:1720 [inline]
__se_sys_socket net/socket.c:1718 [inline]
__x64_sys_socket+0x7a/0x90 net/socket.c:1718
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Freed by task 5965:
kasan_save_stack mm/kasan/common.c:47 [inline]
kasan_save_track+0x3f/0x80 mm/kasan/common.c:68
kasan_save_free_info+0x40/0x50 mm/kasan/generic.c:579
poison_slab_object+0xe0/0x150 mm/kasan/common.c:240
__kasan_slab_free+0x37/0x60 mm/kasan/common.c:256
kasan_slab_free include/linux/kasan.h:184 [inline]
slab_free_hook mm/slub.c:2195 [inline]
slab_free mm/slub.c:4436 [inline]
kfree+0x149/0x360 mm/slub.c:4557
sk_prot_free net/core/sock.c:2116 [inline]
__sk_destruct+0x476/0x5f0 net/core/sock.c:2208
l2cap_sock_release+0x15b/0x1d0 net/bluetooth/l2cap_sock.c:1417
__sock_release net/socket.c:659 [inline]
sock_close+0xbc/0x240 net/socket.c:1421
__fput+0x406/0x8b0 fs/file_table.c:422
__do_sys_close fs/open.c:1555 [inline]
__se_sys_close fs/open.c:1540 [inline]
__x64_sys_close+0x7f/0x110 fs/open.c:1540
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
The buggy address belongs to the object at ffff88807ae86000
which belongs to the cache kmalloc-2k of size 2048
The buggy address is located 1176 bytes inside of
freed 2048-byte region [ffff88807ae86000, ffff88807ae86800)
The buggy address belongs to the physical page:
page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x7ae80
head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
flags: 0xfff00000000040(head|node=0|zone=1|lastcpupid=0x7ff)
page_type: 0xffffefff(slab)
raw: 00fff00000000040 ffff888015042000 dead000000000122 0000000000000000
raw: 0000000000000000 0000000080080008 00000001ffffefff 0000000000000000
head: 00fff00000000040 ffff888015042000 dead000000000122 0000000000000000
head: 0000000000000000 0000000080080008 00000001ffffefff 0000000000000000
head: 00fff00000000003 ffffea0001eba001 ffffffffffffffff 0000000000000000
head: 0000000000000008 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 3, migratetype Unmovable, gfp_mask 0x1d20c0(__GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC|__GFP_HARDWALL), pid 5658, tgid 5658 (syz-executor), ts 103136223027, free_ts 103057739028
set_page_owner include/linux/page_owner.h:32 [inline]
post_alloc_hook+0x1f3/0x230 mm/page_alloc.c:1468
prep_new_page mm/page_alloc.c:1476 [inline]
get_page_from_freelist+0x2e2d/0x2ee0 mm/page_alloc.c:3402
__alloc_pages_noprof+0x256/0x6c0 mm/page_alloc.c:4660
__alloc_pages_node_noprof include/linux/gfp.h:269 [inline]
alloc_pages_node_noprof include/linux/gfp.h:296 [inline]
alloc_slab_page+0x5f/0x120 mm/slub.c:2264
allocate_slab+0x5a/0x2e0 mm/slub.c:2427
new_slab mm/slub.c:2480 [inline]
___slab_alloc+0xcd1/0x14b0 mm/slub.c:3666
__slab_alloc+0x58/0xa0 mm/slub.c:3756
__slab_alloc_node mm/slub.c:3809 [inline]
slab_alloc_node mm/slub.c:3988 [inline]
__do_kmalloc_node mm/slub.c:4120 [inline]
__kmalloc_noprof+0x257/0x400 mm/slub.c:4134
kmalloc_noprof include/linux/slab.h:664 [inline]
kzalloc_noprof include/linux/slab.h:778 [inline]
ip6t_alloc_initial_table+0x71/0x640 net/ipv6/netfilter/ip6_tables.c:40
ip6table_mangle_table_init+0x1c/0x70 net/ipv6/netfilter/ip6table_mangle.c:82
xt_find_table_lock+0x2d4/0x3b0 net/netfilter/x_tables.c:1260
xt_request_find_table_lock+0x26/0x100 net/netfilter/x_tables.c:1285
get_info net/ipv6/netfilter/ip6_tables.c:979 [inline]
do_ip6t_get_ctl+0x89e/0x1820 net/ipv6/netfilter/ip6_tables.c:1668
nf_getsockopt+0x299/0x2c0 net/netfilter/nf_sockopt.c:116
ipv6_getsockopt+0x263/0x380 net/ipv6/ipv6_sockglue.c:1494
tcp_getsockopt+0x163/0x1c0 net/ipv4/tcp.c:4399
page last free pid 4534 tgid 4534 stack trace:
reset_page_owner include/linux/page_owner.h:25 [inline]
free_pages_prepare mm/page_alloc.c:1088 [inline]
free_unref_page+0xd19/0xea0 mm/page_alloc.c:2565
__slab_free+0x31b/0x3d0 mm/slub.c:4347
qlink_free mm/kasan/quarantine.c:163 [inline]
qlist_free_all+0x9e/0x140 mm/kasan/quarantine.c:179
kasan_quarantine_reduce+0x14f/0x170 mm/kasan/quarantine.c:286
__kasan_slab_alloc+0x23/0x80 mm/kasan/common.c:322
kasan_slab_alloc include/linux/kasan.h:201 [inline]
slab_post_alloc_hook mm/slub.c:3940 [inline]
slab_alloc_node mm/slub.c:4000 [inline]
kmem_cache_alloc_noprof+0x135/0x2a0 mm/slub.c:4007
getname_flags+0xbd/0x4f0 fs/namei.c:139
do_sys_openat2+0xd2/0x1d0 fs/open.c:1399
do_sys_open fs/open.c:1420 [inline]
__do_sys_openat fs/open.c:1436 [inline]
__se_sys_openat fs/open.c:1431 [inline]
__x64_sys_openat+0x247/0x2a0 fs/open.c:1431
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Memory state around the buggy address:
ffff88807ae86380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88807ae86400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff88807ae86480: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88807ae86500: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88807ae86580: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
Tested on:
commit: cc8ed4d0 Merge tag 'drm-fixes-2024-06-01' of https://g..
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=147f0d02980000
kernel config: https://syzkaller.appspot.com/x/.config?x=47d282ddffae809f
dashboard link: https://syzkaller.appspot.com/bug?extid=b7f6f8c9303466e16c8a
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=11e10922980000
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb
2024-06-07 6:02 [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb syzbot
2024-06-11 1:47 ` Edward Adam Davis
2024-06-11 2:28 ` Edward Adam Davis
@ 2024-06-11 3:00 ` Edward Adam Davis
2024-06-11 3:43 ` syzbot
2024-06-11 4:09 ` Edward Adam Davis
` (7 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Edward Adam Davis @ 2024-06-11 3:00 UTC (permalink / raw)
To: syzbot+b7f6f8c9303466e16c8a; +Cc: linux-kernel, syzkaller-bugs
please test null ptr defref in l2cap_sock_recv_cb
#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git cc8ed4d0a848
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 6db60946c627..c0072f81e81a 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1486,6 +1486,14 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
int err;
lock_sock(sk);
+ sock_hold(sk);
+ l2cap_chan_hold(chan);
+ l2cap_chan_lock(chan);
+ if (sock_flag(sk, SOCK_DEAD)) {
+ err = -ENXIO;
+ goto done;
+ }
+
if (chan->mode == L2CAP_MODE_ERTM && !list_empty(&pi->rx_busy)) {
err = -ENOMEM;
@@ -1534,6 +1542,9 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
}
done:
+ sock_put(sk);
+ l2cap_chan_unlock(chan);
+ l2cap_chan_put(chan);
release_sock(sk);
return err;
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb
2024-06-11 3:00 ` Edward Adam Davis
@ 2024-06-11 3:43 ` syzbot
0 siblings, 0 replies; 33+ messages in thread
From: syzbot @ 2024-06-11 3:43 UTC (permalink / raw)
To: eadavis, linux-kernel, syzkaller-bugs
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue:
WARNING: held lock freed in l2cap_sock_recv_cb
=========================
WARNING: held lock freed!
6.10.0-rc1-syzkaller-00267-gcc8ed4d0a848-dirty #0 Not tainted
-------------------------
kworker/u9:3/6296 is freeing memory ffff88807b68b000-ffff88807b68b7ff, with a lock still held there!
ffff88807b68b258 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1602 [inline]
ffff88807b68b258 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}, at: l2cap_sock_recv_cb+0x53/0x6d0 net/bluetooth/l2cap_sock.c:1488
4 locks held by kworker/u9:3/6296:
#0: ffff88807c089948 ((wq_completion)hci1#2){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3206 [inline]
#0: ffff88807c089948 ((wq_completion)hci1#2){+.+.}-{0:0}, at: process_scheduled_works+0x90a/0x1830 kernel/workqueue.c:3312
#1: ffffc900032b7d00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3207 [inline]
#1: ffffc900032b7d00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_scheduled_works+0x945/0x1830 kernel/workqueue.c:3312
#2: ffff88807b68b258 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1602 [inline]
#2: ffff88807b68b258 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}, at: l2cap_sock_recv_cb+0x53/0x6d0 net/bluetooth/l2cap_sock.c:1488
#3: ffff88807b68c518 (&chan->lock/1){+.+.}-{3:3}, at: l2cap_chan_lock include/net/bluetooth/l2cap.h:827 [inline]
#3: ffff88807b68c518 (&chan->lock/1){+.+.}-{3:3}, at: l2cap_sock_recv_cb+0xf3/0x6d0 net/bluetooth/l2cap_sock.c:1491
stack backtrace:
CPU: 1 PID: 6296 Comm: kworker/u9:3 Not tainted 6.10.0-rc1-syzkaller-00267-gcc8ed4d0a848-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/02/2024
Workqueue: hci1 hci_rx_work
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
print_freed_lock_bug kernel/locking/lockdep.c:6538 [inline]
debug_check_no_locks_freed+0x3c5/0x4a0 kernel/locking/lockdep.c:6571
slab_free_hook mm/slub.c:2159 [inline]
slab_free mm/slub.c:4436 [inline]
kfree+0xfa/0x360 mm/slub.c:4557
sk_prot_free net/core/sock.c:2116 [inline]
__sk_destruct+0x476/0x5f0 net/core/sock.c:2208
sock_put include/net/sock.h:1879 [inline]
l2cap_sock_recv_cb+0x59c/0x6d0 net/bluetooth/l2cap_sock.c:1545
l2cap_conless_channel net/bluetooth/l2cap_core.c:6780 [inline]
l2cap_recv_frame+0x8b6d/0x10670 net/bluetooth/l2cap_core.c:6833
hci_acldata_packet net/bluetooth/hci_core.c:3842 [inline]
hci_rx_work+0x50f/0xca0 net/bluetooth/hci_core.c:4079
process_one_work kernel/workqueue.c:3231 [inline]
process_scheduled_works+0xa2c/0x1830 kernel/workqueue.c:3312
worker_thread+0x86d/0xd70 kernel/workqueue.c:3393
kthread+0x2f0/0x390 kernel/kthread.c:389
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
==================================================================
BUG: KASAN: slab-use-after-free in debug_spin_lock_before kernel/locking/spinlock_debug.c:86 [inline]
BUG: KASAN: slab-use-after-free in do_raw_spin_lock+0x299/0x370 kernel/locking/spinlock_debug.c:115
Read of size 4 at addr ffff88807b68b1c4 by task kworker/u9:3/6296
CPU: 1 PID: 6296 Comm: kworker/u9:3 Not tainted 6.10.0-rc1-syzkaller-00267-gcc8ed4d0a848-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/02/2024
Workqueue: hci1 hci_rx_work
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
print_address_description mm/kasan/report.c:377 [inline]
print_report+0x169/0x550 mm/kasan/report.c:488
kasan_report+0x143/0x180 mm/kasan/report.c:601
debug_spin_lock_before kernel/locking/spinlock_debug.c:86 [inline]
do_raw_spin_lock+0x299/0x370 kernel/locking/spinlock_debug.c:115
spin_lock_bh include/linux/spinlock.h:356 [inline]
release_sock+0x30/0x1f0 net/core/sock.c:3547
l2cap_sock_recv_cb+0x5c8/0x6d0 net/bluetooth/l2cap_sock.c:1548
l2cap_conless_channel net/bluetooth/l2cap_core.c:6780 [inline]
l2cap_recv_frame+0x8b6d/0x10670 net/bluetooth/l2cap_core.c:6833
hci_acldata_packet net/bluetooth/hci_core.c:3842 [inline]
hci_rx_work+0x50f/0xca0 net/bluetooth/hci_core.c:4079
process_one_work kernel/workqueue.c:3231 [inline]
process_scheduled_works+0xa2c/0x1830 kernel/workqueue.c:3312
worker_thread+0x86d/0xd70 kernel/workqueue.c:3393
kthread+0x2f0/0x390 kernel/kthread.c:389
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
Allocated by task 6310:
kasan_save_stack mm/kasan/common.c:47 [inline]
kasan_save_track+0x3f/0x80 mm/kasan/common.c:68
poison_kmalloc_redzone mm/kasan/common.c:370 [inline]
__kasan_kmalloc+0x98/0xb0 mm/kasan/common.c:387
kasan_kmalloc include/linux/kasan.h:211 [inline]
__do_kmalloc_node mm/slub.c:4121 [inline]
__kmalloc_noprof+0x1f9/0x400 mm/slub.c:4134
kmalloc_noprof include/linux/slab.h:664 [inline]
sk_prot_alloc+0xe0/0x210 net/core/sock.c:2080
sk_alloc+0x38/0x370 net/core/sock.c:2133
bt_sock_alloc+0x3c/0x340 net/bluetooth/af_bluetooth.c:148
l2cap_sock_alloc net/bluetooth/l2cap_sock.c:1880 [inline]
l2cap_sock_create+0x13f/0x2d0 net/bluetooth/l2cap_sock.c:1920
bt_sock_create+0x161/0x230 net/bluetooth/af_bluetooth.c:132
__sock_create+0x490/0x920 net/socket.c:1571
sock_create net/socket.c:1622 [inline]
__sys_socket_create net/socket.c:1659 [inline]
__sys_socket+0x150/0x3c0 net/socket.c:1706
__do_sys_socket net/socket.c:1720 [inline]
__se_sys_socket net/socket.c:1718 [inline]
__x64_sys_socket+0x7a/0x90 net/socket.c:1718
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Freed by task 6296:
kasan_save_stack mm/kasan/common.c:47 [inline]
kasan_save_track+0x3f/0x80 mm/kasan/common.c:68
kasan_save_free_info+0x40/0x50 mm/kasan/generic.c:579
poison_slab_object+0xe0/0x150 mm/kasan/common.c:240
__kasan_slab_free+0x37/0x60 mm/kasan/common.c:256
kasan_slab_free include/linux/kasan.h:184 [inline]
slab_free_hook mm/slub.c:2195 [inline]
slab_free mm/slub.c:4436 [inline]
kfree+0x149/0x360 mm/slub.c:4557
sk_prot_free net/core/sock.c:2116 [inline]
__sk_destruct+0x476/0x5f0 net/core/sock.c:2208
sock_put include/net/sock.h:1879 [inline]
l2cap_sock_recv_cb+0x59c/0x6d0 net/bluetooth/l2cap_sock.c:1545
l2cap_conless_channel net/bluetooth/l2cap_core.c:6780 [inline]
l2cap_recv_frame+0x8b6d/0x10670 net/bluetooth/l2cap_core.c:6833
hci_acldata_packet net/bluetooth/hci_core.c:3842 [inline]
hci_rx_work+0x50f/0xca0 net/bluetooth/hci_core.c:4079
process_one_work kernel/workqueue.c:3231 [inline]
process_scheduled_works+0xa2c/0x1830 kernel/workqueue.c:3312
worker_thread+0x86d/0xd70 kernel/workqueue.c:3393
kthread+0x2f0/0x390 kernel/kthread.c:389
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
The buggy address belongs to the object at ffff88807b68b000
which belongs to the cache kmalloc-2k of size 2048
The buggy address is located 452 bytes inside of
freed 2048-byte region [ffff88807b68b000, ffff88807b68b800)
The buggy address belongs to the physical page:
page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x7b688
head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
flags: 0xfff00000000040(head|node=0|zone=1|lastcpupid=0x7ff)
page_type: 0xffffefff(slab)
raw: 00fff00000000040 ffff888015042000 dead000000000122 0000000000000000
raw: 0000000000000000 0000000000080008 00000001ffffefff 0000000000000000
head: 00fff00000000040 ffff888015042000 dead000000000122 0000000000000000
head: 0000000000000000 0000000000080008 00000001ffffefff 0000000000000000
head: 00fff00000000003 ffffea0001eda201 ffffffffffffffff 0000000000000000
head: 0000000000000008 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 3, migratetype Unmovable, gfp_mask 0x1d20c0(__GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC|__GFP_HARDWALL), pid 6294, tgid 6294 (syz-executor), ts 136748851035, free_ts 136662215287
set_page_owner include/linux/page_owner.h:32 [inline]
post_alloc_hook+0x1f3/0x230 mm/page_alloc.c:1468
prep_new_page mm/page_alloc.c:1476 [inline]
get_page_from_freelist+0x2e2d/0x2ee0 mm/page_alloc.c:3402
__alloc_pages_noprof+0x256/0x6c0 mm/page_alloc.c:4660
__alloc_pages_node_noprof include/linux/gfp.h:269 [inline]
alloc_pages_node_noprof include/linux/gfp.h:296 [inline]
alloc_slab_page+0x5f/0x120 mm/slub.c:2264
allocate_slab+0x5a/0x2e0 mm/slub.c:2427
new_slab mm/slub.c:2480 [inline]
___slab_alloc+0xcd1/0x14b0 mm/slub.c:3666
__slab_alloc+0x58/0xa0 mm/slub.c:3756
__slab_alloc_node mm/slub.c:3809 [inline]
slab_alloc_node mm/slub.c:3988 [inline]
__do_kmalloc_node mm/slub.c:4120 [inline]
__kmalloc_noprof+0x257/0x400 mm/slub.c:4134
kmalloc_noprof include/linux/slab.h:664 [inline]
kzalloc_noprof include/linux/slab.h:778 [inline]
ip6t_alloc_initial_table+0x71/0x640 net/ipv6/netfilter/ip6_tables.c:40
ip6table_nat_table_init+0x23/0x2d0 net/ipv6/netfilter/ip6table_nat.c:113
xt_find_table_lock+0x2d4/0x3b0 net/netfilter/x_tables.c:1260
xt_request_find_table_lock+0x26/0x100 net/netfilter/x_tables.c:1285
get_info net/ipv6/netfilter/ip6_tables.c:979 [inline]
do_ip6t_get_ctl+0x89e/0x1820 net/ipv6/netfilter/ip6_tables.c:1668
nf_getsockopt+0x299/0x2c0 net/netfilter/nf_sockopt.c:116
ipv6_getsockopt+0x263/0x380 net/ipv6/ipv6_sockglue.c:1494
tcp_getsockopt+0x163/0x1c0 net/ipv4/tcp.c:4399
page last free pid 6294 tgid 6294 stack trace:
reset_page_owner include/linux/page_owner.h:25 [inline]
free_pages_prepare mm/page_alloc.c:1088 [inline]
free_unref_page+0xd19/0xea0 mm/page_alloc.c:2565
__slab_free+0x31b/0x3d0 mm/slub.c:4347
qlink_free mm/kasan/quarantine.c:163 [inline]
qlist_free_all+0x9e/0x140 mm/kasan/quarantine.c:179
kasan_quarantine_reduce+0x14f/0x170 mm/kasan/quarantine.c:286
__kasan_slab_alloc+0x23/0x80 mm/kasan/common.c:322
kasan_slab_alloc include/linux/kasan.h:201 [inline]
slab_post_alloc_hook mm/slub.c:3940 [inline]
slab_alloc_node mm/slub.c:4000 [inline]
__do_kmalloc_node mm/slub.c:4120 [inline]
__kmalloc_noprof+0x1a3/0x400 mm/slub.c:4134
kmalloc_noprof include/linux/slab.h:664 [inline]
kzalloc_noprof include/linux/slab.h:778 [inline]
ieee80211_register_hw+0x1bb2/0x3d80 net/mac80211/main.c:1328
mac80211_hwsim_new_radio+0x2597/0x44c0 drivers/net/wireless/virtual/mac80211_hwsim.c:5470
hwsim_new_radio_nl+0xe4c/0x21d0 drivers/net/wireless/virtual/mac80211_hwsim.c:6151
genl_family_rcv_msg_doit net/netlink/genetlink.c:1115 [inline]
genl_family_rcv_msg net/netlink/genetlink.c:1195 [inline]
genl_rcv_msg+0xb14/0xec0 net/netlink/genetlink.c:1210
netlink_rcv_skb+0x1e3/0x430 net/netlink/af_netlink.c:2564
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
netlink_unicast_kernel net/netlink/af_netlink.c:1335 [inline]
netlink_unicast+0x7ea/0x980 net/netlink/af_netlink.c:1361
netlink_sendmsg+0x8db/0xcb0 net/netlink/af_netlink.c:1905
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0x221/0x270 net/socket.c:745
__sys_sendto+0x3a4/0x4f0 net/socket.c:2192
Memory state around the buggy address:
ffff88807b68b080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88807b68b100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff88807b68b180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88807b68b200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88807b68b280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
Tested on:
commit: cc8ed4d0 Merge tag 'drm-fixes-2024-06-01' of https://g..
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=105ae75e980000
kernel config: https://syzkaller.appspot.com/x/.config?x=47d282ddffae809f
dashboard link: https://syzkaller.appspot.com/bug?extid=b7f6f8c9303466e16c8a
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=1098dcca980000
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb
2024-06-07 6:02 [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb syzbot
` (2 preceding siblings ...)
2024-06-11 3:00 ` Edward Adam Davis
@ 2024-06-11 4:09 ` Edward Adam Davis
2024-06-11 4:34 ` syzbot
2024-06-11 6:35 ` Edward Adam Davis
` (6 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Edward Adam Davis @ 2024-06-11 4:09 UTC (permalink / raw)
To: syzbot+b7f6f8c9303466e16c8a; +Cc: linux-kernel, syzkaller-bugs
please test null ptr defref in l2cap_sock_recv_cb
#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git cc8ed4d0a848
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 6db60946c627..278cc4db922f 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1486,7 +1486,14 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
int err;
lock_sock(sk);
-
+ sock_hold(sk);
+ l2cap_chan_hold(chan);
+ l2cap_chan_lock(chan);
+ if (sock_flag(sk, SOCK_DEAD)) {
+ err = -ENXIO;
+ goto done;
+ }
+
if (chan->mode == L2CAP_MODE_ERTM && !list_empty(&pi->rx_busy)) {
err = -ENOMEM;
goto done;
@@ -1534,7 +1541,11 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
}
done:
- release_sock(sk);
+ l2cap_chan_unlock(chan);
+ l2cap_chan_put(chan);
+ sock_put(sk);
+ if (err != -ENXIO)
+ release_sock(sk);
return err;
}
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb
2024-06-11 4:09 ` Edward Adam Davis
@ 2024-06-11 4:34 ` syzbot
0 siblings, 0 replies; 33+ messages in thread
From: syzbot @ 2024-06-11 4:34 UTC (permalink / raw)
To: eadavis, linux-kernel, syzkaller-bugs
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue:
WARNING: held lock freed in l2cap_sock_recv_cb
=========================
WARNING: held lock freed!
6.10.0-rc1-syzkaller-00267-gcc8ed4d0a848-dirty #0 Not tainted
-------------------------
kworker/u9:3/6458 is freeing memory ffff88802f212000-ffff88802f2127ff, with a lock still held there!
ffff88802f212258 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1602 [inline]
ffff88802f212258 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}, at: l2cap_sock_recv_cb+0x58/0x6f0 net/bluetooth/l2cap_sock.c:1488
3 locks held by kworker/u9:3/6458:
#0: ffff888079178148 ((wq_completion)hci1#2){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3206 [inline]
#0: ffff888079178148 ((wq_completion)hci1#2){+.+.}-{0:0}, at: process_scheduled_works+0x90a/0x1830 kernel/workqueue.c:3312
#1: ffffc90004347d00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3207 [inline]
#1: ffffc90004347d00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_scheduled_works+0x945/0x1830 kernel/workqueue.c:3312
#2: ffff88802f212258 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1602 [inline]
#2: ffff88802f212258 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}, at: l2cap_sock_recv_cb+0x58/0x6f0 net/bluetooth/l2cap_sock.c:1488
stack backtrace:
CPU: 0 PID: 6458 Comm: kworker/u9:3 Not tainted 6.10.0-rc1-syzkaller-00267-gcc8ed4d0a848-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/02/2024
Workqueue: hci1 hci_rx_work
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
print_freed_lock_bug kernel/locking/lockdep.c:6538 [inline]
debug_check_no_locks_freed+0x3c5/0x4a0 kernel/locking/lockdep.c:6571
slab_free_hook mm/slub.c:2159 [inline]
slab_free mm/slub.c:4436 [inline]
kfree+0xfa/0x360 mm/slub.c:4557
sk_prot_free net/core/sock.c:2116 [inline]
__sk_destruct+0x476/0x5f0 net/core/sock.c:2208
sock_put include/net/sock.h:1879 [inline]
l2cap_sock_recv_cb+0x596/0x6f0 net/bluetooth/l2cap_sock.c:1546
l2cap_conless_channel net/bluetooth/l2cap_core.c:6780 [inline]
l2cap_recv_frame+0x8b6d/0x10670 net/bluetooth/l2cap_core.c:6833
hci_acldata_packet net/bluetooth/hci_core.c:3842 [inline]
hci_rx_work+0x50f/0xca0 net/bluetooth/hci_core.c:4079
process_one_work kernel/workqueue.c:3231 [inline]
process_scheduled_works+0xa2c/0x1830 kernel/workqueue.c:3312
worker_thread+0x86d/0xd70 kernel/workqueue.c:3393
kthread+0x2f0/0x390 kernel/kthread.c:389
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
Bluetooth: hci1: command tx timeout
Tested on:
commit: cc8ed4d0 Merge tag 'drm-fixes-2024-06-01' of https://g..
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=1082c82e980000
kernel config: https://syzkaller.appspot.com/x/.config?x=47d282ddffae809f
dashboard link: https://syzkaller.appspot.com/bug?extid=b7f6f8c9303466e16c8a
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=1706587a980000
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb
2024-06-07 6:02 [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb syzbot
` (3 preceding siblings ...)
2024-06-11 4:09 ` Edward Adam Davis
@ 2024-06-11 6:35 ` Edward Adam Davis
2024-06-11 12:05 ` syzbot
2024-06-11 13:27 ` Edward Adam Davis
` (5 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Edward Adam Davis @ 2024-06-11 6:35 UTC (permalink / raw)
To: syzbot+b7f6f8c9303466e16c8a; +Cc: linux-kernel, syzkaller-bugs
please test null ptr defref in l2cap_sock_recv_cb
#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git cc8ed4d0a848
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 6db60946c627..a9edcf9152c3 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1413,6 +1413,9 @@ static int l2cap_sock_release(struct socket *sock)
l2cap_chan_hold(chan);
l2cap_chan_lock(chan);
+ printk("chan %p data: %p, sk: %p, %s\n", chan, chan->data, sk, __func__);
+ if (refcount_read(&sk->sk_refcnt))
+ chan->data = NULL;
sock_orphan(sk);
l2cap_sock_kill(sk);
@@ -1481,12 +1484,23 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
{
- struct sock *sk = chan->data;
- struct l2cap_pinfo *pi = l2cap_pi(sk);
+ struct sock *sk;
+ struct l2cap_pinfo *pi;
int err;
- lock_sock(sk);
+ l2cap_chan_hold(chan);
+ l2cap_chan_lock(chan);
+ sk = chan->data;
+ printk("chan %p data: %p, is :%d, %s\n", chan, chan->data, IS_ERR_OR_NULL(sk), __func__);
+ if (IS_ERR_OR_NULL(sk) || sock_flag(sk, SOCK_DEAD)) {
+ l2cap_chan_unlock(chan);
+ l2cap_chan_put(chan);
+ return -ENXIO;
+ }
+
+ pi = l2cap_pi(sk);
+ lock_sock(sk);
if (chan->mode == L2CAP_MODE_ERTM && !list_empty(&pi->rx_busy)) {
err = -ENOMEM;
goto done;
@@ -1535,6 +1549,8 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
done:
release_sock(sk);
+ l2cap_chan_unlock(chan);
+ l2cap_chan_put(chan);
return err;
}
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb
2024-06-07 6:02 [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb syzbot
` (4 preceding siblings ...)
2024-06-11 6:35 ` Edward Adam Davis
@ 2024-06-11 13:27 ` Edward Adam Davis
2024-06-11 14:06 ` syzbot
2024-06-11 14:50 ` [PATCH] bluetooth/l2cap: sync sock recv cb and release Edward Adam Davis
` (4 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Edward Adam Davis @ 2024-06-11 13:27 UTC (permalink / raw)
To: syzbot+b7f6f8c9303466e16c8a; +Cc: linux-kernel, syzkaller-bugs
please test null ptr defref in l2cap_sock_recv_cb
#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git cc8ed4d0a848
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 6db60946c627..9938d3681772 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1413,6 +1413,8 @@ static int l2cap_sock_release(struct socket *sock)
l2cap_chan_hold(chan);
l2cap_chan_lock(chan);
+ if (refcount_read(&sk->sk_refcnt) == 1)
+ chan->data = NULL;
sock_orphan(sk);
l2cap_sock_kill(sk);
@@ -1481,12 +1483,23 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
{
- struct sock *sk = chan->data;
- struct l2cap_pinfo *pi = l2cap_pi(sk);
+ struct sock *sk;
+ struct l2cap_pinfo *pi;
int err;
- lock_sock(sk);
+ l2cap_chan_hold(chan);
+ l2cap_chan_lock(chan);
+ sk = chan->data;
+
+ if (!sk) {
+ printk("%s\n", __func__);
+ l2cap_chan_unlock(chan);
+ l2cap_chan_put(chan);
+ return -ENXIO;
+ }
+ pi = l2cap_pi(sk);
+ lock_sock(sk);
if (chan->mode == L2CAP_MODE_ERTM && !list_empty(&pi->rx_busy)) {
err = -ENOMEM;
goto done;
@@ -1535,6 +1548,8 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
done:
release_sock(sk);
+ l2cap_chan_unlock(chan);
+ l2cap_chan_put(chan);
return err;
}
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH] bluetooth/l2cap: sync sock recv cb and release
2024-06-07 6:02 [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb syzbot
` (5 preceding siblings ...)
2024-06-11 13:27 ` Edward Adam Davis
@ 2024-06-11 14:50 ` Edward Adam Davis
2024-06-11 19:24 ` Luiz Augusto von Dentz
2024-06-15 1:25 ` [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb Edward Adam Davis
` (3 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Edward Adam Davis @ 2024-06-11 14:50 UTC (permalink / raw)
To: syzbot+b7f6f8c9303466e16c8a
Cc: johan.hedberg, linux-bluetooth, linux-kernel, luiz.dentz, marcel,
syzkaller-bugs
The problem occurs between the system call to close the sock and hci_rx_work,
where the former releases the sock and the latter accesses it without lock protection.
CPU0 CPU1
---- ----
sock_close hci_rx_work
l2cap_sock_release hci_acldata_packet
l2cap_sock_kill l2cap_recv_frame
sk_free l2cap_conless_channel
l2cap_sock_recv_cb
If hci_rx_work processes the data that needs to be received before the sock is
closed, then everything is normal; Otherwise, the work thread may access the
released sock when receiving data.
Add a chan mutex in the rx callback of the sock to achieve synchronization between
the sock release and recv cb.
Reported-and-tested-by: syzbot+b7f6f8c9303466e16c8a@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
net/bluetooth/l2cap_sock.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 6db60946c627..f3e9236293e1 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1413,6 +1413,8 @@ static int l2cap_sock_release(struct socket *sock)
l2cap_chan_hold(chan);
l2cap_chan_lock(chan);
+ if (refcount_read(&sk->sk_refcnt) == 1)
+ chan->data = NULL;
sock_orphan(sk);
l2cap_sock_kill(sk);
@@ -1481,12 +1483,22 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
{
- struct sock *sk = chan->data;
- struct l2cap_pinfo *pi = l2cap_pi(sk);
+ struct sock *sk;
+ struct l2cap_pinfo *pi;
int err;
- lock_sock(sk);
+ l2cap_chan_hold(chan);
+ l2cap_chan_lock(chan);
+ sk = chan->data;
+
+ if (!sk) {
+ l2cap_chan_unlock(chan);
+ l2cap_chan_put(chan);
+ return -ENXIO;
+ }
+ pi = l2cap_pi(sk);
+ lock_sock(sk);
if (chan->mode == L2CAP_MODE_ERTM && !list_empty(&pi->rx_busy)) {
err = -ENOMEM;
goto done;
@@ -1535,6 +1547,8 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
done:
release_sock(sk);
+ l2cap_chan_unlock(chan);
+ l2cap_chan_put(chan);
return err;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH] bluetooth/l2cap: sync sock recv cb and release
2024-06-11 14:50 ` [PATCH] bluetooth/l2cap: sync sock recv cb and release Edward Adam Davis
@ 2024-06-11 19:24 ` Luiz Augusto von Dentz
2024-06-12 0:33 ` Edward Adam Davis
2024-06-15 1:45 ` [PATCH v2] " Edward Adam Davis
0 siblings, 2 replies; 33+ messages in thread
From: Luiz Augusto von Dentz @ 2024-06-11 19:24 UTC (permalink / raw)
To: Edward Adam Davis
Cc: syzbot+b7f6f8c9303466e16c8a, johan.hedberg, linux-bluetooth,
linux-kernel, marcel, syzkaller-bugs
Hi Edward,
On Tue, Jun 11, 2024 at 10:50 AM Edward Adam Davis <eadavis@qq.com> wrote:
>
> The problem occurs between the system call to close the sock and hci_rx_work,
> where the former releases the sock and the latter accesses it without lock protection.
>
> CPU0 CPU1
> ---- ----
> sock_close hci_rx_work
> l2cap_sock_release hci_acldata_packet
> l2cap_sock_kill l2cap_recv_frame
> sk_free l2cap_conless_channel
> l2cap_sock_recv_cb
>
> If hci_rx_work processes the data that needs to be received before the sock is
> closed, then everything is normal; Otherwise, the work thread may access the
> released sock when receiving data.
>
> Add a chan mutex in the rx callback of the sock to achieve synchronization between
> the sock release and recv cb.
>
> Reported-and-tested-by: syzbot+b7f6f8c9303466e16c8a@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> net/bluetooth/l2cap_sock.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 6db60946c627..f3e9236293e1 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -1413,6 +1413,8 @@ static int l2cap_sock_release(struct socket *sock)
> l2cap_chan_hold(chan);
> l2cap_chan_lock(chan);
>
> + if (refcount_read(&sk->sk_refcnt) == 1)
> + chan->data = NULL;
Might be a good idea to add some comment on why checking for refcount
== 1 is the right thing to do here, or perhaps we can always assign
chan->data to NULL, instead of that perhaps we could have it done in
l2cap_sock_kill?
> sock_orphan(sk);
> l2cap_sock_kill(sk);
>
> @@ -1481,12 +1483,22 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
>
> static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
> {
> - struct sock *sk = chan->data;
> - struct l2cap_pinfo *pi = l2cap_pi(sk);
> + struct sock *sk;
> + struct l2cap_pinfo *pi;
> int err;
>
> - lock_sock(sk);
> + l2cap_chan_hold(chan);
> + l2cap_chan_lock(chan);
> + sk = chan->data;
> +
> + if (!sk) {
> + l2cap_chan_unlock(chan);
> + l2cap_chan_put(chan);
> + return -ENXIO;
> + }
>
> + pi = l2cap_pi(sk);
> + lock_sock(sk);
> if (chan->mode == L2CAP_MODE_ERTM && !list_empty(&pi->rx_busy)) {
> err = -ENOMEM;
> goto done;
> @@ -1535,6 +1547,8 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
>
> done:
> release_sock(sk);
> + l2cap_chan_unlock(chan);
> + l2cap_chan_put(chan);
>
> return err;
> }
> --
> 2.43.0
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH] bluetooth/l2cap: sync sock recv cb and release
2024-06-11 19:24 ` Luiz Augusto von Dentz
@ 2024-06-12 0:33 ` Edward Adam Davis
2024-06-15 1:45 ` [PATCH v2] " Edward Adam Davis
1 sibling, 0 replies; 33+ messages in thread
From: Edward Adam Davis @ 2024-06-12 0:33 UTC (permalink / raw)
To: luiz.dentz
Cc: eadavis, johan.hedberg, linux-bluetooth, linux-kernel, marcel,
syzbot+b7f6f8c9303466e16c8a, syzkaller-bugs
Hi Luiz Augusto von Dentz,
On Tue, 11 Jun 2024 15:24:52 -0400, Luiz Augusto von Dentz wrote:
> > The problem occurs between the system call to close the sock and hci_rx_work,
> > where the former releases the sock and the latter accesses it without lock protection.
> >
> > CPU0 CPU1
> > ---- ----
> > sock_close hci_rx_work
> > l2cap_sock_release hci_acldata_packet
> > l2cap_sock_kill l2cap_recv_frame
> > sk_free l2cap_conless_channel
> > l2cap_sock_recv_cb
> >
> > If hci_rx_work processes the data that needs to be received before the sock is
> > closed, then everything is normal; Otherwise, the work thread may access the
> > released sock when receiving data.
> >
> > Add a chan mutex in the rx callback of the sock to achieve synchronization between
> > the sock release and recv cb.
> >
> > Reported-and-tested-by: syzbot+b7f6f8c9303466e16c8a@syzkaller.appspotmail.com
> > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > ---
> > net/bluetooth/l2cap_sock.c | 20 +++++++++++++++++---
> > 1 file changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > index 6db60946c627..f3e9236293e1 100644
> > --- a/net/bluetooth/l2cap_sock.c
> > +++ b/net/bluetooth/l2cap_sock.c
> > @@ -1413,6 +1413,8 @@ static int l2cap_sock_release(struct socket *sock)
> > l2cap_chan_hold(chan);
> > l2cap_chan_lock(chan);
> >
> > + if (refcount_read(&sk->sk_refcnt) == 1)
> > + chan->data = NULL;
>
> Might be a good idea to add some comment on why checking for refcount
> == 1 is the right thing to do here, or perhaps we can always assign
> chan->data to NULL, instead of that perhaps we could have it done in
> l2cap_sock_kill?
In this case, it is possible to always set chan->data to NULL, but I think a
better approach would be to release sock in l2cap_sock_kill when the reference
count of the sock is 1. Previously, it was mentioned that setting chan->data to
NULL is more rigorous.
And chan->data is bound one-on-one to the sock, if the sock is not released,
I can't confirm whether setting chan->data to NULL will affect the operation of
the sock on other paths.
>
> > sock_orphan(sk);
> > l2cap_sock_kill(sk);
> >
> > @@ -1481,12 +1483,22 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
> >
> > static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
> > {
> > - struct sock *sk = chan->data;
> > - struct l2cap_pinfo *pi = l2cap_pi(sk);
> > + struct sock *sk;
> > + struct l2cap_pinfo *pi;
> > int err;
> >
> > - lock_sock(sk);
> > + l2cap_chan_hold(chan);
> > + l2cap_chan_lock(chan);
> > + sk = chan->data;
> > +
> > + if (!sk) {
> > + l2cap_chan_unlock(chan);
> > + l2cap_chan_put(chan);
> > + return -ENXIO;
> > + }
> >
> > + pi = l2cap_pi(sk);
> > + lock_sock(sk);
> > if (chan->mode == L2CAP_MODE_ERTM && !list_empty(&pi->rx_busy)) {
> > err = -ENOMEM;
> > goto done;
> > @@ -1535,6 +1547,8 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
> >
> > done:
> > release_sock(sk);
> > + l2cap_chan_unlock(chan);
> > + l2cap_chan_put(chan);
> >
> > return err;
> > }
--
Edward
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH v2] bluetooth/l2cap: sync sock recv cb and release
2024-06-11 19:24 ` Luiz Augusto von Dentz
2024-06-12 0:33 ` Edward Adam Davis
@ 2024-06-15 1:45 ` Edward Adam Davis
2024-06-17 19:00 ` patchwork-bot+bluetooth
2024-06-20 16:53 ` Luiz Augusto von Dentz
1 sibling, 2 replies; 33+ messages in thread
From: Edward Adam Davis @ 2024-06-15 1:45 UTC (permalink / raw)
To: luiz.dentz
Cc: eadavis, johan.hedberg, linux-bluetooth, linux-kernel, marcel,
syzbot+b7f6f8c9303466e16c8a, syzkaller-bugs
The problem occurs between the system call to close the sock and hci_rx_work,
where the former releases the sock and the latter accesses it without lock protection.
CPU0 CPU1
---- ----
sock_close hci_rx_work
l2cap_sock_release hci_acldata_packet
l2cap_sock_kill l2cap_recv_frame
sk_free l2cap_conless_channel
l2cap_sock_recv_cb
If hci_rx_work processes the data that needs to be received before the sock is
closed, then everything is normal; Otherwise, the work thread may access the
released sock when receiving data.
Add a chan mutex in the rx callback of the sock to achieve synchronization between
the sock release and recv cb.
Sock is dead, so set chan data to NULL, avoid others use invalid sock pointer.
Reported-and-tested-by: syzbot+b7f6f8c9303466e16c8a@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
net/bluetooth/l2cap_sock.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 6db60946c627..f45cdf9bc985 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1239,6 +1239,10 @@ static void l2cap_sock_kill(struct sock *sk)
BT_DBG("sk %p state %s", sk, state_to_string(sk->sk_state));
+ /* Sock is dead, so set chan data to NULL, avoid other task use invalid
+ * sock pointer.
+ */
+ l2cap_pi(sk)->chan->data = NULL;
/* Kill poor orphan */
l2cap_chan_put(l2cap_pi(sk)->chan);
@@ -1481,12 +1485,25 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
{
- struct sock *sk = chan->data;
- struct l2cap_pinfo *pi = l2cap_pi(sk);
+ struct sock *sk;
+ struct l2cap_pinfo *pi;
int err;
- lock_sock(sk);
+ /* To avoid race with sock_release, a chan lock needs to be added here
+ * to synchronize the sock.
+ */
+ l2cap_chan_hold(chan);
+ l2cap_chan_lock(chan);
+ sk = chan->data;
+ if (!sk) {
+ l2cap_chan_unlock(chan);
+ l2cap_chan_put(chan);
+ return -ENXIO;
+ }
+
+ pi = l2cap_pi(sk);
+ lock_sock(sk);
if (chan->mode == L2CAP_MODE_ERTM && !list_empty(&pi->rx_busy)) {
err = -ENOMEM;
goto done;
@@ -1535,6 +1552,8 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
done:
release_sock(sk);
+ l2cap_chan_unlock(chan);
+ l2cap_chan_put(chan);
return err;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v2] bluetooth/l2cap: sync sock recv cb and release
2024-06-15 1:45 ` [PATCH v2] " Edward Adam Davis
@ 2024-06-17 19:00 ` patchwork-bot+bluetooth
2024-06-20 16:53 ` Luiz Augusto von Dentz
1 sibling, 0 replies; 33+ messages in thread
From: patchwork-bot+bluetooth @ 2024-06-17 19:00 UTC (permalink / raw)
To: Edward Adam Davis
Cc: luiz.dentz, johan.hedberg, linux-bluetooth, linux-kernel, marcel,
syzbot+b7f6f8c9303466e16c8a, syzkaller-bugs
Hello:
This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
On Sat, 15 Jun 2024 09:45:54 +0800 you wrote:
> The problem occurs between the system call to close the sock and hci_rx_work,
> where the former releases the sock and the latter accesses it without lock protection.
>
> CPU0 CPU1
> ---- ----
> sock_close hci_rx_work
> l2cap_sock_release hci_acldata_packet
> l2cap_sock_kill l2cap_recv_frame
> sk_free l2cap_conless_channel
> l2cap_sock_recv_cb
>
> [...]
Here is the summary with links:
- [v2] bluetooth/l2cap: sync sock recv cb and release
https://git.kernel.org/bluetooth/bluetooth-next/c/e3203b177717
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v2] bluetooth/l2cap: sync sock recv cb and release
2024-06-15 1:45 ` [PATCH v2] " Edward Adam Davis
2024-06-17 19:00 ` patchwork-bot+bluetooth
@ 2024-06-20 16:53 ` Luiz Augusto von Dentz
2024-06-21 14:45 ` [PATCH] " Edward Adam Davis
2024-06-22 3:46 ` Edward Adam Davis
1 sibling, 2 replies; 33+ messages in thread
From: Luiz Augusto von Dentz @ 2024-06-20 16:53 UTC (permalink / raw)
To: Edward Adam Davis
Cc: johan.hedberg, linux-bluetooth, linux-kernel, marcel,
syzbot+b7f6f8c9303466e16c8a, syzkaller-bugs
Hi Edward,
On Fri, Jun 14, 2024 at 9:46 PM Edward Adam Davis <eadavis@qq.com> wrote:
>
> The problem occurs between the system call to close the sock and hci_rx_work,
> where the former releases the sock and the latter accesses it without lock protection.
>
> CPU0 CPU1
> ---- ----
> sock_close hci_rx_work
> l2cap_sock_release hci_acldata_packet
> l2cap_sock_kill l2cap_recv_frame
> sk_free l2cap_conless_channel
> l2cap_sock_recv_cb
>
> If hci_rx_work processes the data that needs to be received before the sock is
> closed, then everything is normal; Otherwise, the work thread may access the
> released sock when receiving data.
>
> Add a chan mutex in the rx callback of the sock to achieve synchronization between
> the sock release and recv cb.
>
> Sock is dead, so set chan data to NULL, avoid others use invalid sock pointer.
>
> Reported-and-tested-by: syzbot+b7f6f8c9303466e16c8a@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> net/bluetooth/l2cap_sock.c | 25 ++++++++++++++++++++++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 6db60946c627..f45cdf9bc985 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -1239,6 +1239,10 @@ static void l2cap_sock_kill(struct sock *sk)
>
> BT_DBG("sk %p state %s", sk, state_to_string(sk->sk_state));
>
> + /* Sock is dead, so set chan data to NULL, avoid other task use invalid
> + * sock pointer.
> + */
> + l2cap_pi(sk)->chan->data = NULL;
> /* Kill poor orphan */
>
> l2cap_chan_put(l2cap_pi(sk)->chan);
> @@ -1481,12 +1485,25 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
>
> static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
> {
> - struct sock *sk = chan->data;
> - struct l2cap_pinfo *pi = l2cap_pi(sk);
> + struct sock *sk;
> + struct l2cap_pinfo *pi;
> int err;
>
> - lock_sock(sk);
> + /* To avoid race with sock_release, a chan lock needs to be added here
> + * to synchronize the sock.
> + */
> + l2cap_chan_hold(chan);
> + l2cap_chan_lock(chan);
> + sk = chan->data;
>
> + if (!sk) {
> + l2cap_chan_unlock(chan);
> + l2cap_chan_put(chan);
> + return -ENXIO;
> + }
> +
> + pi = l2cap_pi(sk);
> + lock_sock(sk);
> if (chan->mode == L2CAP_MODE_ERTM && !list_empty(&pi->rx_busy)) {
> err = -ENOMEM;
> goto done;
> @@ -1535,6 +1552,8 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
>
> done:
> release_sock(sk);
> + l2cap_chan_unlock(chan);
> + l2cap_chan_put(chan);
>
> return err;
> }
> --
> 2.43.0
Looks like this was never really tested properly:
============================================
WARNING: possible recursive locking detected
6.10.0-rc3-g4029dba6b6f1 #6823 Not tainted
--------------------------------------------
kworker/u5:0/35 is trying to acquire lock:
ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
l2cap_sock_recv_cb+0x44/0x1e0
but task is already holding lock:
ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
l2cap_get_chan_by_scid+0xaf/0xd0
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&chan->lock#2/1);
lock(&chan->lock#2/1);
*** DEADLOCK ***
May be due to missing lock nesting notation
3 locks held by kworker/u5:0/35:
#0: ffff888002b8a940 ((wq_completion)hci0#2){+.+.}-{0:0}, at:
process_one_work+0x750/0x930
#1: ffff888002c67dd0 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0},
at: process_one_work+0x44e/0x930
#2: ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
l2cap_get_chan_by_scid+0xaf/0xd0
l2cap_sock_recv_cb is assumed to be called with the chan_lock held so
perhaps we can just do:
sk = chan->data;
if (!sk)
return -ENXIO;
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH] bluetooth/l2cap: sync sock recv cb and release
2024-06-20 16:53 ` Luiz Augusto von Dentz
@ 2024-06-21 14:45 ` Edward Adam Davis
2024-06-22 3:46 ` Edward Adam Davis
1 sibling, 0 replies; 33+ messages in thread
From: Edward Adam Davis @ 2024-06-21 14:45 UTC (permalink / raw)
To: luiz.dentz
Cc: eadavis, johan.hedberg, linux-bluetooth, linux-kernel, marcel,
syzbot+b7f6f8c9303466e16c8a, syzkaller-bugs
Hi Luiz Augusto von Dentz,
On Thu, 20 Jun 2024 12:53:19 -0400, Luiz Augusto von Dentz wrote:
> > release_sock(sk);
> > + l2cap_chan_unlock(chan);
> > + l2cap_chan_put(chan);
> >
> > return err;
> > }
> > --
> > 2.43.0
>
> Looks like this was never really tested properly:
>
> ============================================
> WARNING: possible recursive locking detected
> 6.10.0-rc3-g4029dba6b6f1 #6823 Not tainted
> --------------------------------------------
> kworker/u5:0/35 is trying to acquire lock:
> ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
> l2cap_sock_recv_cb+0x44/0x1e0
>
> but task is already holding lock:
> ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
> l2cap_get_chan_by_scid+0xaf/0xd0
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(&chan->lock#2/1);
> lock(&chan->lock#2/1);
>
> *** DEADLOCK ***
>
> May be due to missing lock nesting notation
>
> 3 locks held by kworker/u5:0/35:
> #0: ffff888002b8a940 ((wq_completion)hci0#2){+.+.}-{0:0}, at:
> process_one_work+0x750/0x930
> #1: ffff888002c67dd0 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0},
> at: process_one_work+0x44e/0x930
> #2: ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
> l2cap_get_chan_by_scid+0xaf/0xd0
>
> l2cap_sock_recv_cb is assumed to be called with the chan_lock held so
> perhaps we can just do:
>
> sk = chan->data;
> if (!sk)
> return -ENXIO;
If the release occurs after this judgment, the same problem will still occur.
Recv and release must be synchronized using locks, which can be solved by
adding new lock.
Can you provide a reproduction program for the AA lock mentioned above?
--
Edward
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH] bluetooth/l2cap: sync sock recv cb and release
2024-06-20 16:53 ` Luiz Augusto von Dentz
2024-06-21 14:45 ` [PATCH] " Edward Adam Davis
@ 2024-06-22 3:46 ` Edward Adam Davis
2024-06-24 13:36 ` Luiz Augusto von Dentz
1 sibling, 1 reply; 33+ messages in thread
From: Edward Adam Davis @ 2024-06-22 3:46 UTC (permalink / raw)
To: luiz.dentz
Cc: eadavis, johan.hedberg, linux-bluetooth, linux-kernel, marcel,
syzbot+b7f6f8c9303466e16c8a, syzkaller-bugs
Hi Luiz Augusto von Dentz,
On Thu, 20 Jun 2024 12:53:19 -0400, Luiz Augusto von Dentz wrote:
> > release_sock(sk);
> > + l2cap_chan_unlock(chan);
> > + l2cap_chan_put(chan);
> >
> > return err;
> > }
> > --
> > 2.43.0
>
> Looks like this was never really tested properly:
>
> ============================================
> WARNING: possible recursive locking detected
> 6.10.0-rc3-g4029dba6b6f1 #6823 Not tainted
> --------------------------------------------
> kworker/u5:0/35 is trying to acquire lock:
> ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
> l2cap_sock_recv_cb+0x44/0x1e0
>
> but task is already holding lock:
> ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
> l2cap_get_chan_by_scid+0xaf/0xd0
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(&chan->lock#2/1);
> lock(&chan->lock#2/1);
>
> *** DEADLOCK ***
>
> May be due to missing lock nesting notation
>
> 3 locks held by kworker/u5:0/35:
> #0: ffff888002b8a940 ((wq_completion)hci0#2){+.+.}-{0:0}, at:
> process_one_work+0x750/0x930
> #1: ffff888002c67dd0 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0},
> at: process_one_work+0x44e/0x930
> #2: ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
> l2cap_get_chan_by_scid+0xaf/0xd0
>
> l2cap_sock_recv_cb is assumed to be called with the chan_lock held so
> perhaps we can just do:
>
> sk = chan->data;
> if (!sk)
> return -ENXIO;
If the release occurs after this judgment, the same problem will still occur.
Recv and release must be synchronized using locks, which can be solved by
adding new lock.
Please use the new patch https://syzkaller.appspot.com/text?tag=Patch&x=15d2c48e980000, I have tested in
https://syzkaller.appspot.com/bug?extid=b7f6f8c9303466e16c8a
--
Edward
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH] bluetooth/l2cap: sync sock recv cb and release
2024-06-22 3:46 ` Edward Adam Davis
@ 2024-06-24 13:36 ` Luiz Augusto von Dentz
2024-06-25 0:52 ` Edward Adam Davis
0 siblings, 1 reply; 33+ messages in thread
From: Luiz Augusto von Dentz @ 2024-06-24 13:36 UTC (permalink / raw)
To: Edward Adam Davis
Cc: johan.hedberg, linux-bluetooth, linux-kernel, marcel,
syzbot+b7f6f8c9303466e16c8a, syzkaller-bugs
Hi Edward,
On Fri, Jun 21, 2024 at 11:46 PM Edward Adam Davis <eadavis@qq.com> wrote:
>
> Hi Luiz Augusto von Dentz,
>
> On Thu, 20 Jun 2024 12:53:19 -0400, Luiz Augusto von Dentz wrote:
> > > release_sock(sk);
> > > + l2cap_chan_unlock(chan);
> > > + l2cap_chan_put(chan);
> > >
> > > return err;
> > > }
> > > --
> > > 2.43.0
> >
> > Looks like this was never really tested properly:
> >
> > ============================================
> > WARNING: possible recursive locking detected
> > 6.10.0-rc3-g4029dba6b6f1 #6823 Not tainted
> > --------------------------------------------
> > kworker/u5:0/35 is trying to acquire lock:
> > ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
> > l2cap_sock_recv_cb+0x44/0x1e0
> >
> > but task is already holding lock:
> > ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
> > l2cap_get_chan_by_scid+0xaf/0xd0
> >
> > other info that might help us debug this:
> > Possible unsafe locking scenario:
> >
> > CPU0
> > ----
> > lock(&chan->lock#2/1);
> > lock(&chan->lock#2/1);
> >
> > *** DEADLOCK ***
> >
> > May be due to missing lock nesting notation
> >
> > 3 locks held by kworker/u5:0/35:
> > #0: ffff888002b8a940 ((wq_completion)hci0#2){+.+.}-{0:0}, at:
> > process_one_work+0x750/0x930
> > #1: ffff888002c67dd0 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0},
> > at: process_one_work+0x44e/0x930
> > #2: ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
> > l2cap_get_chan_by_scid+0xaf/0xd0
> >
> > l2cap_sock_recv_cb is assumed to be called with the chan_lock held so
> > perhaps we can just do:
> >
> > sk = chan->data;
> > if (!sk)
> > return -ENXIO;
>
> If the release occurs after this judgment, the same problem will still occur.
> Recv and release must be synchronized using locks, which can be solved by
> adding new lock.
>
> Please use the new patch https://syzkaller.appspot.com/text?tag=Patch&x=15d2c48e980000, I have tested in
> https://syzkaller.appspot.com/bug?extid=b7f6f8c9303466e16c8a
Hmm, why don't we just fix l2cap_conless_channel? Btw,
l2cap_conless_channel is normally not used by any profiles thus why
there isn't any CI covering it, on the other hand l2cap_data_channel
is used by 99% of the profiles.
> --
> Edward
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH] bluetooth/l2cap: sync sock recv cb and release
2024-06-24 13:36 ` Luiz Augusto von Dentz
@ 2024-06-25 0:52 ` Edward Adam Davis
2024-06-25 3:08 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 33+ messages in thread
From: Edward Adam Davis @ 2024-06-25 0:52 UTC (permalink / raw)
To: luiz.dentz
Cc: eadavis, johan.hedberg, linux-bluetooth, linux-kernel, marcel,
syzbot+b7f6f8c9303466e16c8a, syzkaller-bugs
Hi Luiz Augusto von Dentz,
On Mon, 24 Jun 2024 09:36:14 -0400, Luiz Augusto von Dentz wrote:
> > > Looks like this was never really tested properly:
> > >
> > > ============================================
> > > WARNING: possible recursive locking detected
> > > 6.10.0-rc3-g4029dba6b6f1 #6823 Not tainted
> > > --------------------------------------------
> > > kworker/u5:0/35 is trying to acquire lock:
> > > ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
> > > l2cap_sock_recv_cb+0x44/0x1e0
> > >
> > > but task is already holding lock:
> > > ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
> > > l2cap_get_chan_by_scid+0xaf/0xd0
> > >
> > > other info that might help us debug this:
> > > Possible unsafe locking scenario:
> > >
> > > CPU0
> > > ----
> > > lock(&chan->lock#2/1);
> > > lock(&chan->lock#2/1);
> > >
> > > *** DEADLOCK ***
> > >
> > > May be due to missing lock nesting notation
> > >
> > > 3 locks held by kworker/u5:0/35:
> > > #0: ffff888002b8a940 ((wq_completion)hci0#2){+.+.}-{0:0}, at:
> > > process_one_work+0x750/0x930
> > > #1: ffff888002c67dd0 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0},
> > > at: process_one_work+0x44e/0x930
> > > #2: ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
> > > l2cap_get_chan_by_scid+0xaf/0xd0
> > >
> > > l2cap_sock_recv_cb is assumed to be called with the chan_lock held so
> > > perhaps we can just do:
> > >
> > > sk = chan->data;
> > > if (!sk)
> > > return -ENXIO;
> >
> > If the release occurs after this judgment, the same problem will still occur.
> > Recv and release must be synchronized using locks, which can be solved by
> > adding new lock.
> >
> > Please use the new patch https://syzkaller.appspot.com/text?tag=Patch&x=15d2c48e980000, I have tested in
> > https://syzkaller.appspot.com/bug?extid=b7f6f8c9303466e16c8a
>
> Hmm, why don't we just fix l2cap_conless_channel? Btw,
> l2cap_conless_channel is normally not used by any profiles thus why
> there isn't any CI covering it, on the other hand l2cap_data_channel
> is used by 99% of the profiles.
Yes, we can fix l2cap_conless_channel, but key point is that "chan->lock"
cannot be used to synchronize l2cap_conless_channel and l2cap_sock_release,
otherwise it will form an AA lock with l2cap_data_channel.
Why not fix it in l2cap_conless_channel but in l2cap_sock_recv_cb, because
l2cap_sock_recv_cb is on the same layer as l2cap_sock_kill, using a new
lock for synchronization is more appropriate.
--
Edward
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH] bluetooth/l2cap: sync sock recv cb and release
2024-06-25 0:52 ` Edward Adam Davis
@ 2024-06-25 3:08 ` Luiz Augusto von Dentz
2024-06-25 10:12 ` [PATCH V3] Bluetooth/l2cap: " Edward Adam Davis
0 siblings, 1 reply; 33+ messages in thread
From: Luiz Augusto von Dentz @ 2024-06-25 3:08 UTC (permalink / raw)
To: Edward Adam Davis
Cc: johan.hedberg, linux-bluetooth, linux-kernel, marcel,
syzbot+b7f6f8c9303466e16c8a, syzkaller-bugs
Hi Edward,
On Mon, Jun 24, 2024 at 8:53 PM Edward Adam Davis <eadavis@qq.com> wrote:
>
> Hi Luiz Augusto von Dentz,
>
> On Mon, 24 Jun 2024 09:36:14 -0400, Luiz Augusto von Dentz wrote:
> > > > Looks like this was never really tested properly:
> > > >
> > > > ============================================
> > > > WARNING: possible recursive locking detected
> > > > 6.10.0-rc3-g4029dba6b6f1 #6823 Not tainted
> > > > --------------------------------------------
> > > > kworker/u5:0/35 is trying to acquire lock:
> > > > ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
> > > > l2cap_sock_recv_cb+0x44/0x1e0
> > > >
> > > > but task is already holding lock:
> > > > ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
> > > > l2cap_get_chan_by_scid+0xaf/0xd0
> > > >
> > > > other info that might help us debug this:
> > > > Possible unsafe locking scenario:
> > > >
> > > > CPU0
> > > > ----
> > > > lock(&chan->lock#2/1);
> > > > lock(&chan->lock#2/1);
> > > >
> > > > *** DEADLOCK ***
> > > >
> > > > May be due to missing lock nesting notation
> > > >
> > > > 3 locks held by kworker/u5:0/35:
> > > > #0: ffff888002b8a940 ((wq_completion)hci0#2){+.+.}-{0:0}, at:
> > > > process_one_work+0x750/0x930
> > > > #1: ffff888002c67dd0 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0},
> > > > at: process_one_work+0x44e/0x930
> > > > #2: ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at:
> > > > l2cap_get_chan_by_scid+0xaf/0xd0
> > > >
> > > > l2cap_sock_recv_cb is assumed to be called with the chan_lock held so
> > > > perhaps we can just do:
> > > >
> > > > sk = chan->data;
> > > > if (!sk)
> > > > return -ENXIO;
> > >
> > > If the release occurs after this judgment, the same problem will still occur.
> > > Recv and release must be synchronized using locks, which can be solved by
> > > adding new lock.
> > >
> > > Please use the new patch https://syzkaller.appspot.com/text?tag=Patch&x=15d2c48e980000, I have tested in
> > > https://syzkaller.appspot.com/bug?extid=b7f6f8c9303466e16c8a
> >
> > Hmm, why don't we just fix l2cap_conless_channel? Btw,
> > l2cap_conless_channel is normally not used by any profiles thus why
> > there isn't any CI covering it, on the other hand l2cap_data_channel
> > is used by 99% of the profiles.
> Yes, we can fix l2cap_conless_channel, but key point is that "chan->lock"
> cannot be used to synchronize l2cap_conless_channel and l2cap_sock_release,
> otherwise it will form an AA lock with l2cap_data_channel.
Don't follow, what l2cap_conless_channel has to do with
l2cap_data_channel? You can't have the same channel calling both, it
is either connectionless or it is not.
> Why not fix it in l2cap_conless_channel but in l2cap_sock_recv_cb, because
> l2cap_sock_recv_cb is on the same layer as l2cap_sock_kill, using a new
> lock for synchronization is more appropriate.
There are dedicated locks for both layers and now that we are doing
chan->lock for before chan->recv the locking sequence should be the
same in all instances.
> --
> Edward
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH V3] Bluetooth/l2cap: sync sock recv cb and release
2024-06-25 3:08 ` Luiz Augusto von Dentz
@ 2024-06-25 10:12 ` Edward Adam Davis
0 siblings, 0 replies; 33+ messages in thread
From: Edward Adam Davis @ 2024-06-25 10:12 UTC (permalink / raw)
To: luiz.dentz
Cc: eadavis, johan.hedberg, linux-bluetooth, linux-kernel, marcel,
syzbot+b7f6f8c9303466e16c8a, syzkaller-bugs
The problem occurs between the system call to close the sock and hci_rx_work,
where the former releases the sock and the latter accesses it without lock protection.
CPU0 CPU1
---- ----
sock_close hci_rx_work
l2cap_sock_release hci_acldata_packet
l2cap_sock_kill l2cap_recv_frame
sk_free l2cap_conless_channel
l2cap_sock_recv_cb
If hci_rx_work processes the data that needs to be received before the sock is
closed, then everything is normal; Otherwise, the work thread may access the
released sock when receiving data.
Add a chan lock in the l2cap_conless_channel of the sock to achieve sync
between the sock release and recv cb.
Sock is dead, so set chan data to NULL, avoid others use invalid sock pointer.
Reported-by: syzbot+b7f6f8c9303466e16c8a@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
net/bluetooth/l2cap_core.c | 3 +++
net/bluetooth/l2cap_sock.c | 13 +++++++++++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index aed025734d04..35a9534eb62d 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -6771,10 +6771,13 @@ static void l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm,
bacpy(&bt_cb(skb)->l2cap.bdaddr, &hcon->dst);
bt_cb(skb)->l2cap.psm = psm;
+ l2cap_chan_lock(chan);
if (!chan->ops->recv(chan, skb)) {
+ l2cap_chan_unlock(chan);
l2cap_chan_put(chan);
return;
}
+ l2cap_chan_unlock(chan);
drop:
l2cap_chan_put(chan);
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 6db60946c627..25091fb992a7 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1239,6 +1239,10 @@ static void l2cap_sock_kill(struct sock *sk)
BT_DBG("sk %p state %s", sk, state_to_string(sk->sk_state));
+ /* Sock is dead, so set chan data to NULL, avoid other task use invalid
+ * sock pointer.
+ */
+ l2cap_pi(sk)->chan->data = NULL;
/* Kill poor orphan */
l2cap_chan_put(l2cap_pi(sk)->chan);
@@ -1481,11 +1485,16 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
{
- struct sock *sk = chan->data;
- struct l2cap_pinfo *pi = l2cap_pi(sk);
+ struct sock *sk;
+ struct l2cap_pinfo *pi;
int err;
+ if (!chan->data)
+ return -ENXIO;
+
+ sk = chan->data;
lock_sock(sk);
+ pi = l2cap_pi(sk);
if (chan->mode == L2CAP_MODE_ERTM && !list_empty(&pi->rx_busy)) {
err = -ENOMEM;
--
2.43.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb
2024-06-07 6:02 [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb syzbot
` (6 preceding siblings ...)
2024-06-11 14:50 ` [PATCH] bluetooth/l2cap: sync sock recv cb and release Edward Adam Davis
@ 2024-06-15 1:25 ` Edward Adam Davis
2024-06-15 15:25 ` syzbot
2024-06-21 14:18 ` Edward Adam Davis
` (2 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Edward Adam Davis @ 2024-06-15 1:25 UTC (permalink / raw)
To: syzbot+b7f6f8c9303466e16c8a; +Cc: linux-kernel, syzkaller-bugs
please test null ptr defref in l2cap_sock_recv_cb
#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git cc8ed4d0a848
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 6db60946c627..d6c2394f0235 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1241,6 +1241,7 @@ static void l2cap_sock_kill(struct sock *sk)
/* Kill poor orphan */
+ l2cap_pi(sk)->chan->data = NULL;
l2cap_chan_put(l2cap_pi(sk)->chan);
sock_set_flag(sk, SOCK_DEAD);
sock_put(sk);
@@ -1413,6 +1414,7 @@ static int l2cap_sock_release(struct socket *sock)
l2cap_chan_hold(chan);
l2cap_chan_lock(chan);
+ printk("err: %d, sk refcnt: %u, %s\n", err, refcount_read(&sk->sk_refcnt), __func__);
sock_orphan(sk);
l2cap_sock_kill(sk);
@@ -1481,12 +1483,23 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
{
- struct sock *sk = chan->data;
- struct l2cap_pinfo *pi = l2cap_pi(sk);
+ struct sock *sk;
+ struct l2cap_pinfo *pi;
int err;
- lock_sock(sk);
+ l2cap_chan_hold(chan);
+ l2cap_chan_lock(chan);
+ sk = chan->data;
+
+ if (!sk) {
+ printk("%s\n", __func__);
+ l2cap_chan_unlock(chan);
+ l2cap_chan_put(chan);
+ return -ENXIO;
+ }
+ pi = l2cap_pi(sk);
+ lock_sock(sk);
if (chan->mode == L2CAP_MODE_ERTM && !list_empty(&pi->rx_busy)) {
err = -ENOMEM;
goto done;
@@ -1535,6 +1548,8 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
done:
release_sock(sk);
+ l2cap_chan_unlock(chan);
+ l2cap_chan_put(chan);
return err;
}
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb
2024-06-07 6:02 [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb syzbot
` (7 preceding siblings ...)
2024-06-15 1:25 ` [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb Edward Adam Davis
@ 2024-06-21 14:18 ` Edward Adam Davis
2024-06-22 1:25 ` syzbot
2024-06-21 14:56 ` Edward Adam Davis
2024-06-22 8:24 ` Hillf Danton
10 siblings, 1 reply; 33+ messages in thread
From: Edward Adam Davis @ 2024-06-21 14:18 UTC (permalink / raw)
To: syzbot+b7f6f8c9303466e16c8a; +Cc: linux-kernel, syzkaller-bugs
please test null ptr defref in l2cap_sock_recv_cb
#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git cc8ed4d0a848
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 6db60946c627..b72f5db97ccb 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1239,6 +1239,7 @@ static void l2cap_sock_kill(struct sock *sk)
BT_DBG("sk %p state %s", sk, state_to_string(sk->sk_state));
+ l2cap_pi(sk)->chan->data = NULL;
/* Kill poor orphan */
l2cap_chan_put(l2cap_pi(sk)->chan);
@@ -1481,10 +1482,15 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
{
- struct sock *sk = chan->data;
- struct l2cap_pinfo *pi = l2cap_pi(sk);
+ struct sock *sk;
+ struct l2cap_pinfo *pi;
int err;
+ sk = chan->data;
+ if (!sk)
+ return -ENXIO;
+
+ pi = l2cap_pi(sk);
lock_sock(sk);
if (chan->mode == L2CAP_MODE_ERTM && !list_empty(&pi->rx_busy)) {
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb
2024-06-21 14:18 ` Edward Adam Davis
@ 2024-06-22 1:25 ` syzbot
0 siblings, 0 replies; 33+ messages in thread
From: syzbot @ 2024-06-22 1:25 UTC (permalink / raw)
To: eadavis, linux-kernel, syzkaller-bugs
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue:
general protection fault in l2cap_sock_recv_cb
Bluetooth: hci0: command tx timeout
Oops: general protection fault, probably for non-canonical address 0xdffffc000000002e: 0000 [#1] PREEMPT SMP KASAN PTI
KASAN: null-ptr-deref in range [0x0000000000000170-0x0000000000000177]
CPU: 0 PID: 4480 Comm: kworker/u9:1 Not tainted 6.10.0-rc1-syzkaller-00267-gcc8ed4d0a848-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 06/07/2024
Workqueue: hci0 hci_rx_work
RIP: 0010:l2cap_publish_rx_avail net/bluetooth/l2cap_sock.c:1137 [inline]
RIP: 0010:l2cap_sock_recv_cb+0x1a0/0x520 net/bluetooth/l2cap_sock.c:1515
Code: df 80 3c 03 00 74 08 4c 89 ff e8 bb dd 7b f7 4d 8b 3f 49 8d bf 74 01 00 00 48 89 f8 48 c1 e8 03 49 bd 00 00 00 00 00 fc ff df <42> 0f b6 04 28 84 c0 0f 85 f3 02 00 00 41 8b 9f 74 01 00 00 49 81
RSP: 0018:ffffc9000d37f468 EFLAGS: 00010207
RAX: 000000000000002e RBX: 1ffff11005827494 RCX: ffff88802f763c00
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000174
RBP: ffff88802c13a000 R08: ffffffff89459100 R09: 1ffff1100582760c
R10: dffffc0000000000 R11: ffffed100582760d R12: 0000000000000000
R13: dffffc0000000000 R14: ffff888069fbd500 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff8880b9400000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000040 CR3: 000000001cb4e000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
l2cap_conless_channel net/bluetooth/l2cap_core.c:6780 [inline]
l2cap_recv_frame+0x8b6d/0x10670 net/bluetooth/l2cap_core.c:6833
hci_acldata_packet net/bluetooth/hci_core.c:3842 [inline]
hci_rx_work+0x50f/0xca0 net/bluetooth/hci_core.c:4079
process_one_work kernel/workqueue.c:3231 [inline]
process_scheduled_works+0xa2c/0x1830 kernel/workqueue.c:3312
worker_thread+0x86d/0xd70 kernel/workqueue.c:3393
kthread+0x2f0/0x390 kernel/kthread.c:389
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:l2cap_publish_rx_avail net/bluetooth/l2cap_sock.c:1137 [inline]
RIP: 0010:l2cap_sock_recv_cb+0x1a0/0x520 net/bluetooth/l2cap_sock.c:1515
Code: df 80 3c 03 00 74 08 4c 89 ff e8 bb dd 7b f7 4d 8b 3f 49 8d bf 74 01 00 00 48 89 f8 48 c1 e8 03 49 bd 00 00 00 00 00 fc ff df <42> 0f b6 04 28 84 c0 0f 85 f3 02 00 00 41 8b 9f 74 01 00 00 49 81
RSP: 0018:ffffc9000d37f468 EFLAGS: 00010207
RAX: 000000000000002e RBX: 1ffff11005827494 RCX: ffff88802f763c00
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000174
RBP: ffff88802c13a000 R08: ffffffff89459100 R09: 1ffff1100582760c
R10: dffffc0000000000 R11: ffffed100582760d R12: 0000000000000000
R13: dffffc0000000000 R14: ffff888069fbd500 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff8880b9500000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007feea42b7ffc CR3: 000000007cc74000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
----------------
Code disassembly (best guess):
0: df 80 3c 03 00 74 filds 0x7400033c(%rax)
6: 08 4c 89 ff or %cl,-0x1(%rcx,%rcx,4)
a: e8 bb dd 7b f7 call 0xf77bddca
f: 4d 8b 3f mov (%r15),%r15
12: 49 8d bf 74 01 00 00 lea 0x174(%r15),%rdi
19: 48 89 f8 mov %rdi,%rax
1c: 48 c1 e8 03 shr $0x3,%rax
20: 49 bd 00 00 00 00 00 movabs $0xdffffc0000000000,%r13
27: fc ff df
* 2a: 42 0f b6 04 28 movzbl (%rax,%r13,1),%eax <-- trapping instruction
2f: 84 c0 test %al,%al
31: 0f 85 f3 02 00 00 jne 0x32a
37: 41 8b 9f 74 01 00 00 mov 0x174(%r15),%ebx
3e: 49 rex.WB
3f: 81 .byte 0x81
Tested on:
commit: cc8ed4d0 Merge tag 'drm-fixes-2024-06-01' of https://g..
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=115864d6980000
kernel config: https://syzkaller.appspot.com/x/.config?x=47d282ddffae809f
dashboard link: https://syzkaller.appspot.com/bug?extid=b7f6f8c9303466e16c8a
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=1106883e980000
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb
2024-06-07 6:02 [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb syzbot
` (8 preceding siblings ...)
2024-06-21 14:18 ` Edward Adam Davis
@ 2024-06-21 14:56 ` Edward Adam Davis
2024-06-22 3:27 ` syzbot
2024-06-22 8:24 ` Hillf Danton
10 siblings, 1 reply; 33+ messages in thread
From: Edward Adam Davis @ 2024-06-21 14:56 UTC (permalink / raw)
To: syzbot+b7f6f8c9303466e16c8a; +Cc: linux-kernel, syzkaller-bugs
please test null ptr defref in l2cap_sock_recv_cb
#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git cc8ed4d0a848
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 6db60946c627..c0fe74a6b45e 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -47,6 +47,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent);
static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock,
int proto, gfp_t prio, int kern);
static void l2cap_sock_cleanup_listen(struct sock *parent);
+static DEFINE_MUTEX(chan_data_lock);
bool l2cap_is_socket(struct socket *sock)
{
@@ -1239,6 +1240,9 @@ static void l2cap_sock_kill(struct sock *sk)
BT_DBG("sk %p state %s", sk, state_to_string(sk->sk_state));
+ mutex_lock(&chan_data_lock);
+ l2cap_pi(sk)->chan->data = NULL;
+ mutex_unlock(&chan_data_lock);
/* Kill poor orphan */
l2cap_chan_put(l2cap_pi(sk)->chan);
@@ -1481,10 +1485,18 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
{
- struct sock *sk = chan->data;
- struct l2cap_pinfo *pi = l2cap_pi(sk);
+ struct sock *sk;
+ struct l2cap_pinfo *pi;
int err;
+ mutex_lock(&chan_data_lock);
+ sk = chan->data;
+ if (!sk) {
+ mutex_unlock(&chan_data_lock);
+ return -ENXIO;
+ }
+
+ pi = l2cap_pi(sk);
lock_sock(sk);
if (chan->mode == L2CAP_MODE_ERTM && !list_empty(&pi->rx_busy)) {
@@ -1535,6 +1547,7 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
done:
release_sock(sk);
+ mutex_unlock(&chan_data_lock);
return err;
}
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb
2024-06-07 6:02 [syzbot] [bluetooth?] general protection fault in l2cap_sock_recv_cb syzbot
` (9 preceding siblings ...)
2024-06-21 14:56 ` Edward Adam Davis
@ 2024-06-22 8:24 ` Hillf Danton
2024-06-22 9:10 ` syzbot
10 siblings, 1 reply; 33+ messages in thread
From: Hillf Danton @ 2024-06-22 8:24 UTC (permalink / raw)
To: syzbot+b7f6f8c9303466e16c8a; +Cc: linux-kernel, syzkaller-bugs
#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git cc8ed4d0a848
--- x/net/bluetooth/l2cap_core.c
+++ y/net/bluetooth/l2cap_core.c
@@ -6756,6 +6756,7 @@ static void l2cap_conless_channel(struct
{
struct hci_conn *hcon = conn->hcon;
struct l2cap_chan *chan;
+ int rc;
if (hcon->type != ACL_LINK)
goto free_skb;
@@ -6767,6 +6768,8 @@ static void l2cap_conless_channel(struct
BT_DBG("chan %p, len %d", chan, skb->len);
+ l2cap_chan_lock(chan);
+
if (chan->state != BT_BOUND && chan->state != BT_CONNECTED)
goto drop;
@@ -6777,15 +6780,15 @@ static void l2cap_conless_channel(struct
bacpy(&bt_cb(skb)->l2cap.bdaddr, &hcon->dst);
bt_cb(skb)->l2cap.psm = psm;
- if (!chan->ops->recv(chan, skb)) {
- l2cap_chan_put(chan);
- return;
- }
+ rc = chan->ops->recv(chan, skb);
drop:
+ l2cap_chan_unlock(chan);
l2cap_chan_put(chan);
+ if (rc) {
free_skb:
- kfree_skb(skb);
+ kfree_skb(skb);
+ }
}
static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb)
--- x/net/bluetooth/l2cap_sock.c
+++ y/net/bluetooth/l2cap_sock.c
@@ -1237,6 +1237,7 @@ static void l2cap_sock_kill(struct sock
if (!sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket)
return;
+ l2cap_pi(sk)->chan->data = NULL;
BT_DBG("sk %p state %s", sk, state_to_string(sk->sk_state));
/* Kill poor orphan */
@@ -1481,11 +1482,16 @@ static struct l2cap_chan *l2cap_sock_new
static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
{
- struct sock *sk = chan->data;
- struct l2cap_pinfo *pi = l2cap_pi(sk);
+ struct sock *sk;
+ struct l2cap_pinfo *pi;
int err;
+ if (!chan->data)
+ return -ENXIO;
+
+ sk = chan->data;
lock_sock(sk);
+ pi = l2cap_pi(sk);
if (chan->mode == L2CAP_MODE_ERTM && !list_empty(&pi->rx_busy)) {
err = -ENOMEM;
--
^ permalink raw reply [flat|nested] 33+ messages in thread