aboutsummaryrefslogtreecommitdiffstats
path: root/target/linux/generic/backport-5.4/080-wireguard-0082-wireguard-noise-reject-peers-with-low-order-public-k.patch
diff options
context:
space:
mode:
authorJason A. Donenfeld <Jason@zx2c4.com>2021-03-04 13:37:13 -0700
committerPetr Štetiar <ynezz@true.cz>2021-03-04 22:06:53 +0100
commit196f3d586f11d96ba4ab60068cfb12420bcd20fd (patch)
tree4f4eb9cd8c8a229972792286361b5bff8743d993 /target/linux/generic/backport-5.4/080-wireguard-0082-wireguard-noise-reject-peers-with-low-order-public-k.patch
parent1d412235a57f3359d1c7bb63bfebce50c56caee6 (diff)
downloadupstream-196f3d586f11d96ba4ab60068cfb12420bcd20fd.tar.gz
upstream-196f3d586f11d96ba4ab60068cfb12420bcd20fd.tar.bz2
upstream-196f3d586f11d96ba4ab60068cfb12420bcd20fd.zip
kernel-5.4: bump to 5.4.102 and refresh patches
5.4.102 backported a lot of stuff that our WireGuard backport already did, in addition to other patches we had, so those patches were removed from that part of the series. In the process other patches were refreshed or reworked to account for upstream changes. This commit involved `update_kernel.sh -v -u 5.4`. Cc: John Audia <graysky@archlinux.us> Cc: David Bauer <mail@david-bauer.net> Cc: Petr Štetiar <ynezz@true.cz> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Diffstat (limited to 'target/linux/generic/backport-5.4/080-wireguard-0082-wireguard-noise-reject-peers-with-low-order-public-k.patch')
-rw-r--r--target/linux/generic/backport-5.4/080-wireguard-0082-wireguard-noise-reject-peers-with-low-order-public-k.patch233
1 files changed, 233 insertions, 0 deletions
diff --git a/target/linux/generic/backport-5.4/080-wireguard-0082-wireguard-noise-reject-peers-with-low-order-public-k.patch b/target/linux/generic/backport-5.4/080-wireguard-0082-wireguard-noise-reject-peers-with-low-order-public-k.patch
new file mode 100644
index 0000000000..55bb276118
--- /dev/null
+++ b/target/linux/generic/backport-5.4/080-wireguard-0082-wireguard-noise-reject-peers-with-low-order-public-k.patch
@@ -0,0 +1,233 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: "Jason A. Donenfeld" <Jason@zx2c4.com>
+Date: Tue, 4 Feb 2020 22:17:26 +0100
+Subject: [PATCH] wireguard: noise: reject peers with low order public keys
+
+commit ec31c2676a10e064878927b243fada8c2fb0c03c upstream.
+
+Our static-static calculation returns a failure if the public key is of
+low order. We check for this when peers are added, and don't allow them
+to be added if they're low order, except in the case where we haven't
+yet been given a private key. In that case, we would defer the removal
+of the peer until we're given a private key, since at that point we're
+doing new static-static calculations which incur failures we can act on.
+This meant, however, that we wound up removing peers rather late in the
+configuration flow.
+
+Syzkaller points out that peer_remove calls flush_workqueue, which in
+turn might then wait for sending a handshake initiation to complete.
+Since handshake initiation needs the static identity lock, holding the
+static identity lock while calling peer_remove can result in a rare
+deadlock. We have precisely this case in this situation of late-stage
+peer removal based on an invalid public key. We can't drop the lock when
+removing, because then incoming handshakes might interact with a bogus
+static-static calculation.
+
+While the band-aid patch for this would involve breaking up the peer
+removal into two steps like wg_peer_remove_all does, in order to solve
+the locking issue, there's actually a much more elegant way of fixing
+this:
+
+If the static-static calculation succeeds with one private key, it
+*must* succeed with all others, because all 32-byte strings map to valid
+private keys, thanks to clamping. That means we can get rid of this
+silly dance and locking headaches of removing peers late in the
+configuration flow, and instead just reject them early on, regardless of
+whether the device has yet been assigned a private key. For the case
+where the device doesn't yet have a private key, we safely use zeros
+just for the purposes of checking for low order points by way of
+checking the output of the calculation.
+
+The following PoC will trigger the deadlock:
+
+ip link add wg0 type wireguard
+ip addr add 10.0.0.1/24 dev wg0
+ip link set wg0 up
+ping -f 10.0.0.2 &
+while true; do
+ wg set wg0 private-key /dev/null peer AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= allowed-ips 10.0.0.0/24 endpoint 10.0.0.3:1234
+ wg set wg0 private-key <(echo AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=)
+done
+
+[ 0.949105] ======================================================
+[ 0.949550] WARNING: possible circular locking dependency detected
+[ 0.950143] 5.5.0-debug+ #18 Not tainted
+[ 0.950431] ------------------------------------------------------
+[ 0.950959] wg/89 is trying to acquire lock:
+[ 0.951252] ffff8880333e2128 ((wq_completion)wg-kex-wg0){+.+.}, at: flush_workqueue+0xe3/0x12f0
+[ 0.951865]
+[ 0.951865] but task is already holding lock:
+[ 0.952280] ffff888032819bc0 (&wg->static_identity.lock){++++}, at: wg_set_device+0x95d/0xcc0
+[ 0.953011]
+[ 0.953011] which lock already depends on the new lock.
+[ 0.953011]
+[ 0.953651]
+[ 0.953651] the existing dependency chain (in reverse order) is:
+[ 0.954292]
+[ 0.954292] -> #2 (&wg->static_identity.lock){++++}:
+[ 0.954804] lock_acquire+0x127/0x350
+[ 0.955133] down_read+0x83/0x410
+[ 0.955428] wg_noise_handshake_create_initiation+0x97/0x700
+[ 0.955885] wg_packet_send_handshake_initiation+0x13a/0x280
+[ 0.956401] wg_packet_handshake_send_worker+0x10/0x20
+[ 0.956841] process_one_work+0x806/0x1500
+[ 0.957167] worker_thread+0x8c/0xcb0
+[ 0.957549] kthread+0x2ee/0x3b0
+[ 0.957792] ret_from_fork+0x24/0x30
+[ 0.958234]
+[ 0.958234] -> #1 ((work_completion)(&peer->transmit_handshake_work)){+.+.}:
+[ 0.958808] lock_acquire+0x127/0x350
+[ 0.959075] process_one_work+0x7ab/0x1500
+[ 0.959369] worker_thread+0x8c/0xcb0
+[ 0.959639] kthread+0x2ee/0x3b0
+[ 0.959896] ret_from_fork+0x24/0x30
+[ 0.960346]
+[ 0.960346] -> #0 ((wq_completion)wg-kex-wg0){+.+.}:
+[ 0.960945] check_prev_add+0x167/0x1e20
+[ 0.961351] __lock_acquire+0x2012/0x3170
+[ 0.961725] lock_acquire+0x127/0x350
+[ 0.961990] flush_workqueue+0x106/0x12f0
+[ 0.962280] peer_remove_after_dead+0x160/0x220
+[ 0.962600] wg_set_device+0xa24/0xcc0
+[ 0.962994] genl_rcv_msg+0x52f/0xe90
+[ 0.963298] netlink_rcv_skb+0x111/0x320
+[ 0.963618] genl_rcv+0x1f/0x30
+[ 0.963853] netlink_unicast+0x3f6/0x610
+[ 0.964245] netlink_sendmsg+0x700/0xb80
+[ 0.964586] __sys_sendto+0x1dd/0x2c0
+[ 0.964854] __x64_sys_sendto+0xd8/0x1b0
+[ 0.965141] do_syscall_64+0x90/0xd9a
+[ 0.965408] entry_SYSCALL_64_after_hwframe+0x49/0xbe
+[ 0.965769]
+[ 0.965769] other info that might help us debug this:
+[ 0.965769]
+[ 0.966337] Chain exists of:
+[ 0.966337] (wq_completion)wg-kex-wg0 --> (work_completion)(&peer->transmit_handshake_work) --> &wg->static_identity.lock
+[ 0.966337]
+[ 0.967417] Possible unsafe locking scenario:
+[ 0.967417]
+[ 0.967836] CPU0 CPU1
+[ 0.968155] ---- ----
+[ 0.968497] lock(&wg->static_identity.lock);
+[ 0.968779] lock((work_completion)(&peer->transmit_handshake_work));
+[ 0.969345] lock(&wg->static_identity.lock);
+[ 0.969809] lock((wq_completion)wg-kex-wg0);
+[ 0.970146]
+[ 0.970146] *** DEADLOCK ***
+[ 0.970146]
+[ 0.970531] 5 locks held by wg/89:
+[ 0.970908] #0: ffffffff827433c8 (cb_lock){++++}, at: genl_rcv+0x10/0x30
+[ 0.971400] #1: ffffffff82743480 (genl_mutex){+.+.}, at: genl_rcv_msg+0x642/0xe90
+[ 0.971924] #2: ffffffff827160c0 (rtnl_mutex){+.+.}, at: wg_set_device+0x9f/0xcc0
+[ 0.972488] #3: ffff888032819de0 (&wg->device_update_lock){+.+.}, at: wg_set_device+0xb0/0xcc0
+[ 0.973095] #4: ffff888032819bc0 (&wg->static_identity.lock){++++}, at: wg_set_device+0x95d/0xcc0
+[ 0.973653]
+[ 0.973653] stack backtrace:
+[ 0.973932] CPU: 1 PID: 89 Comm: wg Not tainted 5.5.0-debug+ #18
+[ 0.974476] Call Trace:
+[ 0.974638] dump_stack+0x97/0xe0
+[ 0.974869] check_noncircular+0x312/0x3e0
+[ 0.975132] ? print_circular_bug+0x1f0/0x1f0
+[ 0.975410] ? __kernel_text_address+0x9/0x30
+[ 0.975727] ? unwind_get_return_address+0x51/0x90
+[ 0.976024] check_prev_add+0x167/0x1e20
+[ 0.976367] ? graph_lock+0x70/0x160
+[ 0.976682] __lock_acquire+0x2012/0x3170
+[ 0.976998] ? register_lock_class+0x1140/0x1140
+[ 0.977323] lock_acquire+0x127/0x350
+[ 0.977627] ? flush_workqueue+0xe3/0x12f0
+[ 0.977890] flush_workqueue+0x106/0x12f0
+[ 0.978147] ? flush_workqueue+0xe3/0x12f0
+[ 0.978410] ? find_held_lock+0x2c/0x110
+[ 0.978662] ? lock_downgrade+0x6e0/0x6e0
+[ 0.978919] ? queue_rcu_work+0x60/0x60
+[ 0.979166] ? netif_napi_del+0x151/0x3b0
+[ 0.979501] ? peer_remove_after_dead+0x160/0x220
+[ 0.979871] peer_remove_after_dead+0x160/0x220
+[ 0.980232] wg_set_device+0xa24/0xcc0
+[ 0.980516] ? deref_stack_reg+0x8e/0xc0
+[ 0.980801] ? set_peer+0xe10/0xe10
+[ 0.981040] ? __ww_mutex_check_waiters+0x150/0x150
+[ 0.981430] ? __nla_validate_parse+0x163/0x270
+[ 0.981719] ? genl_family_rcv_msg_attrs_parse+0x13f/0x310
+[ 0.982078] genl_rcv_msg+0x52f/0xe90
+[ 0.982348] ? genl_family_rcv_msg_attrs_parse+0x310/0x310
+[ 0.982690] ? register_lock_class+0x1140/0x1140
+[ 0.983049] netlink_rcv_skb+0x111/0x320
+[ 0.983298] ? genl_family_rcv_msg_attrs_parse+0x310/0x310
+[ 0.983645] ? netlink_ack+0x880/0x880
+[ 0.983888] genl_rcv+0x1f/0x30
+[ 0.984168] netlink_unicast+0x3f6/0x610
+[ 0.984443] ? netlink_detachskb+0x60/0x60
+[ 0.984729] ? find_held_lock+0x2c/0x110
+[ 0.984976] netlink_sendmsg+0x700/0xb80
+[ 0.985220] ? netlink_broadcast_filtered+0xa60/0xa60
+[ 0.985533] __sys_sendto+0x1dd/0x2c0
+[ 0.985763] ? __x64_sys_getpeername+0xb0/0xb0
+[ 0.986039] ? sockfd_lookup_light+0x17/0x160
+[ 0.986397] ? __sys_recvmsg+0x8c/0xf0
+[ 0.986711] ? __sys_recvmsg_sock+0xd0/0xd0
+[ 0.987018] __x64_sys_sendto+0xd8/0x1b0
+[ 0.987283] ? lockdep_hardirqs_on+0x39b/0x5a0
+[ 0.987666] do_syscall_64+0x90/0xd9a
+[ 0.987903] entry_SYSCALL_64_after_hwframe+0x49/0xbe
+[ 0.988223] RIP: 0033:0x7fe77c12003e
+[ 0.988508] Code: c3 8b 07 85 c0 75 24 49 89 fb 48 89 f0 48 89 d7 48 89 ce 4c 89 c2 4d 89 ca 4c 8b 44 24 08 4c 8b 4c 24 10 4c 4
+[ 0.989666] RSP: 002b:00007fffada2ed58 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
+[ 0.990137] RAX: ffffffffffffffda RBX: 00007fe77c159d48 RCX: 00007fe77c12003e
+[ 0.990583] RDX: 0000000000000040 RSI: 000055fd1d38e020 RDI: 0000000000000004
+[ 0.991091] RBP: 000055fd1d38e020 R08: 000055fd1cb63358 R09: 000000000000000c
+[ 0.991568] R10: 0000000000000000 R11: 0000000000000246 R12: 000000000000002c
+[ 0.992014] R13: 0000000000000004 R14: 000055fd1d38e020 R15: 0000000000000001
+
+Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
+Reported-by: syzbot <syzkaller@googlegroups.com>
+Signed-off-by: David S. Miller <davem@davemloft.net>
+Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
+---
+ drivers/net/wireguard/netlink.c | 6 ++----
+ drivers/net/wireguard/noise.c | 10 +++++++---
+ 2 files changed, 9 insertions(+), 7 deletions(-)
+
+--- a/drivers/net/wireguard/netlink.c
++++ b/drivers/net/wireguard/netlink.c
+@@ -575,10 +575,8 @@ static int wg_set_device(struct sk_buff
+ private_key);
+ list_for_each_entry_safe(peer, temp, &wg->peer_list,
+ peer_list) {
+- if (wg_noise_precompute_static_static(peer))
+- wg_noise_expire_current_peer_keypairs(peer);
+- else
+- wg_peer_remove(peer);
++ BUG_ON(!wg_noise_precompute_static_static(peer));
++ wg_noise_expire_current_peer_keypairs(peer);
+ }
+ wg_cookie_checker_precompute_device_keys(&wg->cookie_checker);
+ up_write(&wg->static_identity.lock);
+--- a/drivers/net/wireguard/noise.c
++++ b/drivers/net/wireguard/noise.c
+@@ -46,17 +46,21 @@ void __init wg_noise_init(void)
+ /* Must hold peer->handshake.static_identity->lock */
+ bool wg_noise_precompute_static_static(struct wg_peer *peer)
+ {
+- bool ret = true;
++ bool ret;
+
+ down_write(&peer->handshake.lock);
+- if (peer->handshake.static_identity->has_identity)
++ if (peer->handshake.static_identity->has_identity) {
+ ret = curve25519(
+ peer->handshake.precomputed_static_static,
+ peer->handshake.static_identity->static_private,
+ peer->handshake.remote_static);
+- else
++ } else {
++ u8 empty[NOISE_PUBLIC_KEY_LEN] = { 0 };
++
++ ret = curve25519(empty, empty, peer->handshake.remote_static);
+ memset(peer->handshake.precomputed_static_static, 0,
+ NOISE_PUBLIC_KEY_LEN);
++ }
+ up_write(&peer->handshake.lock);
+ return ret;
+ }