From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: netdev@vger.kernel.org, davem@davemloft.net, kuba@kernel.org
Subject: [PATCH net 4/6] wireguard: netlink: check for dangling peer via is_dead instead of empty list
Date: Thu, 14 Mar 2024 16:49:09 -0600 [thread overview]
Message-ID: <20240314224911.6653-5-Jason@zx2c4.com> (raw)
In-Reply-To: <20240314224911.6653-1-Jason@zx2c4.com>
If all peers are removed via wg_peer_remove_all(), rather than setting
peer_list to empty, the peer is added to a temporary list with a head on
the stack of wg_peer_remove_all(). If a netlink dump is resumed and the
cursored peer is one that has been removed via wg_peer_remove_all(), it
will iterate from that peer and then attempt to dump freed peers.
Fix this by instead checking peer->is_dead, which was explictly created
for this purpose. Also move up the device_update_lock lockdep assertion,
since reading is_dead relies on that.
It can be reproduced by a small script like:
echo "Setting config..."
ip link add dev wg0 type wireguard
wg setconf wg0 /big-config
(
while true; do
echo "Showing config..."
wg showconf wg0 > /dev/null
done
) &
sleep 4
wg setconf wg0 <(printf "[Peer]\nPublicKey=$(wg genkey)\n")
Resulting in:
BUG: KASAN: slab-use-after-free in __lock_acquire+0x182a/0x1b20
Read of size 8 at addr ffff88811956ec70 by task wg/59
CPU: 2 PID: 59 Comm: wg Not tainted 6.8.0-rc2-debug+ #5
Call Trace:
<TASK>
dump_stack_lvl+0x47/0x70
print_address_description.constprop.0+0x2c/0x380
print_report+0xab/0x250
kasan_report+0xba/0xf0
__lock_acquire+0x182a/0x1b20
lock_acquire+0x191/0x4b0
down_read+0x80/0x440
get_peer+0x140/0xcb0
wg_get_device_dump+0x471/0x1130
Cc: stable@vger.kernel.org
Fixes: e7096c131e51 ("net: WireGuard secure network tunnel")
Reported-by: Lillian Berry <lillian@star-ark.net>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
drivers/net/wireguard/netlink.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireguard/netlink.c b/drivers/net/wireguard/netlink.c
index e220d761b1f2..c17aee454fa3 100644
--- a/drivers/net/wireguard/netlink.c
+++ b/drivers/net/wireguard/netlink.c
@@ -255,17 +255,17 @@ static int wg_get_device_dump(struct sk_buff *skb, struct netlink_callback *cb)
if (!peers_nest)
goto out;
ret = 0;
- /* If the last cursor was removed via list_del_init in peer_remove, then
+ lockdep_assert_held(&wg->device_update_lock);
+ /* If the last cursor was removed in peer_remove or peer_remove_all, then
* we just treat this the same as there being no more peers left. The
* reason is that seq_nr should indicate to userspace that this isn't a
* coherent dump anyway, so they'll try again.
*/
if (list_empty(&wg->peer_list) ||
- (ctx->next_peer && list_empty(&ctx->next_peer->peer_list))) {
+ (ctx->next_peer && ctx->next_peer->is_dead)) {
nla_nest_cancel(skb, peers_nest);
goto out;
}
- lockdep_assert_held(&wg->device_update_lock);
peer = list_prepare_entry(ctx->next_peer, &wg->peer_list, peer_list);
list_for_each_entry_continue(peer, &wg->peer_list, peer_list) {
if (get_peer(peer, skb, ctx)) {
--
2.44.0
next prev parent reply other threads:[~2024-03-14 22:49 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-14 22:49 [PATCH net 0/6] wireguard fixes for 6.9-rc1 Jason A. Donenfeld
2024-03-14 22:49 ` [PATCH net 1/6] wireguard: receive: annotate data-race around receiving_counter.counter Jason A. Donenfeld
2024-03-14 22:49 ` [PATCH net 2/6] wireguard: device: leverage core stats allocator Jason A. Donenfeld
2024-03-14 22:49 ` [PATCH net 3/6] wireguard: device: remove generic .ndo_get_stats64 Jason A. Donenfeld
2024-03-14 22:49 ` Jason A. Donenfeld [this message]
2024-03-14 22:49 ` [PATCH net 5/6] wireguard: netlink: access device through ctx instead of peer Jason A. Donenfeld
2024-03-14 22:49 ` [PATCH net 6/6] wireguard: selftests: set RISCV_ISA_FALLBACK on riscv{32,64} Jason A. Donenfeld
2024-03-18 11:31 ` [PATCH net 0/6] wireguard fixes for 6.9-rc1 Jiri Pirko
2024-03-19 10:30 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240314224911.6653-5-Jason@zx2c4.com \
--to=jason@zx2c4.com \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).