NetBSD Problem Report #58477

From www@netbsd.org  Sun Jul 28 11:24:30 2024
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
	 client-signature RSA-PSS (2048 bits) client-digest SHA256)
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 73B681A923C
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 28 Jul 2024 11:24:30 +0000 (UTC)
Message-Id: <20240728112429.13E1B1A923E@mollari.NetBSD.org>
Date: Sun, 28 Jul 2024 11:24:29 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: experimental wg(4) ALTQ support is probably buggy
X-Send-Pr-Version: www-1.0

>Number:         58477
>Category:       kern
>Synopsis:       experimental wg(4) ALTQ support is probably buggy
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Jul 28 11:25:00 +0000 2024
>Last-Modified:  Sun Jul 28 14:50:29 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current, 10
>Organization:
The AltWGQ Foundation
>Environment:
>Description:
When using ALTQ, packets in-flight to a peer may be stored in the ifp->if_snd queue, not just in wg_pktq and in the peer's wgp->wgp_pending.

When destroying a peer, we need to make sure there are no more references to it by packets in-flight, in wg_purge_pending_packets.  Currently this clears wgp->wgp_pending and waits for wg_pktq, but doesn't do anything about the packets in ifp->if_snd, so using wg(4) with ALTQ may lead to use-after-free.
>How-To-Repeat:
use wg(4) with ALTQ and remove peers during traffic
>Fix:
Not sure there's any good way to drop just those packets destined to a particular peer, so it's probably easiest to just do wg_start(&wgp->wgp_sc->wg_if) to wait for all queued packets to be processed.

>Audit-Trail:
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58477 CVS commit: src/sys/net
Date: Sun, 28 Jul 2024 14:48:47 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Sun Jul 28 14:48:47 UTC 2024

 Modified Files:
 	src/sys/net: if_wg.c

 Log Message:
 wg(4): Process all altq'd packets when deleting peer.

 Can't just drop them because we can only go through all packets on an
 interface at a time, for all peers -- so we'd either have to drop all
 peers' packets, or requeue the packets for other peers.  Probably not
 worth the trouble, so let's just wait for all the packets currently
 queued up to go through first.

 This requires reordering teardown so that we wg_destroy_all_peers,
 and thus wg_purge_pending_packets, _before_ we wg_if_detach, because
 wg_if_detach -> if_detach destroys the lock that IFQ_DEQUEUE uses.

 PR kern/58477: experimental wg(4) ALTQ support is probably buggy


 To generate a diff of this commit:
 cvs rdiff -u -r1.106 -r1.107 src/sys/net/if_wg.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Home	PR Database Search