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:
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, 0 insertions, 233 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
deleted file mode 100644
index 55bb276118..0000000000
--- a/target/linux/generic/backport-5.4/080-wireguard-0082-wireguard-noise-reject-peers-with-low-order-public-k.patch
+++ /dev/null
@@ -1,233 +0,0 @@
-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;
- }