diff options
Diffstat (limited to 'target/linux/generic/backport-5.4/080-wireguard-0114-wireguard-noise-take-lock-when-removing-handshake-en.patch')
| -rw-r--r-- | target/linux/generic/backport-5.4/080-wireguard-0114-wireguard-noise-take-lock-when-removing-handshake-en.patch | 127 |
1 files changed, 0 insertions, 127 deletions
diff --git a/target/linux/generic/backport-5.4/080-wireguard-0114-wireguard-noise-take-lock-when-removing-handshake-en.patch b/target/linux/generic/backport-5.4/080-wireguard-0114-wireguard-noise-take-lock-when-removing-handshake-en.patch deleted file mode 100644 index e80528c91b..0000000000 --- a/target/linux/generic/backport-5.4/080-wireguard-0114-wireguard-noise-take-lock-when-removing-handshake-en.patch +++ /dev/null @@ -1,127 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: "Jason A. Donenfeld" <Jason@zx2c4.com> -Date: Wed, 9 Sep 2020 13:58:14 +0200 -Subject: [PATCH] wireguard: noise: take lock when removing handshake entry - from table - -commit 9179ba31367bcf481c3c79b5f028c94faad9f30a upstream. - -Eric reported that syzkaller found a race of this variety: - -CPU 1 CPU 2 --------------------------------------------|--------------------------------------- -wg_index_hashtable_replace(old, ...) | - if (hlist_unhashed(&old->index_hash)) | - | wg_index_hashtable_remove(old) - | hlist_del_init_rcu(&old->index_hash) - | old->index_hash.pprev = NULL - hlist_replace_rcu(&old->index_hash, ...) | - *old->index_hash.pprev | - -Syzbot wasn't actually able to reproduce this more than once or create a -reproducer, because the race window between checking "hlist_unhashed" and -calling "hlist_replace_rcu" is just so small. Adding an mdelay(5) or -similar there helps make this demonstrable using this simple script: - - #!/bin/bash - set -ex - trap 'kill $pid1; kill $pid2; ip link del wg0; ip link del wg1' EXIT - ip link add wg0 type wireguard - ip link add wg1 type wireguard - wg set wg0 private-key <(wg genkey) listen-port 9999 - wg set wg1 private-key <(wg genkey) peer $(wg show wg0 public-key) endpoint 127.0.0.1:9999 persistent-keepalive 1 - wg set wg0 peer $(wg show wg1 public-key) - ip link set wg0 up - yes link set wg1 up | ip -force -batch - & - pid1=$! - yes link set wg1 down | ip -force -batch - & - pid2=$! - wait - -The fundumental underlying problem is that we permit calls to wg_index_ -hashtable_remove(handshake.entry) without requiring the caller to take -the handshake mutex that is intended to protect members of handshake -during mutations. This is consistently the case with calls to wg_index_ -hashtable_insert(handshake.entry) and wg_index_hashtable_replace( -handshake.entry), but it's missing from a pertinent callsite of wg_ -index_hashtable_remove(handshake.entry). So, this patch makes sure that -mutex is taken. - -The original code was a little bit funky though, in the form of: - - remove(handshake.entry) - lock(), memzero(handshake.some_members), unlock() - remove(handshake.entry) - -The original intention of that double removal pattern outside the lock -appears to be some attempt to prevent insertions that might happen while -locks are dropped during expensive crypto operations, but actually, all -callers of wg_index_hashtable_insert(handshake.entry) take the write -lock and then explicitly check handshake.state, as they should, which -the aforementioned memzero clears, which means an insertion should -already be impossible. And regardless, the original intention was -necessarily racy, since it wasn't guaranteed that something else would -run after the unlock() instead of after the remove(). So, from a -soundness perspective, it seems positive to remove what looks like a -hack at best. - -The crash from both syzbot and from the script above is as follows: - - general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN - KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] - CPU: 0 PID: 7395 Comm: kworker/0:3 Not tainted 5.9.0-rc4-syzkaller #0 - Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 - Workqueue: wg-kex-wg1 wg_packet_handshake_receive_worker - RIP: 0010:hlist_replace_rcu include/linux/rculist.h:505 [inline] - RIP: 0010:wg_index_hashtable_replace+0x176/0x330 drivers/net/wireguard/peerlookup.c:174 - Code: 00 fc ff df 48 89 f9 48 c1 e9 03 80 3c 01 00 0f 85 44 01 00 00 48 b9 00 00 00 00 00 fc ff df 48 8b 45 10 48 89 c6 48 c1 ee 03 <80> 3c 0e 00 0f 85 06 01 00 00 48 85 d2 4c 89 28 74 47 e8 a3 4f b5 - RSP: 0018:ffffc90006a97bf8 EFLAGS: 00010246 - RAX: 0000000000000000 RBX: ffff888050ffc4f8 RCX: dffffc0000000000 - RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff88808e04e010 - RBP: ffff88808e04e000 R08: 0000000000000001 R09: ffff8880543d0000 - R10: ffffed100a87a000 R11: 000000000000016e R12: ffff8880543d0000 - R13: ffff88808e04e008 R14: ffff888050ffc508 R15: ffff888050ffc500 - FS: 0000000000000000(0000) GS:ffff8880ae600000(0000) knlGS:0000000000000000 - CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 - CR2: 00000000f5505db0 CR3: 0000000097cf7000 CR4: 00000000001526f0 - DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 - DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 - Call Trace: - wg_noise_handshake_begin_session+0x752/0xc9a drivers/net/wireguard/noise.c:820 - wg_receive_handshake_packet drivers/net/wireguard/receive.c:183 [inline] - wg_packet_handshake_receive_worker+0x33b/0x730 drivers/net/wireguard/receive.c:220 - process_one_work+0x94c/0x1670 kernel/workqueue.c:2269 - worker_thread+0x64c/0x1120 kernel/workqueue.c:2415 - kthread+0x3b5/0x4a0 kernel/kthread.c:292 - ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294 - -Reported-by: syzbot <syzkaller@googlegroups.com> -Reported-by: Eric Dumazet <edumazet@google.com> -Link: https://lore.kernel.org/wireguard/20200908145911.4090480-1-edumazet@google.com/ -Fixes: e7096c131e51 ("net: WireGuard secure network tunnel") -Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> -Signed-off-by: David S. Miller <davem@davemloft.net> -Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> ---- - drivers/net/wireguard/noise.c | 5 +---- - 1 file changed, 1 insertion(+), 4 deletions(-) - ---- a/drivers/net/wireguard/noise.c -+++ b/drivers/net/wireguard/noise.c -@@ -87,15 +87,12 @@ static void handshake_zero(struct noise_ - - void wg_noise_handshake_clear(struct noise_handshake *handshake) - { -+ down_write(&handshake->lock); - wg_index_hashtable_remove( - handshake->entry.peer->device->index_hashtable, - &handshake->entry); -- down_write(&handshake->lock); - handshake_zero(handshake); - up_write(&handshake->lock); -- wg_index_hashtable_remove( -- handshake->entry.peer->device->index_hashtable, -- &handshake->entry); - } - - static struct noise_keypair *keypair_create(struct wg_peer *peer) |