NetBSD Problem Report #58520

From www@netbsd.org  Mon Jul 29 18:59:12 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)
	 client-signature RSA-PSS (2048 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id A630E1A923C
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 29 Jul 2024 18:59:12 +0000 (UTC)
Message-Id: <20240729185911.39CF31A923E@mollari.NetBSD.org>
Date: Mon, 29 Jul 2024 18:59:11 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: experimental wg(4) lacks barriers around access to packet pending initiation
X-Send-Pr-Version: www-1.0

>Number:         58520
>Category:       kern
>Synopsis:       experimental wg(4) lacks barriers around access to packet pending initiation
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jul 29 19:00:03 +0000 2024
>Closed-Date:    Wed Oct 09 14:39:32 +0000 2024
>Last-Modified:  Wed Oct 09 14:39:32 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current, 10
>Organization:
The NetWG Pendingmemoryation
>Environment:
>Description:
When there's no established session, wg_output stashes the packet the caller is trying to transmit in a single-entry queue before initiating a session:

   4241 		/*
   4242 		 * No established session.  If we're the first to try
   4243 		 * sending data, schedule a handshake and queue the
   4244 		 * packet for when the handshake is done; otherwise
   4245 		 * just drop the packet and let the ongoing handshake
   4246 		 * attempt continue.  We could queue more data packets
   4247 		 * but it's not clear that's worthwhile.
   4248 		 */
   4249 		if ((m = atomic_swap_ptr(&wgp->wgp_pending, m)) == NULL) {
   4250 			WG_TRACE("queued first packet; init handshake");
   4251 			wg_schedule_peer_task(wgp, WGP_TASK_SEND_INIT_MESSAGE);
https://nxr.netbsd.org/xref/src/sys/net/if_wg.c?r=1.122#4241

That way, once the session is established, the queued packet can be transmitted right away:

   2075 	/*
   2076 	 * If we had a data packet queued up, send it.
   2077 	 *
   2078 	 * If not, but we're the initiator, send a keepalive message --
   2079 	 * if we're the initiator we have to send something immediately
   2080 	 * or else the responder will never answer.
   2081 	 */
   2082 	if ((m = atomic_swap_ptr(&wgp->wgp_pending, NULL)) != NULL) {
   2083 		kpreempt_disable();
   2084 		const uint32_t h = curcpu()->ci_index; // pktq_rps_hash(m)
   2085 		M_SETCTX(m, wgp);
   2086 		if (__predict_false(!pktq_enqueue(wg_pktq, m, h))) {
   2087 			WGLOG(LOG_ERR, "%s: pktq full, dropping\n",
   2088 			    if_name(&wg->wg_if));
   2089 			m_freem(m);
   2090 		}
   2091 		kpreempt_enable();
https://nxr.netbsd.org/xref/src/sys/net/if_wg.c?r=1.122#2075

However, nothing guarantees the initialization of m in the caller of wg_output happens-before the use of m to transmit it.
>How-To-Repeat:
code inspection
>Fix:
membar_release and membar_acquire

>Release-Note:

>Audit-Trail:
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58520 CVS commit: src/sys/net
Date: Mon, 29 Jul 2024 19:44:23 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Mon Jul 29 19:44:23 UTC 2024

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

 Log Message:
 wg(4): Add missing barriers around wgp_pending access.

 PR kern/58520: experimental wg(4) lacks barriers around access to
 packet pending initiation


 To generate a diff of this commit:
 cvs rdiff -u -r1.124 -r1.125 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.

State-Changed-From-To: open->needs-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Mon, 29 Jul 2024 20:17:17 +0000
State-Changed-Why:
fixed in HEAD, needs pullup-10


State-Changed-From-To: needs-pullups->closed
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Wed, 09 Oct 2024 14:39:32 +0000
State-Changed-Why:
fixed in HEAD, pulled up to 10
pullup-10 #934 https://releng.netbsd.org/cgi-bin/req-10.cgi?show=934


>Unformatted:

Home	PR Database Search