* [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
@ 2024-09-03 13:13 syzbot
2024-09-03 13:43 ` Edward Adam Davis
` (9 more replies)
0 siblings, 10 replies; 36+ messages in thread
From: syzbot @ 2024-09-03 13:13 UTC (permalink / raw)
To: davem, edumazet, geliang, kuba, linux-kernel, martineau, matttbe,
mptcp, netdev, pabeni, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: 1934261d8974 Merge tag 'input-for-v6.11-rc5' of git://git...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=146be1db980000
kernel config: https://syzkaller.appspot.com/x/.config?x=996585887acdadb3
dashboard link: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
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=11229d09980000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/d02c28e8dbf3/disk-1934261d.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/464d0e233034/vmlinux-1934261d.xz
kernel image: https://storage.googleapis.com/syzbot-assets/8735d78fb16a/bzImage-1934261d.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: slab-use-after-free in lock_release+0x151/0xa30 kernel/locking/lockdep.c:5772
Read of size 8 at addr ffff888076a77758 by task kworker/0:0/8
CPU: 0 UID: 0 PID: 8 Comm: kworker/0:0 Not tainted 6.11.0-rc5-syzkaller-00219-g1934261d8974 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/06/2024
Workqueue: events mptcp_worker
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:93 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:119
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
lock_release+0x151/0xa30 kernel/locking/lockdep.c:5772
__timer_delete_sync+0x157/0x310 kernel/time/timer.c:1648
del_timer_sync include/linux/timer.h:185 [inline]
sk_stop_timer_sync+0x1c/0x90 net/core/sock.c:3454
mptcp_pm_del_add_timer+0x18d/0x250 net/mptcp/pm_netlink.c:345
mptcp_incoming_options+0x158d/0x2570 net/mptcp/options.c:1166
tcp_data_queue+0xf5/0x76c0 net/ipv4/tcp_input.c:5206
tcp_rcv_established+0xfba/0x2020 net/ipv4/tcp_input.c:6230
tcp_v4_do_rcv+0x96d/0xc70 net/ipv4/tcp_ipv4.c:1911
tcp_v4_rcv+0x2dc0/0x37f0 net/ipv4/tcp_ipv4.c:2346
ip_protocol_deliver_rcu+0x22e/0x440 net/ipv4/ip_input.c:205
ip_local_deliver_finish+0x341/0x5f0 net/ipv4/ip_input.c:233
NF_HOOK+0x3a4/0x450 include/linux/netfilter.h:314
NF_HOOK+0x3a4/0x450 include/linux/netfilter.h:314
__netif_receive_skb_one_core net/core/dev.c:5661 [inline]
__netif_receive_skb+0x2bf/0x650 net/core/dev.c:5775
process_backlog+0x662/0x15b0 net/core/dev.c:6108
__napi_poll+0xcb/0x490 net/core/dev.c:6772
napi_poll net/core/dev.c:6841 [inline]
net_rx_action+0x89b/0x1240 net/core/dev.c:6963
handle_softirqs+0x2c4/0x970 kernel/softirq.c:554
do_softirq+0x11b/0x1e0 kernel/softirq.c:455
</IRQ>
<TASK>
__local_bh_enable_ip+0x1bb/0x200 kernel/softirq.c:382
mptcp_pm_send_ack net/mptcp/pm_netlink.c:496 [inline]
mptcp_pm_nl_addr_send_ack+0x3ec/0x4e0 net/mptcp/pm_netlink.c:785
mptcp_pm_nl_work+0x18e8/0x1fc0 net/mptcp/pm_netlink.c:924
mptcp_worker+0x12f/0x1440 net/mptcp/protocol.c:2759
process_one_work kernel/workqueue.c:3231 [inline]
process_scheduled_works+0xa2c/0x1830 kernel/workqueue.c:3312
worker_thread+0x86d/0xd10 kernel/workqueue.c:3389
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 5379:
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]
__kmalloc_cache_noprof+0x19c/0x2c0 mm/slub.c:4189
kmalloc_noprof include/linux/slab.h:681 [inline]
mptcp_pm_alloc_anno_list+0x14e/0x390 net/mptcp/pm_netlink.c:370
mptcp_pm_create_subflow_or_signal_addr+0x1920/0x22c0 net/mptcp/pm_netlink.c:586
mptcp_nl_add_subflow_or_signal_addr net/mptcp/pm_netlink.c:1356 [inline]
mptcp_pm_nl_add_addr_doit+0x1276/0x1b80 net/mptcp/pm_netlink.c:1428
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:2550
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
netlink_unicast_kernel net/netlink/af_netlink.c:1331 [inline]
netlink_unicast+0x7f6/0x990 net/netlink/af_netlink.c:1357
netlink_sendmsg+0x8e4/0xcb0 net/netlink/af_netlink.c:1901
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0x221/0x270 net/socket.c:745
____sys_sendmsg+0x525/0x7d0 net/socket.c:2597
___sys_sendmsg net/socket.c:2651 [inline]
__sys_sendmsg+0x2b0/0x3a0 net/socket.c:2680
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 5379:
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:2252 [inline]
slab_free mm/slub.c:4473 [inline]
kfree+0x149/0x360 mm/slub.c:4594
remove_anno_list_by_saddr+0x156/0x190 net/mptcp/pm_netlink.c:1466
mptcp_pm_remove_addrs_and_subflows net/mptcp/pm_netlink.c:1687 [inline]
mptcp_nl_remove_addrs_list net/mptcp/pm_netlink.c:1718 [inline]
mptcp_pm_nl_flush_addrs_doit+0x664/0xd60 net/mptcp/pm_netlink.c:1759
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:2550
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
netlink_unicast_kernel net/netlink/af_netlink.c:1331 [inline]
netlink_unicast+0x7f6/0x990 net/netlink/af_netlink.c:1357
netlink_sendmsg+0x8e4/0xcb0 net/netlink/af_netlink.c:1901
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0x221/0x270 net/socket.c:745
____sys_sendmsg+0x525/0x7d0 net/socket.c:2597
___sys_sendmsg net/socket.c:2651 [inline]
__sys_sendmsg+0x2b0/0x3a0 net/socket.c:2680
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 ffff888076a77700
which belongs to the cache kmalloc-192 of size 192
The buggy address is located 88 bytes inside of
freed 192-byte region [ffff888076a77700, ffff888076a777c0)
The buggy address belongs to the physical page:
page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x76a77
flags: 0xfff00000000000(node=0|zone=1|lastcpupid=0x7ff)
page_type: 0xfdffffff(slab)
raw: 00fff00000000000 ffff88801ac413c0 dead000000000122 0000000000000000
raw: 0000000000000000 0000000080100010 00000001fdffffff 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 0, migratetype Unmovable, gfp_mask 0x352800(GFP_NOWAIT|__GFP_NORETRY|__GFP_COMP|__GFP_HARDWALL|__GFP_THISNODE), pid 5373, tgid 5372 (syz.0.20), ts 69219069347, free_ts 69201611728
set_page_owner include/linux/page_owner.h:32 [inline]
post_alloc_hook+0x1f3/0x230 mm/page_alloc.c:1493
prep_new_page mm/page_alloc.c:1501 [inline]
get_page_from_freelist+0x2e4c/0x2f10 mm/page_alloc.c:3439
__alloc_pages_noprof+0x256/0x6c0 mm/page_alloc.c:4695
__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:2321
allocate_slab+0x5a/0x2f0 mm/slub.c:2484
new_slab mm/slub.c:2537 [inline]
___slab_alloc+0xcd1/0x14b0 mm/slub.c:3723
__slab_alloc+0x58/0xa0 mm/slub.c:3813
__slab_alloc_node mm/slub.c:3866 [inline]
slab_alloc_node mm/slub.c:4025 [inline]
__do_kmalloc_node mm/slub.c:4157 [inline]
__kmalloc_node_noprof+0x286/0x440 mm/slub.c:4164
kmalloc_array_node_noprof include/linux/slab.h:788 [inline]
alloc_slab_obj_exts mm/slub.c:1976 [inline]
account_slab mm/slub.c:2447 [inline]
allocate_slab+0xb6/0x2f0 mm/slub.c:2502
new_slab mm/slub.c:2537 [inline]
___slab_alloc+0xcd1/0x14b0 mm/slub.c:3723
__slab_alloc+0x58/0xa0 mm/slub.c:3813
__slab_alloc_node mm/slub.c:3866 [inline]
slab_alloc_node mm/slub.c:4025 [inline]
kmem_cache_alloc_noprof+0x1c1/0x2a0 mm/slub.c:4044
reqsk_alloc_noprof net/ipv4/inet_connection_sock.c:920 [inline]
inet_reqsk_alloc+0xa8/0x800 net/ipv4/inet_connection_sock.c:951
tcp_conn_request+0x252/0x34a0 net/ipv4/tcp_input.c:7179
tcp_rcv_state_process+0x1bd7/0x4570 net/ipv4/tcp_input.c:6716
tcp_v4_do_rcv+0x77d/0xc70 net/ipv4/tcp_ipv4.c:1934
page last free pid 5372 tgid 5372 stack trace:
reset_page_owner include/linux/page_owner.h:25 [inline]
free_pages_prepare mm/page_alloc.c:1094 [inline]
free_unref_page+0xd22/0xea0 mm/page_alloc.c:2612
__slab_free+0x31b/0x3d0 mm/slub.c:4384
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:3988 [inline]
slab_alloc_node mm/slub.c:4037 [inline]
__do_kmalloc_node mm/slub.c:4157 [inline]
__kmalloc_noprof+0x1a6/0x400 mm/slub.c:4170
kmalloc_noprof include/linux/slab.h:685 [inline]
kzalloc_noprof include/linux/slab.h:807 [inline]
tomoyo_encode2 security/tomoyo/realpath.c:45 [inline]
tomoyo_encode+0x26f/0x540 security/tomoyo/realpath.c:80
tomoyo_path_perm+0x3ca/0x740 security/tomoyo/file.c:831
tomoyo_path_symlink+0xde/0x120 security/tomoyo/tomoyo.c:212
security_path_symlink+0xe3/0x140 security/security.c:1876
do_symlinkat+0x136/0x3a0 fs/namei.c:4592
__do_sys_symlinkat fs/namei.c:4610 [inline]
__se_sys_symlinkat fs/namei.c:4607 [inline]
__x64_sys_symlinkat+0x95/0xb0 fs/namei.c:4607
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:
ffff888076a77600: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff888076a77680: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
>ffff888076a77700: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff888076a77780: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
ffff888076a77800: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================
---
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.
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] 36+ messages in thread
* Re: [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
2024-09-03 13:13 [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync syzbot
@ 2024-09-03 13:43 ` Edward Adam Davis
2024-09-03 14:09 ` syzbot
2024-09-03 13:52 ` Edward Adam Davis
` (8 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Edward Adam Davis @ 2024-09-03 13:43 UTC (permalink / raw)
To: syzbot+f3a31fb909db9b2a5c4d; +Cc: linux-kernel
entry need to be protected by pm.lock.
#syz test
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 3e4ad801786f..d28bf0c9ad66 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -336,11 +336,12 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk,
entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
if (entry && (!check_id || entry->addr.id == addr->id))
entry->retrans_times = ADD_ADDR_RETRANS_MAX;
- spin_unlock_bh(&msk->pm.lock);
if (entry && (!check_id || entry->addr.id == addr->id))
sk_stop_timer_sync(sk, &entry->add_timer);
+ spin_unlock_bh(&msk->pm.lock);
+
return entry;
}
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
2024-09-03 13:13 [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync syzbot
2024-09-03 13:43 ` Edward Adam Davis
@ 2024-09-03 13:52 ` Edward Adam Davis
2024-09-03 14:24 ` syzbot
2024-09-03 15:09 ` [PATCH] mptcp: pm: Fix uaf " Edward Adam Davis
` (7 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Edward Adam Davis @ 2024-09-03 13:52 UTC (permalink / raw)
To: syzbot+f3a31fb909db9b2a5c4d; +Cc: linux-kernel
entry need to be protected by pm.lock.
#syz test
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 3e4ad801786f..06a6846e316c 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -336,11 +336,12 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk,
entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
if (entry && (!check_id || entry->addr.id == addr->id))
entry->retrans_times = ADD_ADDR_RETRANS_MAX;
- spin_unlock_bh(&msk->pm.lock);
if (entry && (!check_id || entry->addr.id == addr->id))
sk_stop_timer_sync(sk, &entry->add_timer);
+ spin_unlock_bh(&msk->pm.lock);
+
return entry;
}
@@ -1637,6 +1638,7 @@ static void mptcp_pm_remove_addrs_and_subflows(struct mptcp_sock *msk,
struct mptcp_rm_list alist = { .nr = 0 }, slist = { .nr = 0 };
struct mptcp_pm_addr_entry *entry;
+ spin_lock_bh(&msk->pm.lock);
list_for_each_entry(entry, rm_list, list) {
if (slist.nr < MPTCP_RM_IDS_MAX &&
lookup_subflow_by_saddr(&msk->conn_list, &entry->addr))
@@ -1647,7 +1649,6 @@ static void mptcp_pm_remove_addrs_and_subflows(struct mptcp_sock *msk,
alist.ids[alist.nr++] = entry->addr.id;
}
- spin_lock_bh(&msk->pm.lock);
if (alist.nr) {
msk->pm.add_addr_signaled -= alist.nr;
mptcp_pm_remove_addr(msk, &alist);
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
2024-09-03 13:43 ` Edward Adam Davis
@ 2024-09-03 14:09 ` syzbot
0 siblings, 0 replies; 36+ messages in thread
From: syzbot @ 2024-09-03 14:09 UTC (permalink / raw)
To: eadavis, linux-kernel, syzkaller-bugs
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Tested on:
commit: 67784a74 Merge tag 'ata-6.11-rc7' of git://git.kernel...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=11b2cd43980000
kernel config: https://syzkaller.appspot.com/x/.config?x=660f6eb11f9c7dc5
dashboard link: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=1512f0fb980000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
2024-09-03 13:52 ` Edward Adam Davis
@ 2024-09-03 14:24 ` syzbot
0 siblings, 0 replies; 36+ messages in thread
From: syzbot @ 2024-09-03 14:24 UTC (permalink / raw)
To: eadavis, linux-kernel, syzkaller-bugs
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue:
possible deadlock in remove_anno_list_by_saddr
============================================
WARNING: possible recursive locking detected
6.11.0-rc6-syzkaller-00019-g67784a74e258-dirty #0 Not tainted
--------------------------------------------
syz.1.16/6115 is trying to acquire lock:
ffff888028c116a0 (&msk->pm.lock){+.-.}-{2:2}, at: spin_lock_bh include/linux/spinlock.h:356 [inline]
ffff888028c116a0 (&msk->pm.lock){+.-.}-{2:2}, at: mptcp_pm_del_add_timer net/mptcp/pm_netlink.c:338 [inline]
ffff888028c116a0 (&msk->pm.lock){+.-.}-{2:2}, at: remove_anno_list_by_saddr+0x24/0x190 net/mptcp/pm_netlink.c:1464
but task is already holding lock:
ffff888028c116a0 (&msk->pm.lock){+.-.}-{2:2}, at: spin_lock_bh include/linux/spinlock.h:356 [inline]
ffff888028c116a0 (&msk->pm.lock){+.-.}-{2:2}, at: mptcp_pm_remove_addrs_and_subflows net/mptcp/pm_netlink.c:1682 [inline]
ffff888028c116a0 (&msk->pm.lock){+.-.}-{2:2}, at: mptcp_nl_remove_addrs_list net/mptcp/pm_netlink.c:1719 [inline]
ffff888028c116a0 (&msk->pm.lock){+.-.}-{2:2}, at: mptcp_pm_nl_flush_addrs_doit+0x509/0xd80 net/mptcp/pm_netlink.c:1760
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&msk->pm.lock);
lock(&msk->pm.lock);
*** DEADLOCK ***
May be due to missing lock nesting notation
4 locks held by syz.1.16/6115:
#0: ffffffff8fcf14b0 (cb_lock){++++}-{3:3}, at: genl_rcv+0x19/0x40 net/netlink/genetlink.c:1218
#1: ffffffff8fcf1368 (genl_mutex){+.+.}-{3:3}, at: genl_lock net/netlink/genetlink.c:35 [inline]
#1: ffffffff8fcf1368 (genl_mutex){+.+.}-{3:3}, at: genl_op_lock net/netlink/genetlink.c:60 [inline]
#1: ffffffff8fcf1368 (genl_mutex){+.+.}-{3:3}, at: genl_rcv_msg+0x121/0xec0 net/netlink/genetlink.c:1209
#2: ffff888028c10e98 (sk_lock-AF_INET6){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1607 [inline]
#2: ffff888028c10e98 (sk_lock-AF_INET6){+.+.}-{0:0}, at: mptcp_nl_remove_addrs_list net/mptcp/pm_netlink.c:1718 [inline]
#2: ffff888028c10e98 (sk_lock-AF_INET6){+.+.}-{0:0}, at: mptcp_pm_nl_flush_addrs_doit+0x4d0/0xd80 net/mptcp/pm_netlink.c:1760
#3: ffff888028c116a0 (&msk->pm.lock){+.-.}-{2:2}, at: spin_lock_bh include/linux/spinlock.h:356 [inline]
#3: ffff888028c116a0 (&msk->pm.lock){+.-.}-{2:2}, at: mptcp_pm_remove_addrs_and_subflows net/mptcp/pm_netlink.c:1682 [inline]
#3: ffff888028c116a0 (&msk->pm.lock){+.-.}-{2:2}, at: mptcp_nl_remove_addrs_list net/mptcp/pm_netlink.c:1719 [inline]
#3: ffff888028c116a0 (&msk->pm.lock){+.-.}-{2:2}, at: mptcp_pm_nl_flush_addrs_doit+0x509/0xd80 net/mptcp/pm_netlink.c:1760
stack backtrace:
CPU: 0 UID: 0 PID: 6115 Comm: syz.1.16 Not tainted 6.11.0-rc6-syzkaller-00019-g67784a74e258-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/06/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:93 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:119
check_deadlock kernel/locking/lockdep.c:3061 [inline]
validate_chain+0x15d3/0x5900 kernel/locking/lockdep.c:3855
__lock_acquire+0x137a/0x2040 kernel/locking/lockdep.c:5142
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5759
__raw_spin_lock_bh include/linux/spinlock_api_smp.h:126 [inline]
_raw_spin_lock_bh+0x35/0x50 kernel/locking/spinlock.c:178
spin_lock_bh include/linux/spinlock.h:356 [inline]
mptcp_pm_del_add_timer net/mptcp/pm_netlink.c:338 [inline]
remove_anno_list_by_saddr+0x24/0x190 net/mptcp/pm_netlink.c:1464
mptcp_pm_remove_addrs_and_subflows net/mptcp/pm_netlink.c:1689 [inline]
mptcp_nl_remove_addrs_list net/mptcp/pm_netlink.c:1719 [inline]
mptcp_pm_nl_flush_addrs_doit+0x689/0xd80 net/mptcp/pm_netlink.c:1760
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:2550
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
netlink_unicast_kernel net/netlink/af_netlink.c:1331 [inline]
netlink_unicast+0x7f6/0x990 net/netlink/af_netlink.c:1357
netlink_sendmsg+0x8e4/0xcb0 net/netlink/af_netlink.c:1901
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0x221/0x270 net/socket.c:745
____sys_sendmsg+0x525/0x7d0 net/socket.c:2597
___sys_sendmsg net/socket.c:2651 [inline]
__sys_sendmsg+0x2b0/0x3a0 net/socket.c:2680
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
RIP: 0033:0x7fb336579eb9
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 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 a8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fb3372d5038 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007fb336715f80 RCX: 00007fb336579eb9
RDX: 0000000001000000 RSI: 0000000020000300 RDI: 0000000000000003
RBP: 00007fb3365e793e R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 0000000000000000 R14: 00007fb336715f80 R15: 00007fb33683fa28
</TASK>
Tested on:
commit: 67784a74 Merge tag 'ata-6.11-rc7' of git://git.kernel...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=13ab2a8f980000
kernel config: https://syzkaller.appspot.com/x/.config?x=660f6eb11f9c7dc5
dashboard link: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=1377f28f980000
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-03 13:13 [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync syzbot
2024-09-03 13:43 ` Edward Adam Davis
2024-09-03 13:52 ` Edward Adam Davis
@ 2024-09-03 15:09 ` Edward Adam Davis
2024-09-03 15:18 ` Eric Dumazet
2024-09-04 0:20 ` [syzbot] [mptcp?] KASAN: slab-use-after-free Read " Edward Adam Davis
` (6 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Edward Adam Davis @ 2024-09-03 15:09 UTC (permalink / raw)
To: syzbot+f3a31fb909db9b2a5c4d
Cc: davem, edumazet, geliang, kuba, linux-kernel, martineau, matttbe,
mptcp, netdev, pabeni, syzkaller-bugs
There are two paths to access mptcp_pm_del_add_timer, result in a race
condition:
CPU1 CPU2
==== ====
net_rx_action
napi_poll netlink_sendmsg
__napi_poll netlink_unicast
process_backlog netlink_unicast_kernel
__netif_receive_skb genl_rcv
__netif_receive_skb_one_core netlink_rcv_skb
NF_HOOK genl_rcv_msg
ip_local_deliver_finish genl_family_rcv_msg
ip_protocol_deliver_rcu genl_family_rcv_msg_doit
tcp_v4_rcv mptcp_pm_nl_flush_addrs_doit
tcp_v4_do_rcv mptcp_nl_remove_addrs_list
tcp_rcv_established mptcp_pm_remove_addrs_and_subflows
tcp_data_queue remove_anno_list_by_saddr
mptcp_incoming_options mptcp_pm_del_add_timer
mptcp_pm_del_add_timer kfree(entry)
In remove_anno_list_by_saddr(running on CPU2), after leaving the critical
zone protected by "pm.lock", the entry will be released, which leads to the
occurrence of uaf in the mptcp_pm_del_add_timer(running on CPU1).
Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
net/mptcp/pm_netlink.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 3e4ad801786f..d28bf0c9ad66 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -336,11 +336,12 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk,
entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
if (entry && (!check_id || entry->addr.id == addr->id))
entry->retrans_times = ADD_ADDR_RETRANS_MAX;
- spin_unlock_bh(&msk->pm.lock);
if (entry && (!check_id || entry->addr.id == addr->id))
sk_stop_timer_sync(sk, &entry->add_timer);
+ spin_unlock_bh(&msk->pm.lock);
+
return entry;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-03 15:09 ` [PATCH] mptcp: pm: Fix uaf " Edward Adam Davis
@ 2024-09-03 15:18 ` Eric Dumazet
2024-09-04 0:52 ` Edward Adam Davis
0 siblings, 1 reply; 36+ messages in thread
From: Eric Dumazet @ 2024-09-03 15:18 UTC (permalink / raw)
To: Edward Adam Davis
Cc: syzbot+f3a31fb909db9b2a5c4d, davem, geliang, kuba, linux-kernel,
martineau, matttbe, mptcp, netdev, pabeni, syzkaller-bugs
On Tue, Sep 3, 2024 at 5:10 PM Edward Adam Davis <eadavis@qq.com> wrote:
>
> There are two paths to access mptcp_pm_del_add_timer, result in a race
> condition:
>
> CPU1 CPU2
> ==== ====
> net_rx_action
> napi_poll netlink_sendmsg
> __napi_poll netlink_unicast
> process_backlog netlink_unicast_kernel
> __netif_receive_skb genl_rcv
> __netif_receive_skb_one_core netlink_rcv_skb
> NF_HOOK genl_rcv_msg
> ip_local_deliver_finish genl_family_rcv_msg
> ip_protocol_deliver_rcu genl_family_rcv_msg_doit
> tcp_v4_rcv mptcp_pm_nl_flush_addrs_doit
> tcp_v4_do_rcv mptcp_nl_remove_addrs_list
> tcp_rcv_established mptcp_pm_remove_addrs_and_subflows
> tcp_data_queue remove_anno_list_by_saddr
> mptcp_incoming_options mptcp_pm_del_add_timer
> mptcp_pm_del_add_timer kfree(entry)
>
> In remove_anno_list_by_saddr(running on CPU2), after leaving the critical
> zone protected by "pm.lock", the entry will be released, which leads to the
> occurrence of uaf in the mptcp_pm_del_add_timer(running on CPU1).
>
> Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> net/mptcp/pm_netlink.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 3e4ad801786f..d28bf0c9ad66 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -336,11 +336,12 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk,
> entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
> if (entry && (!check_id || entry->addr.id == addr->id))
> entry->retrans_times = ADD_ADDR_RETRANS_MAX;
> - spin_unlock_bh(&msk->pm.lock);
>
> if (entry && (!check_id || entry->addr.id == addr->id))
> sk_stop_timer_sync(sk, &entry->add_timer);
>
> + spin_unlock_bh(&msk->pm.lock);
mptcp_pm_add_timer() needs to lock msk->pm.lock
Your patch might add a deadlock, because sk_stop_timer_sync() is
calling del_timer_sync()
What is preventing this ?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
2024-09-03 13:13 [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync syzbot
` (2 preceding siblings ...)
2024-09-03 15:09 ` [PATCH] mptcp: pm: Fix uaf " Edward Adam Davis
@ 2024-09-04 0:20 ` Edward Adam Davis
2024-09-04 0:42 ` syzbot
2024-09-04 17:05 ` [PATCH net] mptcp: pm: Fix uaf " Matthieu Baerts (NGI0)
` (5 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Edward Adam Davis @ 2024-09-04 0:20 UTC (permalink / raw)
To: syzbot+f3a31fb909db9b2a5c4d; +Cc: linux-kernel
entry need to be protected by pm.lock.
#syz test
Upstream-Status: Pending
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 3e4ad801786f..d4cbf7dcf983 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1430,8 +1430,10 @@ static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
entry = mptcp_pm_del_add_timer(msk, addr, false);
if (entry) {
+ spin_lock_bh(&msk->pm.lock);
list_del(&entry->list);
kfree(entry);
+ spin_unlock_bh(&msk->pm.lock);
return true;
}
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
2024-09-04 0:20 ` [syzbot] [mptcp?] KASAN: slab-use-after-free Read " Edward Adam Davis
@ 2024-09-04 0:42 ` syzbot
0 siblings, 0 replies; 36+ messages in thread
From: syzbot @ 2024-09-04 0:42 UTC (permalink / raw)
To: eadavis, linux-kernel, syzkaller-bugs
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Tested on:
commit: 88fac175 Merge tag 'fuse-fixes-6.11-rc7' of git://git...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=119dcd63980000
kernel config: https://syzkaller.appspot.com/x/.config?x=660f6eb11f9c7dc5
dashboard link: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=124282ab980000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-03 15:18 ` Eric Dumazet
@ 2024-09-04 0:52 ` Edward Adam Davis
2024-09-04 1:01 ` [PATCH V2] " Edward Adam Davis
0 siblings, 1 reply; 36+ messages in thread
From: Edward Adam Davis @ 2024-09-04 0:52 UTC (permalink / raw)
To: edumazet
Cc: davem, eadavis, geliang, kuba, linux-kernel, martineau, matttbe,
mptcp, netdev, pabeni, syzbot+f3a31fb909db9b2a5c4d,
syzkaller-bugs
On Tue, 3 Sep 2024 17:18:42 +0200, Eric Dumazet <edumazet@google.com> wrote:
>On Tue, Sep 3, 2024 at 5:10 PM Edward Adam Davis <eadavis@qq.com> wrote:
>>
>> There are two paths to access mptcp_pm_del_add_timer, result in a race
>> condition:
>>
>> CPU1 CPU2
>> ==== ====
>> net_rx_action
>> napi_poll netlink_sendmsg
>> __napi_poll netlink_unicast
>> process_backlog netlink_unicast_kernel
>> __netif_receive_skb genl_rcv
>> __netif_receive_skb_one_core netlink_rcv_skb
>> NF_HOOK genl_rcv_msg
>> ip_local_deliver_finish genl_family_rcv_msg
>> ip_protocol_deliver_rcu genl_family_rcv_msg_doit
>> tcp_v4_rcv mptcp_pm_nl_flush_addrs_doit
>> tcp_v4_do_rcv mptcp_nl_remove_addrs_list
>> tcp_rcv_established mptcp_pm_remove_addrs_and_subflows
>> tcp_data_queue remove_anno_list_by_saddr
>> mptcp_incoming_options mptcp_pm_del_add_timer
>> mptcp_pm_del_add_timer kfree(entry)
>>
>> In remove_anno_list_by_saddr(running on CPU2), after leaving the critical
>> zone protected by "pm.lock", the entry will be released, which leads to the
>> occurrence of uaf in the mptcp_pm_del_add_timer(running on CPU1).
>>
>> Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
>> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
>> ---
>> net/mptcp/pm_netlink.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>> index 3e4ad801786f..d28bf0c9ad66 100644
>> --- a/net/mptcp/pm_netlink.c
>> +++ b/net/mptcp/pm_netlink.c
>> @@ -336,11 +336,12 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk,
>> entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
>> if (entry && (!check_id || entry->addr.id == addr->id))
>> entry->retrans_times = ADD_ADDR_RETRANS_MAX;
>> - spin_unlock_bh(&msk->pm.lock);
>>
>> if (entry && (!check_id || entry->addr.id == addr->id))
>> sk_stop_timer_sync(sk, &entry->add_timer);
>>
>> + spin_unlock_bh(&msk->pm.lock);
>
>
>mptcp_pm_add_timer() needs to lock msk->pm.lock
>
>Your patch might add a deadlock, because sk_stop_timer_sync() is
Got it. MPTCP CI reported it.
>calling del_timer_sync()
>
>What is preventing this ?
I will change the strategy for protecting entry and use pm.lock to
synchronize when it is released in remove_anno_list_by_saddr.
Link: https://syzkaller.appspot.com/text?tag=Patch&x=124282ab980000
BR,
Edward
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH V2] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-04 0:52 ` Edward Adam Davis
@ 2024-09-04 1:01 ` Edward Adam Davis
2024-09-04 20:39 ` Matthieu Baerts
0 siblings, 1 reply; 36+ messages in thread
From: Edward Adam Davis @ 2024-09-04 1:01 UTC (permalink / raw)
To: eadavis
Cc: davem, edumazet, geliang, kuba, linux-kernel, martineau, matttbe,
mptcp, netdev, pabeni, syzbot+f3a31fb909db9b2a5c4d,
syzkaller-bugs
There are two paths to access mptcp_pm_del_add_timer, result in a race
condition:
CPU1 CPU2
==== ====
net_rx_action
napi_poll netlink_sendmsg
__napi_poll netlink_unicast
process_backlog netlink_unicast_kernel
__netif_receive_skb genl_rcv
__netif_receive_skb_one_core netlink_rcv_skb
NF_HOOK genl_rcv_msg
ip_local_deliver_finish genl_family_rcv_msg
ip_protocol_deliver_rcu genl_family_rcv_msg_doit
tcp_v4_rcv mptcp_pm_nl_flush_addrs_doit
tcp_v4_do_rcv mptcp_nl_remove_addrs_list
tcp_rcv_established mptcp_pm_remove_addrs_and_subflows
tcp_data_queue remove_anno_list_by_saddr
mptcp_incoming_options mptcp_pm_del_add_timer
mptcp_pm_del_add_timer kfree(entry)
In remove_anno_list_by_saddr(running on CPU2), after leaving the critical
zone protected by "pm.lock", the entry will be released, which leads to the
occurrence of uaf in the mptcp_pm_del_add_timer(running on CPU1).
Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
net/mptcp/pm_netlink.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 3e4ad801786f..d4cbf7dcf983 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1430,8 +1430,10 @@ static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
entry = mptcp_pm_del_add_timer(msk, addr, false);
if (entry) {
+ spin_lock_bh(&msk->pm.lock);
list_del(&entry->list);
kfree(entry);
+ spin_unlock_bh(&msk->pm.lock);
return true;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-03 13:13 [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync syzbot
` (3 preceding siblings ...)
2024-09-04 0:20 ` [syzbot] [mptcp?] KASAN: slab-use-after-free Read " Edward Adam Davis
@ 2024-09-04 17:05 ` Matthieu Baerts (NGI0)
2024-09-05 0:46 ` [syzbot] [mptcp?] KASAN: slab-use-after-free Read " syzbot
2024-09-04 17:45 ` [PATCH net v2] mptcp: pm: Fix uaf " Matthieu Baerts (NGI0)
` (4 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Matthieu Baerts (NGI0) @ 2024-09-04 17:05 UTC (permalink / raw)
To: syzbot+f3a31fb909db9b2a5c4d; +Cc: linux-kernel, Matthieu Baerts (NGI0)
#syz test
Fixes: 00cfd77b9063 ("mptcp: retransmit ADD_ADDR when timeout")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
net/mptcp/pm_netlink.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index f891bc714668..792b5f09a915 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -332,17 +332,21 @@ struct mptcp_pm_add_entry *
mptcp_pm_del_add_timer(struct mptcp_sock *msk,
const struct mptcp_addr_info *addr, bool check_id)
{
- struct mptcp_pm_add_entry *entry;
struct sock *sk = (struct sock *)msk;
+ struct timer_list *add_timer = NULL;
+ struct mptcp_pm_add_entry *entry;
spin_lock_bh(&msk->pm.lock);
entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
- if (entry && (!check_id || entry->addr.id == addr->id))
+ if (entry && (!check_id || entry->addr.id == addr->id)) {
entry->retrans_times = ADD_ADDR_RETRANS_MAX;
+ add_timer = &entry->add_timer;
+ }
spin_unlock_bh(&msk->pm.lock);
- if (entry && (!check_id || entry->addr.id == addr->id))
- sk_stop_timer_sync(sk, &entry->add_timer);
+ /* no lock, because sk_stop_timer_sync() is calling del_timer_sync() */
+ if (add_timer)
+ sk_stop_timer_sync(sk, add_timer);
return entry;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net v2] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-03 13:13 [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync syzbot
` (4 preceding siblings ...)
2024-09-04 17:05 ` [PATCH net] mptcp: pm: Fix uaf " Matthieu Baerts (NGI0)
@ 2024-09-04 17:45 ` Matthieu Baerts (NGI0)
2024-09-05 1:10 ` [syzbot] [mptcp?] KASAN: slab-use-after-free Read " syzbot
2024-09-10 3:54 ` Edward Adam Davis
` (3 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Matthieu Baerts (NGI0) @ 2024-09-04 17:45 UTC (permalink / raw)
To: syzbot+f3a31fb909db9b2a5c4d; +Cc: linux-kernel, Matthieu Baerts (NGI0)
#syz test
v2: include Edward's patch, easier to test with syzbot.
Fixes: 00cfd77b9063 ("mptcp: retransmit ADD_ADDR when timeout")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
net/mptcp/pm_netlink.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index f891bc714668..3a5839ba92f1 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -332,17 +332,21 @@ struct mptcp_pm_add_entry *
mptcp_pm_del_add_timer(struct mptcp_sock *msk,
const struct mptcp_addr_info *addr, bool check_id)
{
- struct mptcp_pm_add_entry *entry;
struct sock *sk = (struct sock *)msk;
+ struct timer_list *add_timer = NULL;
+ struct mptcp_pm_add_entry *entry;
spin_lock_bh(&msk->pm.lock);
entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
- if (entry && (!check_id || entry->addr.id == addr->id))
+ if (entry && (!check_id || entry->addr.id == addr->id)) {
entry->retrans_times = ADD_ADDR_RETRANS_MAX;
+ add_timer = &entry->add_timer;
+ }
spin_unlock_bh(&msk->pm.lock);
- if (entry && (!check_id || entry->addr.id == addr->id))
- sk_stop_timer_sync(sk, &entry->add_timer);
+ /* no lock, because sk_stop_timer_sync() is calling del_timer_sync() */
+ if (add_timer)
+ sk_stop_timer_sync(sk, add_timer);
return entry;
}
@@ -1462,8 +1466,10 @@ static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
entry = mptcp_pm_del_add_timer(msk, addr, false);
if (entry) {
+ spin_lock_bh(&msk->pm.lock);
list_del(&entry->list);
kfree(entry);
+ spin_unlock_bh(&msk->pm.lock);
return true;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH V2] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-04 1:01 ` [PATCH V2] " Edward Adam Davis
@ 2024-09-04 20:39 ` Matthieu Baerts
2024-09-05 12:27 ` [PATCH net v3] " Edward Adam Davis
2024-09-05 12:35 ` [PATCH V2] " Edward Adam Davis
0 siblings, 2 replies; 36+ messages in thread
From: Matthieu Baerts @ 2024-09-04 20:39 UTC (permalink / raw)
To: Edward Adam Davis
Cc: davem, edumazet, geliang, kuba, linux-kernel, martineau, mptcp,
netdev, pabeni, syzbot+f3a31fb909db9b2a5c4d, syzkaller-bugs
Hello,
Thank you for this patch!
On 04/09/2024 03:01, Edward Adam Davis wrote:
> There are two paths to access mptcp_pm_del_add_timer, result in a race
> condition:
>
> CPU1 CPU2
> ==== ====
> net_rx_action
> napi_poll netlink_sendmsg
> __napi_poll netlink_unicast
> process_backlog netlink_unicast_kernel
> __netif_receive_skb genl_rcv
> __netif_receive_skb_one_core netlink_rcv_skb
> NF_HOOK genl_rcv_msg
> ip_local_deliver_finish genl_family_rcv_msg
> ip_protocol_deliver_rcu genl_family_rcv_msg_doit
> tcp_v4_rcv mptcp_pm_nl_flush_addrs_doit
> tcp_v4_do_rcv mptcp_nl_remove_addrs_list
> tcp_rcv_established mptcp_pm_remove_addrs_and_subflows
> tcp_data_queue remove_anno_list_by_saddr
> mptcp_incoming_options mptcp_pm_del_add_timer
> mptcp_pm_del_add_timer kfree(entry)
>
> In remove_anno_list_by_saddr(running on CPU2), after leaving the critical
> zone protected by "pm.lock", the entry will be released, which leads to the
> occurrence of uaf in the mptcp_pm_del_add_timer(running on CPU1).
>
> Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
Please add a Fixes tag and Cc stable.
And add 'net' after PATCH in the subject:
[PATCH net v3]
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> net/mptcp/pm_netlink.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 3e4ad801786f..d4cbf7dcf983 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1430,8 +1430,10 @@ static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
>
> entry = mptcp_pm_del_add_timer(msk, addr, false);
> if (entry) {
> + spin_lock_bh(&msk->pm.lock);
> list_del(&entry->list);
> kfree(entry);
> + spin_unlock_bh(&msk->pm.lock);
Mmh, I can understand it would help to reduce issues here, but I don't
think that's enough: in mptcp_pm_del_add_timer(), CPU1 can get the entry
from the list under the lock, then immediately after, the free can
happen on CPU2, while CPU1 is trying to access entry->add_timer outside
the lock, no? Something like this:
CPU1 CPU2
==== ====
entry = (...)
kfree(entry)
entry->add_timer
What about keeping a reference to add_timer inside the lock, and calling
sk_stop_timer_sync() with this reference, instead of "entry->add_timer"?
I'm thinking about something like that to be applied *on top* of your
patch, WDYT?
https://lore.kernel.org/20240904170517.237863-2-matttbe@kernel.org
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
2024-09-04 17:05 ` [PATCH net] mptcp: pm: Fix uaf " Matthieu Baerts (NGI0)
@ 2024-09-05 0:46 ` syzbot
0 siblings, 0 replies; 36+ messages in thread
From: syzbot @ 2024-09-05 0:46 UTC (permalink / raw)
To: linux-kernel, matttbe, syzkaller-bugs
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Tested on:
commit: c763c433 Merge tag 'bcachefs-2024-09-04' of git://evil..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=16ca0563980000
kernel config: https://syzkaller.appspot.com/x/.config?x=660f6eb11f9c7dc5
dashboard link: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=11da4e8f980000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
2024-09-04 17:45 ` [PATCH net v2] mptcp: pm: Fix uaf " Matthieu Baerts (NGI0)
@ 2024-09-05 1:10 ` syzbot
0 siblings, 0 replies; 36+ messages in thread
From: syzbot @ 2024-09-05 1:10 UTC (permalink / raw)
To: linux-kernel, matttbe, syzkaller-bugs
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Tested on:
commit: c763c433 Merge tag 'bcachefs-2024-09-04' of git://evil..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=16b4bfdb980000
kernel config: https://syzkaller.appspot.com/x/.config?x=660f6eb11f9c7dc5
dashboard link: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=132cf08b980000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH net v3] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-04 20:39 ` Matthieu Baerts
@ 2024-09-05 12:27 ` Edward Adam Davis
2024-09-06 18:55 ` Matthieu Baerts
2024-09-09 13:07 ` Paolo Abeni
2024-09-05 12:35 ` [PATCH V2] " Edward Adam Davis
1 sibling, 2 replies; 36+ messages in thread
From: Edward Adam Davis @ 2024-09-05 12:27 UTC (permalink / raw)
To: matttbe
Cc: davem, eadavis, edumazet, geliang, kuba, linux-kernel, martineau,
mptcp, netdev, pabeni, syzbot+f3a31fb909db9b2a5c4d,
syzkaller-bugs
There are two paths to access mptcp_pm_del_add_timer, result in a race
condition:
CPU1 CPU2
==== ====
net_rx_action
napi_poll netlink_sendmsg
__napi_poll netlink_unicast
process_backlog netlink_unicast_kernel
__netif_receive_skb genl_rcv
__netif_receive_skb_one_core netlink_rcv_skb
NF_HOOK genl_rcv_msg
ip_local_deliver_finish genl_family_rcv_msg
ip_protocol_deliver_rcu genl_family_rcv_msg_doit
tcp_v4_rcv mptcp_pm_nl_flush_addrs_doit
tcp_v4_do_rcv mptcp_nl_remove_addrs_list
tcp_rcv_established mptcp_pm_remove_addrs_and_subflows
tcp_data_queue remove_anno_list_by_saddr
mptcp_incoming_options mptcp_pm_del_add_timer
mptcp_pm_del_add_timer kfree(entry)
In remove_anno_list_by_saddr(running on CPU2), after leaving the critical
zone protected by "pm.lock", the entry will be released, which leads to the
occurrence of uaf in the mptcp_pm_del_add_timer(running on CPU1).
Keeping a reference to add_timer inside the lock, and calling
sk_stop_timer_sync() with this reference, instead of "entry->add_timer".
Fixes: 00cfd77b9063 ("mptcp: retransmit ADD_ADDR when timeout")
Cc: stable@vger.kernel.org
Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
net/mptcp/pm_netlink.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 3e4ad801786f..7ddb373cc6ad 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -329,17 +329,21 @@ struct mptcp_pm_add_entry *
mptcp_pm_del_add_timer(struct mptcp_sock *msk,
const struct mptcp_addr_info *addr, bool check_id)
{
- struct mptcp_pm_add_entry *entry;
struct sock *sk = (struct sock *)msk;
+ struct timer_list *add_timer = NULL;
+ struct mptcp_pm_add_entry *entry;
spin_lock_bh(&msk->pm.lock);
entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
- if (entry && (!check_id || entry->addr.id == addr->id))
+ if (entry && (!check_id || entry->addr.id == addr->id)) {
entry->retrans_times = ADD_ADDR_RETRANS_MAX;
+ add_timer = &entry->add_timer;
+ }
spin_unlock_bh(&msk->pm.lock);
- if (entry && (!check_id || entry->addr.id == addr->id))
- sk_stop_timer_sync(sk, &entry->add_timer);
+ /* no lock, because sk_stop_timer_sync() is calling del_timer_sync() */
+ if (add_timer)
+ sk_stop_timer_sync(sk, add_timer);
return entry;
}
@@ -1430,8 +1434,10 @@ static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
entry = mptcp_pm_del_add_timer(msk, addr, false);
if (entry) {
+ spin_lock_bh(&msk->pm.lock);
list_del(&entry->list);
kfree(entry);
+ spin_unlock_bh(&msk->pm.lock);
return true;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH V2] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-04 20:39 ` Matthieu Baerts
2024-09-05 12:27 ` [PATCH net v3] " Edward Adam Davis
@ 2024-09-05 12:35 ` Edward Adam Davis
1 sibling, 0 replies; 36+ messages in thread
From: Edward Adam Davis @ 2024-09-05 12:35 UTC (permalink / raw)
To: matttbe
Cc: davem, eadavis, edumazet, geliang, kuba, linux-kernel, martineau,
mptcp, netdev, pabeni, syzbot+f3a31fb909db9b2a5c4d,
syzkaller-bugs
On Wed, 4 Sep 2024 22:39:10 +0200, Matthieu Baerts wrote:
>On 04/09/2024 03:01, Edward Adam Davis wrote:
>> There are two paths to access mptcp_pm_del_add_timer, result in a race
>> condition:
>>
>> CPU1 CPU2
>> ==== ====
>> net_rx_action
>> napi_poll netlink_sendmsg
>> __napi_poll netlink_unicast
>> process_backlog netlink_unicast_kernel
>> __netif_receive_skb genl_rcv
>> __netif_receive_skb_one_core netlink_rcv_skb
>> NF_HOOK genl_rcv_msg
>> ip_local_deliver_finish genl_family_rcv_msg
>> ip_protocol_deliver_rcu genl_family_rcv_msg_doit
>> tcp_v4_rcv mptcp_pm_nl_flush_addrs_doit
>> tcp_v4_do_rcv mptcp_nl_remove_addrs_list
>> tcp_rcv_established mptcp_pm_remove_addrs_and_subflows
>> tcp_data_queue remove_anno_list_by_saddr
>> mptcp_incoming_options mptcp_pm_del_add_timer
>> mptcp_pm_del_add_timer kfree(entry)
>>
>> In remove_anno_list_by_saddr(running on CPU2), after leaving the critical
>> zone protected by "pm.lock", the entry will be released, which leads to the
>> occurrence of uaf in the mptcp_pm_del_add_timer(running on CPU1).
>>
>> Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
>
>Please add a Fixes tag and Cc stable.
>
>And add 'net' after PATCH in the subject:
Got it, I have added them in V3 patch.
>
> [PATCH net v3]
>
>> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
>> ---
>> net/mptcp/pm_netlink.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>> index 3e4ad801786f..d4cbf7dcf983 100644
>> --- a/net/mptcp/pm_netlink.c
>> +++ b/net/mptcp/pm_netlink.c
>> @@ -1430,8 +1430,10 @@ static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
>>
>> entry = mptcp_pm_del_add_timer(msk, addr, false);
>> if (entry) {
>> + spin_lock_bh(&msk->pm.lock);
>> list_del(&entry->list);
>> kfree(entry);
>> + spin_unlock_bh(&msk->pm.lock);
>
>Mmh, I can understand it would help to reduce issues here, but I don't
>think that's enough: in mptcp_pm_del_add_timer(), CPU1 can get the entry
>from the list under the lock, then immediately after, the free can
>happen on CPU2, while CPU1 is trying to access entry->add_timer outside
>the lock, no? Something like this:
>
> CPU1 CPU2
> ==== ====
> entry = (...)
> kfree(entry)
> entry->add_timer
>
>
>What about keeping a reference to add_timer inside the lock, and calling
>sk_stop_timer_sync() with this reference, instead of "entry->add_timer"?
>I'm thinking about something like that to be applied *on top* of your
>patch, WDYT?
I strongly agree. This can avoid accessing the entry outside the lock.
I have integrated your code to my patch.
BR,
Edward
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net v3] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-05 12:27 ` [PATCH net v3] " Edward Adam Davis
@ 2024-09-06 18:55 ` Matthieu Baerts
2024-09-06 22:02 ` Jakub Kicinski
2024-09-09 13:07 ` Paolo Abeni
1 sibling, 1 reply; 36+ messages in thread
From: Matthieu Baerts @ 2024-09-06 18:55 UTC (permalink / raw)
To: Edward Adam Davis
Cc: davem, edumazet, geliang, kuba, linux-kernel, martineau, mptcp,
netdev, pabeni, syzbot+f3a31fb909db9b2a5c4d, syzkaller-bugs
Hi Edward,
On 05/09/2024 14:27, Edward Adam Davis wrote:
> There are two paths to access mptcp_pm_del_add_timer, result in a race
> condition:
>
> CPU1 CPU2
> ==== ====
> net_rx_action
> napi_poll netlink_sendmsg
> __napi_poll netlink_unicast
> process_backlog netlink_unicast_kernel
> __netif_receive_skb genl_rcv
> __netif_receive_skb_one_core netlink_rcv_skb
> NF_HOOK genl_rcv_msg
> ip_local_deliver_finish genl_family_rcv_msg
> ip_protocol_deliver_rcu genl_family_rcv_msg_doit
> tcp_v4_rcv mptcp_pm_nl_flush_addrs_doit
> tcp_v4_do_rcv mptcp_nl_remove_addrs_list
> tcp_rcv_established mptcp_pm_remove_addrs_and_subflows
> tcp_data_queue remove_anno_list_by_saddr
> mptcp_incoming_options mptcp_pm_del_add_timer
> mptcp_pm_del_add_timer kfree(entry)
>
> In remove_anno_list_by_saddr(running on CPU2), after leaving the critical
> zone protected by "pm.lock", the entry will be released, which leads to the
> occurrence of uaf in the mptcp_pm_del_add_timer(running on CPU1).
>
> Keeping a reference to add_timer inside the lock, and calling
> sk_stop_timer_sync() with this reference, instead of "entry->add_timer".
>
> Fixes: 00cfd77b9063 ("mptcp: retransmit ADD_ADDR when timeout")
> Cc: stable@vger.kernel.org
> Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
According to the doc [1], a 'Co-dev' tag is supposed to be added before
this SoB:
Co-developed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
But I'm sure that's fine without it.
[1]
https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
This v3 now looks good to me. I don't know if it needs to be reviewed by
someone else as I'm now listed as co-developed.
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net v3] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-06 18:55 ` Matthieu Baerts
@ 2024-09-06 22:02 ` Jakub Kicinski
2024-09-09 7:01 ` Matthieu Baerts
0 siblings, 1 reply; 36+ messages in thread
From: Jakub Kicinski @ 2024-09-06 22:02 UTC (permalink / raw)
To: Matthieu Baerts
Cc: Edward Adam Davis, davem, edumazet, geliang, linux-kernel,
martineau, mptcp, netdev, pabeni, syzbot+f3a31fb909db9b2a5c4d,
syzkaller-bugs
On Fri, 6 Sep 2024 20:55:20 +0200 Matthieu Baerts wrote:
> > Fixes: 00cfd77b9063 ("mptcp: retransmit ADD_ADDR when timeout")
> > Cc: stable@vger.kernel.org
> > Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
> > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>
> According to the doc [1], a 'Co-dev' tag is supposed to be added before
> this SoB:
>
> Co-developed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>
> But I'm sure that's fine without it.
To be clear, would you like us to pick this up directly for net?
Or you'll send it back to us with other MPTCP patches?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net v3] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-06 22:02 ` Jakub Kicinski
@ 2024-09-09 7:01 ` Matthieu Baerts
0 siblings, 0 replies; 36+ messages in thread
From: Matthieu Baerts @ 2024-09-09 7:01 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Edward Adam Davis, davem, edumazet, geliang, linux-kernel,
martineau, mptcp, netdev, pabeni, syzbot+f3a31fb909db9b2a5c4d,
syzkaller-bugs
Hi Jakub,
On 07/09/2024 00:02, Jakub Kicinski wrote:
> On Fri, 6 Sep 2024 20:55:20 +0200 Matthieu Baerts wrote:
>>> Fixes: 00cfd77b9063 ("mptcp: retransmit ADD_ADDR when timeout")
>>> Cc: stable@vger.kernel.org
>>> Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
>>> Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
>>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>
>> According to the doc [1], a 'Co-dev' tag is supposed to be added before
>> this SoB:
>>
>> Co-developed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>
>> But I'm sure that's fine without it.
>
> To be clear, would you like us to pick this up directly for net?
Sorry, I forgot that: yes, can you pick this up directly please?
I think that's best to do that for fixes that are ready.
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net v3] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-05 12:27 ` [PATCH net v3] " Edward Adam Davis
2024-09-06 18:55 ` Matthieu Baerts
@ 2024-09-09 13:07 ` Paolo Abeni
2024-09-10 3:59 ` Edward Adam Davis
1 sibling, 1 reply; 36+ messages in thread
From: Paolo Abeni @ 2024-09-09 13:07 UTC (permalink / raw)
To: Edward Adam Davis, matttbe
Cc: davem, edumazet, geliang, kuba, linux-kernel, martineau, mptcp,
netdev, syzbot+f3a31fb909db9b2a5c4d, syzkaller-bugs
On 9/5/24 14:27, Edward Adam Davis wrote:
> There are two paths to access mptcp_pm_del_add_timer, result in a race
> condition:
>
> CPU1 CPU2
> ==== ====
> net_rx_action
> napi_poll netlink_sendmsg
> __napi_poll netlink_unicast
> process_backlog netlink_unicast_kernel
> __netif_receive_skb genl_rcv
> __netif_receive_skb_one_core netlink_rcv_skb
> NF_HOOK genl_rcv_msg
> ip_local_deliver_finish genl_family_rcv_msg
> ip_protocol_deliver_rcu genl_family_rcv_msg_doit
> tcp_v4_rcv mptcp_pm_nl_flush_addrs_doit
> tcp_v4_do_rcv mptcp_nl_remove_addrs_list
> tcp_rcv_established mptcp_pm_remove_addrs_and_subflows
> tcp_data_queue remove_anno_list_by_saddr
> mptcp_incoming_options mptcp_pm_del_add_timer
> mptcp_pm_del_add_timer kfree(entry)
>
> In remove_anno_list_by_saddr(running on CPU2), after leaving the critical
> zone protected by "pm.lock", the entry will be released, which leads to the
> occurrence of uaf in the mptcp_pm_del_add_timer(running on CPU1).
>
> Keeping a reference to add_timer inside the lock, and calling
> sk_stop_timer_sync() with this reference, instead of "entry->add_timer".
>
> Fixes: 00cfd77b9063 ("mptcp: retransmit ADD_ADDR when timeout")
> Cc: stable@vger.kernel.org
> Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> net/mptcp/pm_netlink.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 3e4ad801786f..7ddb373cc6ad 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -329,17 +329,21 @@ struct mptcp_pm_add_entry *
> mptcp_pm_del_add_timer(struct mptcp_sock *msk,
> const struct mptcp_addr_info *addr, bool check_id)
> {
> - struct mptcp_pm_add_entry *entry;
> struct sock *sk = (struct sock *)msk;
> + struct timer_list *add_timer = NULL;
> + struct mptcp_pm_add_entry *entry;
>
> spin_lock_bh(&msk->pm.lock);
> entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
> - if (entry && (!check_id || entry->addr.id == addr->id))
> + if (entry && (!check_id || entry->addr.id == addr->id)) {
> entry->retrans_times = ADD_ADDR_RETRANS_MAX;
> + add_timer = &entry->add_timer;
> + }
> spin_unlock_bh(&msk->pm.lock);
>
> - if (entry && (!check_id || entry->addr.id == addr->id))
> - sk_stop_timer_sync(sk, &entry->add_timer);
> + /* no lock, because sk_stop_timer_sync() is calling del_timer_sync() */
> + if (add_timer)
> + sk_stop_timer_sync(sk, add_timer);
>
> return entry;
> }
> @@ -1430,8 +1434,10 @@ static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
>
> entry = mptcp_pm_del_add_timer(msk, addr, false);
> if (entry) {
> + spin_lock_bh(&msk->pm.lock);
> list_del(&entry->list);
> kfree(entry);
> + spin_unlock_bh(&msk->pm.lock);
I'm sorry for the late feedback.
I think this is not enough to fix races for good, i.e.
mptcp_nl_remove_subflow_and_signal_addr() -> mptcp_pm_remove_anno_addr()
-> remove_anno_list_by_saddr()
could race with:
mptcp_pm_remove_addrs() -> remove_anno_list_by_saddr()
and both CPUs could see the same 'entry' returned by
mptcp_pm_del_add_timer().
I think the list_del() in remove_anno_list_by_saddr() should moved under
the pm lock protection inside mptcp_pm_del_add_timer(), and no need to
add spin_lock_bh() around the kfree call.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
2024-09-03 13:13 [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync syzbot
` (5 preceding siblings ...)
2024-09-04 17:45 ` [PATCH net v2] mptcp: pm: Fix uaf " Matthieu Baerts (NGI0)
@ 2024-09-10 3:54 ` Edward Adam Davis
2024-09-10 6:56 ` syzbot
2024-09-10 7:10 ` Edward Adam Davis
` (2 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Edward Adam Davis @ 2024-09-10 3:54 UTC (permalink / raw)
To: syzbot+f3a31fb909db9b2a5c4d; +Cc: linux-kernel
entry need to be protected by pm.lock.
#syz test
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 3e4ad801786f..881d0772223e 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -329,17 +329,25 @@ struct mptcp_pm_add_entry *
mptcp_pm_del_add_timer(struct mptcp_sock *msk,
const struct mptcp_addr_info *addr, bool check_id)
{
- struct mptcp_pm_add_entry *entry;
struct sock *sk = (struct sock *)msk;
+ struct timer_list *add_timer = NULL;
+ struct mptcp_pm_add_entry *entry;
spin_lock_bh(&msk->pm.lock);
entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
- if (entry && (!check_id || entry->addr.id == addr->id))
+ if (entry && (!check_id || entry->addr.id == addr->id)) {
entry->retrans_times = ADD_ADDR_RETRANS_MAX;
+ add_timer = &entry->add_timer;
+ }
+ if (!check_id) {
+ list_del(&entry->list);
+ kfree(entry);
+ }
spin_unlock_bh(&msk->pm.lock);
- if (entry && (!check_id || entry->addr.id == addr->id))
- sk_stop_timer_sync(sk, &entry->add_timer);
+ /* no lock, because sk_stop_timer_sync() is calling del_timer_sync() */
+ if (add_timer)
+ sk_stop_timer_sync(sk, add_timer);
return entry;
}
@@ -1426,16 +1434,7 @@ int mptcp_pm_nl_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int
static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
const struct mptcp_addr_info *addr)
{
- struct mptcp_pm_add_entry *entry;
-
- entry = mptcp_pm_del_add_timer(msk, addr, false);
- if (entry) {
- list_del(&entry->list);
- kfree(entry);
- return true;
- }
-
- return false;
+ return mptcp_pm_del_add_timer(msk, addr, false) != NULL;
}
static bool mptcp_pm_remove_anno_addr(struct mptcp_sock *msk,
--
2.43.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH net v3] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-09 13:07 ` Paolo Abeni
@ 2024-09-10 3:59 ` Edward Adam Davis
2024-09-10 9:58 ` [PATCH V4] " Edward Adam Davis
2024-09-10 9:58 ` [PATCH net " Edward Adam Davis
0 siblings, 2 replies; 36+ messages in thread
From: Edward Adam Davis @ 2024-09-10 3:59 UTC (permalink / raw)
To: pabeni
Cc: davem, eadavis, edumazet, geliang, kuba, linux-kernel, martineau,
matttbe, mptcp, netdev, syzbot+f3a31fb909db9b2a5c4d,
syzkaller-bugs
On Mon, 9 Sep 2024 15:07:21 +0200, Paolo Abeni wrote:
> On 9/5/24 14:27, Edward Adam Davis wrote:
> > There are two paths to access mptcp_pm_del_add_timer, result in a race
> > condition:
> >
> > CPU1 CPU2
> > ==== ====
> > net_rx_action
> > napi_poll netlink_sendmsg
> > __napi_poll netlink_unicast
> > process_backlog netlink_unicast_kernel
> > __netif_receive_skb genl_rcv
> > __netif_receive_skb_one_core netlink_rcv_skb
> > NF_HOOK genl_rcv_msg
> > ip_local_deliver_finish genl_family_rcv_msg
> > ip_protocol_deliver_rcu genl_family_rcv_msg_doit
> > tcp_v4_rcv mptcp_pm_nl_flush_addrs_doit
> > tcp_v4_do_rcv mptcp_nl_remove_addrs_list
> > tcp_rcv_established mptcp_pm_remove_addrs_and_subflows
> > tcp_data_queue remove_anno_list_by_saddr
> > mptcp_incoming_options mptcp_pm_del_add_timer
> > mptcp_pm_del_add_timer kfree(entry)
> >
> > In remove_anno_list_by_saddr(running on CPU2), after leaving the critical
> > zone protected by "pm.lock", the entry will be released, which leads to the
> > occurrence of uaf in the mptcp_pm_del_add_timer(running on CPU1).
> >
> > Keeping a reference to add_timer inside the lock, and calling
> > sk_stop_timer_sync() with this reference, instead of "entry->add_timer".
> >
> > Fixes: 00cfd77b9063 ("mptcp: retransmit ADD_ADDR when timeout")
> > Cc: stable@vger.kernel.org
> > Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
> > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > ---
> > net/mptcp/pm_netlink.c | 14 ++++++++++----
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> > index 3e4ad801786f..7ddb373cc6ad 100644
> > --- a/net/mptcp/pm_netlink.c
> > +++ b/net/mptcp/pm_netlink.c
> > @@ -329,17 +329,21 @@ struct mptcp_pm_add_entry *
> > mptcp_pm_del_add_timer(struct mptcp_sock *msk,
> > const struct mptcp_addr_info *addr, bool check_id)
> > {
> > - struct mptcp_pm_add_entry *entry;
> > struct sock *sk = (struct sock *)msk;
> > + struct timer_list *add_timer = NULL;
> > + struct mptcp_pm_add_entry *entry;
> >
> > spin_lock_bh(&msk->pm.lock);
> > entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
> > - if (entry && (!check_id || entry->addr.id == addr->id))
> > + if (entry && (!check_id || entry->addr.id == addr->id)) {
> > entry->retrans_times = ADD_ADDR_RETRANS_MAX;
> > + add_timer = &entry->add_timer;
> > + }
> > spin_unlock_bh(&msk->pm.lock);
> >
> > - if (entry && (!check_id || entry->addr.id == addr->id))
> > - sk_stop_timer_sync(sk, &entry->add_timer);
> > + /* no lock, because sk_stop_timer_sync() is calling del_timer_sync() */
> > + if (add_timer)
> > + sk_stop_timer_sync(sk, add_timer);
> >
> > return entry;
> > }
> > @@ -1430,8 +1434,10 @@ static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
> >
> > entry = mptcp_pm_del_add_timer(msk, addr, false);
> > if (entry) {
> > + spin_lock_bh(&msk->pm.lock);
> > list_del(&entry->list);
> > kfree(entry);
> > + spin_unlock_bh(&msk->pm.lock);
>
> I'm sorry for the late feedback.
>
> I think this is not enough to fix races for good, i.e.
>
> mptcp_nl_remove_subflow_and_signal_addr() -> mptcp_pm_remove_anno_addr()
> -> remove_anno_list_by_saddr()
>
> could race with:
>
> mptcp_pm_remove_addrs() -> remove_anno_list_by_saddr()
>
> and both CPUs could see the same 'entry' returned by
> mptcp_pm_del_add_timer().
Yes, you are right.
>
> I think the list_del() in remove_anno_list_by_saddr() should moved under
> the pm lock protection inside mptcp_pm_del_add_timer(), and no need to
> add spin_lock_bh() around the kfree call.
Agreed, I will update the patch.
BR,
Edward
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
2024-09-10 3:54 ` Edward Adam Davis
@ 2024-09-10 6:56 ` syzbot
0 siblings, 0 replies; 36+ messages in thread
From: syzbot @ 2024-09-10 6:56 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 __timer_delete_sync
==================================================================
BUG: KASAN: slab-use-after-free in __lock_acquire+0x77/0x2040 kernel/locking/lockdep.c:5007
Read of size 8 at addr ffff8880244d5c58 by task syz.2.17/5997
CPU: 1 UID: 0 PID: 5997 Comm: syz.2.17 Not tainted 6.11.0-rc7-syzkaller-gbc83b4d1f086-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/06/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:93 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:119
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
__lock_acquire+0x77/0x2040 kernel/locking/lockdep.c:5007
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5759
__timer_delete_sync+0x148/0x310 kernel/time/timer.c:1647
del_timer_sync include/linux/timer.h:185 [inline]
sk_stop_timer_sync+0x1c/0x90 net/core/sock.c:3454
mptcp_pm_del_add_timer+0x23f/0x2d0 net/mptcp/pm_netlink.c:353
remove_anno_list_by_saddr net/mptcp/pm_netlink.c:1469 [inline]
mptcp_pm_remove_addrs_and_subflows net/mptcp/pm_netlink.c:1686 [inline]
mptcp_nl_remove_addrs_list net/mptcp/pm_netlink.c:1717 [inline]
mptcp_pm_nl_flush_addrs_doit+0x670/0xdd0 net/mptcp/pm_netlink.c:1758
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:2550
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
netlink_unicast_kernel net/netlink/af_netlink.c:1331 [inline]
netlink_unicast+0x7f6/0x990 net/netlink/af_netlink.c:1357
netlink_sendmsg+0x8e4/0xcb0 net/netlink/af_netlink.c:1901
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0x221/0x270 net/socket.c:745
____sys_sendmsg+0x525/0x7d0 net/socket.c:2597
___sys_sendmsg net/socket.c:2651 [inline]
__sys_sendmsg+0x2b0/0x3a0 net/socket.c:2680
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
RIP: 0033:0x7f394df79eb9
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 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 a8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f394ed91038 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007f394e115f80 RCX: 00007f394df79eb9
RDX: 0000000001000000 RSI: 0000000020000300 RDI: 0000000000000003
RBP: 00007f394dfe793e R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 0000000000000000 R14: 00007f394e115f80 R15: 00007f394e23fa28
</TASK>
Allocated by task 5997:
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]
__kmalloc_cache_noprof+0x19c/0x2c0 mm/slub.c:4193
kmalloc_noprof include/linux/slab.h:681 [inline]
mptcp_pm_alloc_anno_list+0x14e/0x390 net/mptcp/pm_netlink.c:378
mptcp_pm_create_subflow_or_signal_addr+0x1920/0x22c0 net/mptcp/pm_netlink.c:594
mptcp_nl_add_subflow_or_signal_addr net/mptcp/pm_netlink.c:1364 [inline]
mptcp_pm_nl_add_addr_doit+0x1276/0x1b80 net/mptcp/pm_netlink.c:1436
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:2550
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
netlink_unicast_kernel net/netlink/af_netlink.c:1331 [inline]
netlink_unicast+0x7f6/0x990 net/netlink/af_netlink.c:1357
netlink_sendmsg+0x8e4/0xcb0 net/netlink/af_netlink.c:1901
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0x221/0x270 net/socket.c:745
____sys_sendmsg+0x525/0x7d0 net/socket.c:2597
___sys_sendmsg net/socket.c:2651 [inline]
__sys_sendmsg+0x2b0/0x3a0 net/socket.c:2680
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 5997:
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:2256 [inline]
slab_free mm/slub.c:4477 [inline]
kfree+0x149/0x360 mm/slub.c:4598
mptcp_pm_del_add_timer+0x221/0x2d0 net/mptcp/pm_netlink.c:347
remove_anno_list_by_saddr net/mptcp/pm_netlink.c:1469 [inline]
mptcp_pm_remove_addrs_and_subflows net/mptcp/pm_netlink.c:1686 [inline]
mptcp_nl_remove_addrs_list net/mptcp/pm_netlink.c:1717 [inline]
mptcp_pm_nl_flush_addrs_doit+0x670/0xdd0 net/mptcp/pm_netlink.c:1758
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:2550
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
netlink_unicast_kernel net/netlink/af_netlink.c:1331 [inline]
netlink_unicast+0x7f6/0x990 net/netlink/af_netlink.c:1357
netlink_sendmsg+0x8e4/0xcb0 net/netlink/af_netlink.c:1901
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0x221/0x270 net/socket.c:745
____sys_sendmsg+0x525/0x7d0 net/socket.c:2597
___sys_sendmsg net/socket.c:2651 [inline]
__sys_sendmsg+0x2b0/0x3a0 net/socket.c:2680
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 ffff8880244d5c00
which belongs to the cache kmalloc-192 of size 192
The buggy address is located 88 bytes inside of
freed 192-byte region [ffff8880244d5c00, ffff8880244d5cc0)
The buggy address belongs to the physical page:
page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x244d5
flags: 0xfff00000000000(node=0|zone=1|lastcpupid=0x7ff)
page_type: 0xfdffffff(slab)
raw: 00fff00000000000 ffff88801ac413c0 dead000000000122 0000000000000000
raw: 0000000000000000 0000000080100010 00000001fdffffff 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 0, migratetype Unmovable, gfp_mask 0x152cc0(GFP_USER|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP), pid 5964, tgid 5964 (syz-executor), ts 116670204712, free_ts 116664550352
set_page_owner include/linux/page_owner.h:32 [inline]
post_alloc_hook+0x1f3/0x230 mm/page_alloc.c:1500
prep_new_page mm/page_alloc.c:1508 [inline]
get_page_from_freelist+0x2e4c/0x2f10 mm/page_alloc.c:3446
__alloc_pages_noprof+0x256/0x6c0 mm/page_alloc.c:4702
__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:2325
allocate_slab+0x5a/0x2f0 mm/slub.c:2488
new_slab mm/slub.c:2541 [inline]
___slab_alloc+0xcd1/0x14b0 mm/slub.c:3727
__slab_alloc+0x58/0xa0 mm/slub.c:3817
__slab_alloc_node mm/slub.c:3870 [inline]
slab_alloc_node mm/slub.c:4029 [inline]
__kmalloc_cache_noprof+0x1d5/0x2c0 mm/slub.c:4188
kmalloc_noprof include/linux/slab.h:681 [inline]
kzalloc_noprof include/linux/slab.h:807 [inline]
cgroup_file_open+0x8e/0x2b0 kernel/cgroup/cgroup.c:4053
kernfs_fop_open+0xa58/0xd10 fs/kernfs/file.c:706
do_dentry_open+0x970/0x1440 fs/open.c:959
vfs_open+0x3e/0x330 fs/open.c:1089
do_open fs/namei.c:3727 [inline]
path_openat+0x2b3e/0x3470 fs/namei.c:3886
do_filp_open+0x235/0x490 fs/namei.c:3913
do_sys_openat2+0x13e/0x1d0 fs/open.c:1416
do_sys_open fs/open.c:1431 [inline]
__do_sys_openat fs/open.c:1447 [inline]
__se_sys_openat fs/open.c:1442 [inline]
__x64_sys_openat+0x247/0x2a0 fs/open.c:1442
page last free pid 5964 tgid 5964 stack trace:
reset_page_owner include/linux/page_owner.h:25 [inline]
free_pages_prepare mm/page_alloc.c:1101 [inline]
free_unref_page+0xd22/0xea0 mm/page_alloc.c:2619
discard_slab mm/slub.c:2587 [inline]
__put_partials+0xeb/0x130 mm/slub.c:3055
put_cpu_partial+0x17c/0x250 mm/slub.c:3130
__slab_free+0x2ea/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:3992 [inline]
slab_alloc_node mm/slub.c:4041 [inline]
kmem_cache_alloc_noprof+0x135/0x2a0 mm/slub.c:4048
getname_flags+0xb7/0x540 fs/namei.c:139
user_path_at+0x24/0x60 fs/namei.c:3001
__do_sys_pivot_root fs/namespace.c:4317 [inline]
__se_sys_pivot_root+0x1da/0x1650 fs/namespace.c:4301
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:
ffff8880244d5b00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff8880244d5b80: 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc fc
>ffff8880244d5c00: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8880244d5c80: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
ffff8880244d5d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==================================================================
Tested on:
commit: bc83b4d1 Merge tag 'bcachefs-2024-09-09' of git://evil..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=17fb449f980000
kernel config: https://syzkaller.appspot.com/x/.config?x=61d235cb8d15001c
dashboard link: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=14aaa807980000
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
2024-09-03 13:13 [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync syzbot
` (6 preceding siblings ...)
2024-09-10 3:54 ` Edward Adam Davis
@ 2024-09-10 7:10 ` Edward Adam Davis
2024-09-10 7:43 ` syzbot
2024-09-10 8:03 ` Edward Adam Davis
2024-09-10 9:32 ` Edward Adam Davis
9 siblings, 1 reply; 36+ messages in thread
From: Edward Adam Davis @ 2024-09-10 7:10 UTC (permalink / raw)
To: syzbot+f3a31fb909db9b2a5c4d; +Cc: linux-kernel
entry need to be protected by pm.lock.
#syz test
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 3e4ad801786f..b09268fc7fc9 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -329,17 +329,28 @@ struct mptcp_pm_add_entry *
mptcp_pm_del_add_timer(struct mptcp_sock *msk,
const struct mptcp_addr_info *addr, bool check_id)
{
- struct mptcp_pm_add_entry *entry;
struct sock *sk = (struct sock *)msk;
+ struct timer_list *add_timer = NULL;
+ struct mptcp_pm_add_entry *entry;
spin_lock_bh(&msk->pm.lock);
entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
- if (entry && (!check_id || entry->addr.id == addr->id))
+ if (entry && (!check_id || entry->addr.id == addr->id)) {
entry->retrans_times = ADD_ADDR_RETRANS_MAX;
+ add_timer = &entry->add_timer;
+ }
spin_unlock_bh(&msk->pm.lock);
- if (entry && (!check_id || entry->addr.id == addr->id))
- sk_stop_timer_sync(sk, &entry->add_timer);
+ /* no lock, because sk_stop_timer_sync() is calling del_timer_sync() */
+ if (add_timer)
+ sk_stop_timer_sync(sk, add_timer);
+
+ spin_lock_bh(&msk->pm.lock);
+ if (!check_id) {
+ list_del(&entry->list);
+ kfree(entry);
+ }
+ spin_unlock_bh(&msk->pm.lock);
return entry;
}
@@ -1426,16 +1437,7 @@ int mptcp_pm_nl_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int
static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
const struct mptcp_addr_info *addr)
{
- struct mptcp_pm_add_entry *entry;
-
- entry = mptcp_pm_del_add_timer(msk, addr, false);
- if (entry) {
- list_del(&entry->list);
- kfree(entry);
- return true;
- }
-
- return false;
+ return mptcp_pm_del_add_timer(msk, addr, false) != NULL;
}
static bool mptcp_pm_remove_anno_addr(struct mptcp_sock *msk,
--
2.43.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
2024-09-10 7:10 ` Edward Adam Davis
@ 2024-09-10 7:43 ` syzbot
0 siblings, 0 replies; 36+ messages in thread
From: syzbot @ 2024-09-10 7: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:
general protection fault in mptcp_pm_del_add_timer
Oops: general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] PREEMPT SMP KASAN PTI
KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
CPU: 0 UID: 0 PID: 6054 Comm: syz.1.16 Not tainted 6.11.0-rc7-syzkaller-gbc83b4d1f086-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/06/2024
RIP: 0010:__list_del_entry_valid_or_report+0x23/0x140 lib/list_debug.c:49
Code: 90 90 90 90 90 90 90 f3 0f 1e fa 41 57 41 56 41 54 53 49 89 ff 49 bc 00 00 00 00 00 fc ff df 48 83 c7 08 48 89 f8 48 c1 e8 03 <42> 80 3c 20 00 74 05 e8 f1 a5 40 fd 49 8b 5f 08 4c 89 f8 48 c1 e8
RSP: 0018:ffffc900032172a0 EFLAGS: 00010202
RAX: 0000000000000001 RBX: ffff88801295ac90 RCX: 0000000000000001
RDX: dffffc0000000000 RSI: 0000000000000004 RDI: 0000000000000008
RBP: 0000000000000000 R08: 0000000000000003 R09: fffff52000642e48
R10: dffffc0000000000 R11: fffff52000642e48 R12: dffffc0000000000
R13: ffff888028988a48 R14: 0000000000000000 R15: 0000000000000000
FS: 00007fe27a4d56c0(0000) GS:ffff8880b8800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ffdededd998 CR3: 000000006406c000 CR4: 00000000003506f0
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 include/linux/list.h:229 [inline]
mptcp_pm_del_add_timer+0x173/0x2c0 net/mptcp/pm_netlink.c:353
remove_anno_list_by_saddr net/mptcp/pm_netlink.c:1472 [inline]
mptcp_pm_remove_addrs_and_subflows net/mptcp/pm_netlink.c:1689 [inline]
mptcp_nl_remove_addrs_list net/mptcp/pm_netlink.c:1720 [inline]
mptcp_pm_nl_flush_addrs_doit+0x670/0xdd0 net/mptcp/pm_netlink.c:1761
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:2550
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
netlink_unicast_kernel net/netlink/af_netlink.c:1331 [inline]
netlink_unicast+0x7f6/0x990 net/netlink/af_netlink.c:1357
netlink_sendmsg+0x8e4/0xcb0 net/netlink/af_netlink.c:1901
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0x221/0x270 net/socket.c:745
____sys_sendmsg+0x525/0x7d0 net/socket.c:2597
___sys_sendmsg net/socket.c:2651 [inline]
__sys_sendmsg+0x2b0/0x3a0 net/socket.c:2680
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
RIP: 0033:0x7fe279779eb9
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 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 a8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fe27a4d5038 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007fe279915f80 RCX: 00007fe279779eb9
RDX: 0000000001000000 RSI: 0000000020000300 RDI: 0000000000000003
RBP: 00007fe2797e793e R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 0000000000000000 R14: 00007fe279915f80 R15: 00007fe279a3fa28
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:__list_del_entry_valid_or_report+0x23/0x140 lib/list_debug.c:49
Code: 90 90 90 90 90 90 90 f3 0f 1e fa 41 57 41 56 41 54 53 49 89 ff 49 bc 00 00 00 00 00 fc ff df 48 83 c7 08 48 89 f8 48 c1 e8 03 <42> 80 3c 20 00 74 05 e8 f1 a5 40 fd 49 8b 5f 08 4c 89 f8 48 c1 e8
RSP: 0018:ffffc900032172a0 EFLAGS: 00010202
RAX: 0000000000000001 RBX: ffff88801295ac90 RCX: 0000000000000001
RDX: dffffc0000000000 RSI: 0000000000000004 RDI: 0000000000000008
RBP: 0000000000000000 R08: 0000000000000003 R09: fffff52000642e48
R10: dffffc0000000000 R11: fffff52000642e48 R12: dffffc0000000000
R13: ffff888028988a48 R14: 0000000000000000 R15: 0000000000000000
FS: 00007fe27a4d56c0(0000) GS:ffff8880b8800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ffdededd998 CR3: 000000006406c000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
----------------
Code disassembly (best guess):
0: 90 nop
1: 90 nop
2: 90 nop
3: 90 nop
4: 90 nop
5: 90 nop
6: 90 nop
7: f3 0f 1e fa endbr64
b: 41 57 push %r15
d: 41 56 push %r14
f: 41 54 push %r12
11: 53 push %rbx
12: 49 89 ff mov %rdi,%r15
15: 49 bc 00 00 00 00 00 movabs $0xdffffc0000000000,%r12
1c: fc ff df
1f: 48 83 c7 08 add $0x8,%rdi
23: 48 89 f8 mov %rdi,%rax
26: 48 c1 e8 03 shr $0x3,%rax
* 2a: 42 80 3c 20 00 cmpb $0x0,(%rax,%r12,1) <-- trapping instruction
2f: 74 05 je 0x36
31: e8 f1 a5 40 fd call 0xfd40a627
36: 49 8b 5f 08 mov 0x8(%r15),%rbx
3a: 4c 89 f8 mov %r15,%rax
3d: 48 rex.W
3e: c1 .byte 0xc1
3f: e8 .byte 0xe8
Tested on:
commit: bc83b4d1 Merge tag 'bcachefs-2024-09-09' of git://evil..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1555a807980000
kernel config: https://syzkaller.appspot.com/x/.config?x=61d235cb8d15001c
dashboard link: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=16746797980000
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
2024-09-03 13:13 [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync syzbot
` (7 preceding siblings ...)
2024-09-10 7:10 ` Edward Adam Davis
@ 2024-09-10 8:03 ` Edward Adam Davis
2024-09-10 8:32 ` syzbot
2024-09-10 9:32 ` Edward Adam Davis
9 siblings, 1 reply; 36+ messages in thread
From: Edward Adam Davis @ 2024-09-10 8:03 UTC (permalink / raw)
To: syzbot+f3a31fb909db9b2a5c4d; +Cc: linux-kernel
entry need to be protected by pm.lock.
#syz test
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 3e4ad801786f..b09268fc7fc9 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -329,17 +329,28 @@ struct mptcp_pm_add_entry *
mptcp_pm_del_add_timer(struct mptcp_sock *msk,
const struct mptcp_addr_info *addr, bool check_id)
{
- struct mptcp_pm_add_entry *entry;
struct sock *sk = (struct sock *)msk;
+ struct timer_list *add_timer = NULL;
+ struct mptcp_pm_add_entry *entry;
spin_lock_bh(&msk->pm.lock);
entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
- if (entry && (!check_id || entry->addr.id == addr->id))
+ if (entry && (!check_id || entry->addr.id == addr->id)) {
entry->retrans_times = ADD_ADDR_RETRANS_MAX;
+ add_timer = &entry->add_timer;
+ }
spin_unlock_bh(&msk->pm.lock);
- if (entry && (!check_id || entry->addr.id == addr->id))
- sk_stop_timer_sync(sk, &entry->add_timer);
+ /* no lock, because sk_stop_timer_sync() is calling del_timer_sync() */
+ if (add_timer)
+ sk_stop_timer_sync(sk, add_timer);
+
+ spin_lock_bh(&msk->pm.lock);
+ if (!check_id && entry) {
+ list_del(&entry->list);
+ kfree(entry);
+ }
+ spin_unlock_bh(&msk->pm.lock);
return entry;
}
@@ -1426,16 +1437,7 @@ int mptcp_pm_nl_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int
static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
const struct mptcp_addr_info *addr)
{
- struct mptcp_pm_add_entry *entry;
-
- entry = mptcp_pm_del_add_timer(msk, addr, false);
- if (entry) {
- list_del(&entry->list);
- kfree(entry);
- return true;
- }
-
- return false;
+ return mptcp_pm_del_add_timer(msk, addr, false) != NULL;
}
static bool mptcp_pm_remove_anno_addr(struct mptcp_sock *msk,
--
2.43.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
2024-09-10 8:03 ` Edward Adam Davis
@ 2024-09-10 8:32 ` syzbot
0 siblings, 0 replies; 36+ messages in thread
From: syzbot @ 2024-09-10 8:32 UTC (permalink / raw)
To: eadavis, linux-kernel, syzkaller-bugs
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Tested on:
commit: bc83b4d1 Merge tag 'bcachefs-2024-09-09' of git://evil..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=151c6797980000
kernel config: https://syzkaller.appspot.com/x/.config?x=61d235cb8d15001c
dashboard link: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=15c47bc7980000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
2024-09-03 13:13 [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync syzbot
` (8 preceding siblings ...)
2024-09-10 8:03 ` Edward Adam Davis
@ 2024-09-10 9:32 ` Edward Adam Davis
2024-09-10 9:57 ` syzbot
9 siblings, 1 reply; 36+ messages in thread
From: Edward Adam Davis @ 2024-09-10 9:32 UTC (permalink / raw)
To: syzbot+f3a31fb909db9b2a5c4d; +Cc: linux-kernel
entry need to be protected by pm.lock.
#syz test
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 3e4ad801786f..f195b577c367 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -331,15 +331,21 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk,
{
struct mptcp_pm_add_entry *entry;
struct sock *sk = (struct sock *)msk;
+ struct timer_list *add_timer = NULL;
spin_lock_bh(&msk->pm.lock);
entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
- if (entry && (!check_id || entry->addr.id == addr->id))
+ if (entry && (!check_id || entry->addr.id == addr->id)) {
entry->retrans_times = ADD_ADDR_RETRANS_MAX;
+ add_timer = &entry->add_timer;
+ }
+ if (!check_id && entry)
+ list_del(&entry->list);
spin_unlock_bh(&msk->pm.lock);
- if (entry && (!check_id || entry->addr.id == addr->id))
- sk_stop_timer_sync(sk, &entry->add_timer);
+ /* no lock, because sk_stop_timer_sync() is calling del_timer_sync() */
+ if (add_timer)
+ sk_stop_timer_sync(sk, add_timer);
return entry;
}
@@ -1430,7 +1436,6 @@ static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
entry = mptcp_pm_del_add_timer(msk, addr, false);
if (entry) {
- list_del(&entry->list);
kfree(entry);
return true;
}
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync
2024-09-10 9:32 ` Edward Adam Davis
@ 2024-09-10 9:57 ` syzbot
0 siblings, 0 replies; 36+ messages in thread
From: syzbot @ 2024-09-10 9:57 UTC (permalink / raw)
To: eadavis, linux-kernel, syzkaller-bugs
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Tested on:
commit: bc83b4d1 Merge tag 'bcachefs-2024-09-09' of git://evil..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=15a27bc7980000
kernel config: https://syzkaller.appspot.com/x/.config?x=61d235cb8d15001c
dashboard link: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=173c6797980000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH V4] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-10 3:59 ` Edward Adam Davis
@ 2024-09-10 9:58 ` Edward Adam Davis
2024-09-10 9:58 ` [PATCH net " Edward Adam Davis
1 sibling, 0 replies; 36+ messages in thread
From: Edward Adam Davis @ 2024-09-10 9:58 UTC (permalink / raw)
To: eadavis
Cc: davem, edumazet, geliang, kuba, linux-kernel, martineau, matttbe,
mptcp, netdev, pabeni, syzbot+f3a31fb909db9b2a5c4d,
syzkaller-bugs
There are two paths to access mptcp_pm_del_add_timer, result in a race
condition:
CPU1 CPU2
==== ====
net_rx_action
napi_poll netlink_sendmsg
__napi_poll netlink_unicast
process_backlog netlink_unicast_kernel
__netif_receive_skb genl_rcv
__netif_receive_skb_one_core netlink_rcv_skb
NF_HOOK genl_rcv_msg
ip_local_deliver_finish genl_family_rcv_msg
ip_protocol_deliver_rcu genl_family_rcv_msg_doit
tcp_v4_rcv mptcp_pm_nl_flush_addrs_doit
tcp_v4_do_rcv mptcp_nl_remove_addrs_list
tcp_rcv_established mptcp_pm_remove_addrs_and_subflows
tcp_data_queue remove_anno_list_by_saddr
mptcp_incoming_options mptcp_pm_del_add_timer
mptcp_pm_del_add_timer kfree(entry)
In remove_anno_list_by_saddr(running on CPU2), after leaving the critical
zone protected by "pm.lock", the entry will be released, which leads to the
occurrence of uaf in the mptcp_pm_del_add_timer(running on CPU1).
Keeping a reference to add_timer inside the lock, and calling
sk_stop_timer_sync() with this reference, instead of "entry->add_timer".
Move list_del(&entry->list) to mptcp_pm_del_add_timer and inside the pm lock,
do not directly access any members of the entry outside the pm lock, which
can avoid similar "entry->x" uaf.
Fixes: 00cfd77b9063 ("mptcp: retransmit ADD_ADDR when timeout")
Cc: stable@vger.kernel.org
Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
net/mptcp/pm_netlink.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 3e4ad801786f..f195b577c367 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -331,15 +331,21 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk,
{
struct mptcp_pm_add_entry *entry;
struct sock *sk = (struct sock *)msk;
+ struct timer_list *add_timer = NULL;
spin_lock_bh(&msk->pm.lock);
entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
- if (entry && (!check_id || entry->addr.id == addr->id))
+ if (entry && (!check_id || entry->addr.id == addr->id)) {
entry->retrans_times = ADD_ADDR_RETRANS_MAX;
+ add_timer = &entry->add_timer;
+ }
+ if (!check_id && entry)
+ list_del(&entry->list);
spin_unlock_bh(&msk->pm.lock);
- if (entry && (!check_id || entry->addr.id == addr->id))
- sk_stop_timer_sync(sk, &entry->add_timer);
+ /* no lock, because sk_stop_timer_sync() is calling del_timer_sync() */
+ if (add_timer)
+ sk_stop_timer_sync(sk, add_timer);
return entry;
}
@@ -1430,7 +1436,6 @@ static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
entry = mptcp_pm_del_add_timer(msk, addr, false);
if (entry) {
- list_del(&entry->list);
kfree(entry);
return true;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net V4] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-10 3:59 ` Edward Adam Davis
2024-09-10 9:58 ` [PATCH V4] " Edward Adam Davis
@ 2024-09-10 9:58 ` Edward Adam Davis
2024-09-10 11:27 ` Paolo Abeni
` (2 more replies)
1 sibling, 3 replies; 36+ messages in thread
From: Edward Adam Davis @ 2024-09-10 9:58 UTC (permalink / raw)
To: eadavis
Cc: davem, edumazet, geliang, kuba, linux-kernel, martineau, matttbe,
mptcp, netdev, pabeni, syzbot+f3a31fb909db9b2a5c4d,
syzkaller-bugs
There are two paths to access mptcp_pm_del_add_timer, result in a race
condition:
CPU1 CPU2
==== ====
net_rx_action
napi_poll netlink_sendmsg
__napi_poll netlink_unicast
process_backlog netlink_unicast_kernel
__netif_receive_skb genl_rcv
__netif_receive_skb_one_core netlink_rcv_skb
NF_HOOK genl_rcv_msg
ip_local_deliver_finish genl_family_rcv_msg
ip_protocol_deliver_rcu genl_family_rcv_msg_doit
tcp_v4_rcv mptcp_pm_nl_flush_addrs_doit
tcp_v4_do_rcv mptcp_nl_remove_addrs_list
tcp_rcv_established mptcp_pm_remove_addrs_and_subflows
tcp_data_queue remove_anno_list_by_saddr
mptcp_incoming_options mptcp_pm_del_add_timer
mptcp_pm_del_add_timer kfree(entry)
In remove_anno_list_by_saddr(running on CPU2), after leaving the critical
zone protected by "pm.lock", the entry will be released, which leads to the
occurrence of uaf in the mptcp_pm_del_add_timer(running on CPU1).
Keeping a reference to add_timer inside the lock, and calling
sk_stop_timer_sync() with this reference, instead of "entry->add_timer".
Move list_del(&entry->list) to mptcp_pm_del_add_timer and inside the pm lock,
do not directly access any members of the entry outside the pm lock, which
can avoid similar "entry->x" uaf.
Fixes: 00cfd77b9063 ("mptcp: retransmit ADD_ADDR when timeout")
Cc: stable@vger.kernel.org
Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
net/mptcp/pm_netlink.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 3e4ad801786f..f195b577c367 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -331,15 +331,21 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk,
{
struct mptcp_pm_add_entry *entry;
struct sock *sk = (struct sock *)msk;
+ struct timer_list *add_timer = NULL;
spin_lock_bh(&msk->pm.lock);
entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
- if (entry && (!check_id || entry->addr.id == addr->id))
+ if (entry && (!check_id || entry->addr.id == addr->id)) {
entry->retrans_times = ADD_ADDR_RETRANS_MAX;
+ add_timer = &entry->add_timer;
+ }
+ if (!check_id && entry)
+ list_del(&entry->list);
spin_unlock_bh(&msk->pm.lock);
- if (entry && (!check_id || entry->addr.id == addr->id))
- sk_stop_timer_sync(sk, &entry->add_timer);
+ /* no lock, because sk_stop_timer_sync() is calling del_timer_sync() */
+ if (add_timer)
+ sk_stop_timer_sync(sk, add_timer);
return entry;
}
@@ -1430,7 +1436,6 @@ static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
entry = mptcp_pm_del_add_timer(msk, addr, false);
if (entry) {
- list_del(&entry->list);
kfree(entry);
return true;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH net V4] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-10 9:58 ` [PATCH net " Edward Adam Davis
@ 2024-09-10 11:27 ` Paolo Abeni
2024-09-10 14:29 ` Matthieu Baerts
2024-09-11 23:20 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 36+ messages in thread
From: Paolo Abeni @ 2024-09-10 11:27 UTC (permalink / raw)
To: Edward Adam Davis
Cc: davem, edumazet, geliang, kuba, linux-kernel, martineau, matttbe,
mptcp, netdev, syzbot+f3a31fb909db9b2a5c4d, syzkaller-bugs
On 9/10/24 11:58, Edward Adam Davis wrote:
> There are two paths to access mptcp_pm_del_add_timer, result in a race
> condition:
>
> CPU1 CPU2
> ==== ====
> net_rx_action
> napi_poll netlink_sendmsg
> __napi_poll netlink_unicast
> process_backlog netlink_unicast_kernel
> __netif_receive_skb genl_rcv
> __netif_receive_skb_one_core netlink_rcv_skb
> NF_HOOK genl_rcv_msg
> ip_local_deliver_finish genl_family_rcv_msg
> ip_protocol_deliver_rcu genl_family_rcv_msg_doit
> tcp_v4_rcv mptcp_pm_nl_flush_addrs_doit
> tcp_v4_do_rcv mptcp_nl_remove_addrs_list
> tcp_rcv_established mptcp_pm_remove_addrs_and_subflows
> tcp_data_queue remove_anno_list_by_saddr
> mptcp_incoming_options mptcp_pm_del_add_timer
> mptcp_pm_del_add_timer kfree(entry)
>
> In remove_anno_list_by_saddr(running on CPU2), after leaving the critical
> zone protected by "pm.lock", the entry will be released, which leads to the
> occurrence of uaf in the mptcp_pm_del_add_timer(running on CPU1).
>
> Keeping a reference to add_timer inside the lock, and calling
> sk_stop_timer_sync() with this reference, instead of "entry->add_timer".
>
> Move list_del(&entry->list) to mptcp_pm_del_add_timer and inside the pm lock,
> do not directly access any members of the entry outside the pm lock, which
> can avoid similar "entry->x" uaf.
>
> Fixes: 00cfd77b9063 ("mptcp: retransmit ADD_ADDR when timeout")
> Cc: stable@vger.kernel.org
> Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
Acked-by: Paolo Abeni <pabeni@redhat.com>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net V4] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-10 9:58 ` [PATCH net " Edward Adam Davis
2024-09-10 11:27 ` Paolo Abeni
@ 2024-09-10 14:29 ` Matthieu Baerts
2024-09-11 23:20 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 36+ messages in thread
From: Matthieu Baerts @ 2024-09-10 14:29 UTC (permalink / raw)
To: Edward Adam Davis
Cc: davem, edumazet, geliang, kuba, linux-kernel, martineau, mptcp,
netdev, pabeni, syzbot+f3a31fb909db9b2a5c4d, syzkaller-bugs
Hi Edward,
On 10/09/2024 11:58, Edward Adam Davis wrote:
> There are two paths to access mptcp_pm_del_add_timer, result in a race
> condition:
>
> CPU1 CPU2
> ==== ====
> net_rx_action
> napi_poll netlink_sendmsg
> __napi_poll netlink_unicast
> process_backlog netlink_unicast_kernel
> __netif_receive_skb genl_rcv
> __netif_receive_skb_one_core netlink_rcv_skb
> NF_HOOK genl_rcv_msg
> ip_local_deliver_finish genl_family_rcv_msg
> ip_protocol_deliver_rcu genl_family_rcv_msg_doit
> tcp_v4_rcv mptcp_pm_nl_flush_addrs_doit
> tcp_v4_do_rcv mptcp_nl_remove_addrs_list
> tcp_rcv_established mptcp_pm_remove_addrs_and_subflows
> tcp_data_queue remove_anno_list_by_saddr
> mptcp_incoming_options mptcp_pm_del_add_timer
> mptcp_pm_del_add_timer kfree(entry)
>
> In remove_anno_list_by_saddr(running on CPU2), after leaving the critical
> zone protected by "pm.lock", the entry will be released, which leads to the
> occurrence of uaf in the mptcp_pm_del_add_timer(running on CPU1).
>
> Keeping a reference to add_timer inside the lock, and calling
> sk_stop_timer_sync() with this reference, instead of "entry->add_timer".
>
> Move list_del(&entry->list) to mptcp_pm_del_add_timer and inside the pm lock,
> do not directly access any members of the entry outside the pm lock, which
> can avoid similar "entry->x" uaf.
>
> Fixes: 00cfd77b9063 ("mptcp: retransmit ADD_ADDR when timeout")
> Cc: stable@vger.kernel.org
> Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
It looks also good to me, but no need for me to add a reviewed-by tag I
suppose.
This v4 can be applied in netdev directly.
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net V4] mptcp: pm: Fix uaf in __timer_delete_sync
2024-09-10 9:58 ` [PATCH net " Edward Adam Davis
2024-09-10 11:27 ` Paolo Abeni
2024-09-10 14:29 ` Matthieu Baerts
@ 2024-09-11 23:20 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 36+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-09-11 23:20 UTC (permalink / raw)
To: Edward Adam Davis
Cc: davem, edumazet, geliang, kuba, linux-kernel, martineau, matttbe,
mptcp, netdev, pabeni, syzbot+f3a31fb909db9b2a5c4d,
syzkaller-bugs
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Tue, 10 Sep 2024 17:58:56 +0800 you wrote:
> There are two paths to access mptcp_pm_del_add_timer, result in a race
> condition:
>
> CPU1 CPU2
> ==== ====
> net_rx_action
> napi_poll netlink_sendmsg
> __napi_poll netlink_unicast
> process_backlog netlink_unicast_kernel
> __netif_receive_skb genl_rcv
> __netif_receive_skb_one_core netlink_rcv_skb
> NF_HOOK genl_rcv_msg
> ip_local_deliver_finish genl_family_rcv_msg
> ip_protocol_deliver_rcu genl_family_rcv_msg_doit
> tcp_v4_rcv mptcp_pm_nl_flush_addrs_doit
> tcp_v4_do_rcv mptcp_nl_remove_addrs_list
> tcp_rcv_established mptcp_pm_remove_addrs_and_subflows
> tcp_data_queue remove_anno_list_by_saddr
> mptcp_incoming_options mptcp_pm_del_add_timer
> mptcp_pm_del_add_timer kfree(entry)
>
> [...]
Here is the summary with links:
- [net,V4] mptcp: pm: Fix uaf in __timer_delete_sync
https://git.kernel.org/netdev/net/c/b4cd80b03389
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] 36+ messages in thread
end of thread, other threads:[~2024-09-11 23:20 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-03 13:13 [syzbot] [mptcp?] KASAN: slab-use-after-free Read in __timer_delete_sync syzbot
2024-09-03 13:43 ` Edward Adam Davis
2024-09-03 14:09 ` syzbot
2024-09-03 13:52 ` Edward Adam Davis
2024-09-03 14:24 ` syzbot
2024-09-03 15:09 ` [PATCH] mptcp: pm: Fix uaf " Edward Adam Davis
2024-09-03 15:18 ` Eric Dumazet
2024-09-04 0:52 ` Edward Adam Davis
2024-09-04 1:01 ` [PATCH V2] " Edward Adam Davis
2024-09-04 20:39 ` Matthieu Baerts
2024-09-05 12:27 ` [PATCH net v3] " Edward Adam Davis
2024-09-06 18:55 ` Matthieu Baerts
2024-09-06 22:02 ` Jakub Kicinski
2024-09-09 7:01 ` Matthieu Baerts
2024-09-09 13:07 ` Paolo Abeni
2024-09-10 3:59 ` Edward Adam Davis
2024-09-10 9:58 ` [PATCH V4] " Edward Adam Davis
2024-09-10 9:58 ` [PATCH net " Edward Adam Davis
2024-09-10 11:27 ` Paolo Abeni
2024-09-10 14:29 ` Matthieu Baerts
2024-09-11 23:20 ` patchwork-bot+netdevbpf
2024-09-05 12:35 ` [PATCH V2] " Edward Adam Davis
2024-09-04 0:20 ` [syzbot] [mptcp?] KASAN: slab-use-after-free Read " Edward Adam Davis
2024-09-04 0:42 ` syzbot
2024-09-04 17:05 ` [PATCH net] mptcp: pm: Fix uaf " Matthieu Baerts (NGI0)
2024-09-05 0:46 ` [syzbot] [mptcp?] KASAN: slab-use-after-free Read " syzbot
2024-09-04 17:45 ` [PATCH net v2] mptcp: pm: Fix uaf " Matthieu Baerts (NGI0)
2024-09-05 1:10 ` [syzbot] [mptcp?] KASAN: slab-use-after-free Read " syzbot
2024-09-10 3:54 ` Edward Adam Davis
2024-09-10 6:56 ` syzbot
2024-09-10 7:10 ` Edward Adam Davis
2024-09-10 7:43 ` syzbot
2024-09-10 8:03 ` Edward Adam Davis
2024-09-10 8:32 ` syzbot
2024-09-10 9:32 ` Edward Adam Davis
2024-09-10 9:57 ` syzbot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox