NetBSD Problem Report #58688

From www@netbsd.org  Sun Sep 22 17:59:11 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 55F6F1A923D
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 22 Sep 2024 17:59:11 +0000 (UTC)
Message-Id: <20240922175909.E21FE1A9242@mollari.NetBSD.org>
Date: Sun, 22 Sep 2024 17:59:09 +0000 (UTC)
From: c@bow.st
Reply-To: c@bow.st
To: gnats-bugs@NetBSD.org
Subject: userland panic of kernel via wg(4) 
X-Send-Pr-Version: www-1.0

>Number:         58688
>Category:       kern
>Synopsis:       userland panic of kernel via wg(4)
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    riastradh
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Sep 22 18:00:01 +0000 2024
>Closed-Date:    Wed Oct 09 14:40:21 +0000 2024
>Last-Modified:  Wed Oct 09 14:40:21 +0000 2024
>Originator:     Mathew, Cherry G.
>Release:        10.0
>Organization:
>Environment:
>Description:
$ ifconfig wg0 create;wgconfig wg0 set listen-port 1234;ifconfig wg0 up;nc -v -u localhost 1234

Panics kernel - mbuf double free seems to be the reported cause.

the UDP read may be done via an unprivileged user.


>How-To-Repeat:
ifconfig wg0 create;wgconfig wg0 set listen-port 1234;ifconfig wg0 up;nc -v -u localhost 1234
>Fix:

>Release-Note:

>Audit-Trail:
From: Harold Gutch <logix@foobar.franken.de>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/58688: userland panic of kernel via wg(4)
Date: Sun, 22 Sep 2024 22:38:01 +0200

 Here is a stacktrace from a current from August 22 (amd64, GENERIC):

 [ 242.2881987] panic: mbuf 0xffffaa36ac1b6460 already freed
 [ 242.2881987] cpu0: Begin traceback...
 [ 242.2881987] vpanic() at netbsd:vpanic+0x171
 [ 242.2982396] panic() at netbsd:panic+0x3c
 [ 242.3081835] m_free() at netbsd:m_free+0xc9
 [ 242.3181815] m_freem() at netbsd:m_freem+0xe
 [ 242.3181815] udp6_input() at netbsd:udp6_input+0x98
 [ 242.3281758] ip6intr() at netbsd:ip6intr+0x812
 [ 242.3382961] softint_dispatch() at netbsd:softint_dispatch+0x112
 [ 242.3382961] DDB lost frame for netbsd:Xsoftintr+0x4c, trying 0xffffbd00288340f0
 [ 242.3482085] Xsoftintr() at netbsd:Xsoftintr+0x4c
 [ 242.3482085] --- interrupt ---
 [ 242.3482085] 0:
 [ 242.3482085] cpu0: End traceback...
 [ 242.3482085] fatal breakpoint trap in supervisor mode
 [ 242.3482085] trap type 1 code 0 rip 0xffffffff8023541d cs 0x8 rflags 0x202 cr2 0x7b16e9455ab0 ilevel 0x4 rsp 0xffffbd0028833d90
 [ 242.3482085] curlwp 0xffffaa36b11a3400 pid 0.3 lowest kstack 0xffffbd002882f2c0


   Harold

From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/58688: userland panic of kernel via wg(4)
Date: Sun, 22 Sep 2024 22:52:37 -0000 (UTC)

 gnats-admin@NetBSD.org ("Harold Gutch via gnats") writes:

 > [ 242.3181815] m_freem() at netbsd:m_freem+0xe
 > [ 242.3181815] udp6_input() at netbsd:udp6_input+0x98

 wg_overudp_cb frees an mbuf and returns -1, but doesn't clear *mp.
 udp6_realinput just passes through the condition as rcvcnt = -1.
 udp6_input frees the mbuf again.

 int
 udp6_input(struct mbuf **mp)
 {
 	struct mbuf *m = *mp;

 	if (udp6_realinput(&m) == 0) {
 		...
 		m = NULL;
 	}

 	m_freem(m);
 }

 int
 udp6_realinput(struct mbuf **mp)
 {
 	...
 	ret = inp->inp_overudp_cb(mp);
 	switch (ret) {
 	case -1: /* Error, m was freed */
 		rcvcnt = -1;
 		goto bad;
 	...
 	}

 bad:
 	return recvcnt;
 }

 int
 wg_overudp_cb(struct mbuf **mp)
 {
 	struct mbuf *m = *mp;

 	...
 	if (...) {
 		m_freem(m);
 		return -1;
 	}
 	...
 }


 An unrelated problem in udp6_realinput, the *mp value isn't
 cached again in m.

                         case 0: /* plain UDP */
                         default: /* Unexpected */
                                 /*
                                  * Normal UDP processing will take place,
                                  * m may have changed.
                                  */
                                 break;
 			...

 		}

                 udp6_sendup(m, off, sin6tosa(src), inp->inp_socket);

 This only works because the overudp routine in wg(4) does not change m
 when returning zero.

Responsible-Changed-From-To: kern-bug-people->riastradh
Responsible-Changed-By: riastradh@NetBSD.org
Responsible-Changed-When: Mon, 23 Sep 2024 09:04:33 +0000
Responsible-Changed-Why:
mine


From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58688 CVS commit: src/tests/net/if_wg
Date: Tue, 8 Oct 2024 02:28:43 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Tue Oct  8 02:28:43 UTC 2024

 Modified Files:
 	src/tests/net/if_wg: t_basic.sh

 Log Message:
 wg(4): Test truncated UDP input from the network.

 This triggers double-free in the IPv6 udp6_input path -- but,
 confusingly, not the IPv4 udp_input path, even though the overudp_cb
 interface ought to be the same:

 /* udp_input -- no further use of m if return is -1 */
 	if ((n = udp4_realinput(&src, &dst, &m, iphlen)) == -1) {
 		UDP_STATINC(UDP_STAT_HDROPS);
 		return;
 	}

 /* udp6_input -- m_freem if return is not 0 */
 	if (udp6_realinput(AF_INET6, &src, &dst, &m, off) == 0) {
 ...
 	}

 bad:
 	m_freem(m);
 	return IPPROTO_DONE;

 The subroutines udp4_realinput and udp6_realinput pass through the
 return value of overudp_cb in essentially the same way:

 /* udp4_realinput */
 		if (inp->inp_overudp_cb != NULL) {
 			int ret;
 			ret = inp->inp_overudp_cb(mp, off, inp->inp_socket,
 			    sintosa(src), inp->inp_overudp_arg);
 			switch (ret) {
 			case -1: /* Error, m was freed */
 				rcvcnt = -1;
 				goto bad;
 ...
 bad:
 	return rcvcnt;

 /* udp6_realinput */
 		if (inp->inp_overudp_cb != NULL) {
 			int ret;
 			ret = inp->inp_overudp_cb(mp, off, inp->inp_socket,
 			    sin6tosa(src), inp->inp_overudp_arg);
 			switch (ret) {
 			case -1: /* Error, m was freed */
 				rcvcnt = -1;
 				goto bad;
 ...
 bad:
 	return rcvcnt;

 PR kern/58688: userland panic of kernel via wg(4)


 To generate a diff of this commit:
 cvs rdiff -u -r1.4 -r1.5 src/tests/net/if_wg/t_basic.sh

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

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58688 CVS commit: src
Date: Tue, 8 Oct 2024 02:29:40 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Tue Oct  8 02:29:40 UTC 2024

 Modified Files:
 	src/sys/net: if_wg.c
 	src/tests/net/if_wg: t_basic.sh

 Log Message:
 wg(4): Fix wg_overudp_cb drop paths to null out *mp as caller needs.

 PR kern/58688: userland panic of kernel via wg(4)


 To generate a diff of this commit:
 cvs rdiff -u -r1.131 -r1.132 src/sys/net/if_wg.c
 cvs rdiff -u -r1.5 -r1.6 src/tests/net/if_wg/t_basic.sh

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

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58688 CVS commit: src/sys
Date: Tue, 8 Oct 2024 02:30:05 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Tue Oct  8 02:30:05 UTC 2024

 Modified Files:
 	src/sys/netinet: udp_usrreq.c
 	src/sys/netinet6: udp6_usrreq.c

 Log Message:
 udp(4): Clarify udp4/6_espinudp and inp_overudp_cb return.

 Cleanup to detect problems like this earlier:

 PR kern/58688: userland panic of kernel via wg(4)


 To generate a diff of this commit:
 cvs rdiff -u -r1.265 -r1.266 src/sys/netinet/udp_usrreq.c
 cvs rdiff -u -r1.155 -r1.156 src/sys/netinet6/udp6_usrreq.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: Tue, 08 Oct 2024 02:38:58 +0000
State-Changed-Why:
fixed in HEAD, needs pullup-10 (once the testbed runs and confirms)


State-Changed-From-To: needs-pullups->closed
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Wed, 09 Oct 2024 14:40:21 +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