NetBSD Problem Report #59220

From www@netbsd.org  Wed Mar 26 16:17:33 2025
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 D0F481A9239
	for <gnats-bugs@gnats.NetBSD.org>; Wed, 26 Mar 2025 16:17:33 +0000 (UTC)
Message-Id: <20250326161732.623701A923D@mollari.NetBSD.org>
Date: Wed, 26 Mar 2025 16:17:32 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: accept(2): null pointer deref
X-Send-Pr-Version: www-1.0

>Number:         59220
>Category:       kern
>Synopsis:       accept(2): null pointer deref
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          needs-pullups
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Mar 26 16:20:00 +0000 2025
>Closed-Date:    
>Last-Modified:  Thu Mar 27 11:05:02 +0000 2025
>Originator:     Taylor R Campbell
>Release:        current, 10, 9, ...
>Organization:
The NetLOCAL_CONNWAIT Foundacrash
>Environment:
>Description:
uvm_fault(0x..., 0x0, 1) -> e
fatal page fault in supervisor mode
trap type 6 code 0 rip 0x... cs 0x8 rflags 0x10202 cr2 0 ilevel 0 rsp 0x...
curlwp 0x... pid 558.558 lowest kstack 0x...
kernel page fault trap, code=0
Stopped in pid 558.558 (t_unix) at      netbsd:solocked2+0x3:   movq    0(%rsi),%rdi
solocked2() at netbsd:solocked2+0x3
do_sys_accept() at netbsd:do_sys_accept+0x17f
sys_accept() at netbsd:sys_accept+0x48
syscall() at netbsd:syscall+0x112
--- syscall (number 30) ---
netbsd:syscall+0x112:
ds          b700
es          0
fs          87c0
gs          3940
rdi         ffff8e88bd7a64e0
rsi         0
rbp         ffff9b005540edc0
rbx         ffff8e88bd7a64e0
rdx         ffff8e88bd6abc00
rcx         7
rax         ffff8e88bcc0b700
r8          0
r9          0
r10         d103a0
r11         246
r12         ffff9b005540ee68
r13         ffff8e88bd7a6980
r14         ffff8e88bd975728
r15         ffff8e88bcc0b700
rip         ffffffff80e83c7e    solocked2+0x3
cs          0
rflags      10202
rsp         ffff9b005540ed98
ss          10
netbsd:solocked2+0x3:   movq    0(%rsi),%rdi
db{1}> 

(gdb) list *(solocked2+0x3)
0xffffffff80e842ce is in solocked2 (/home/riastradh/netbsd/current/src/sys/kern/uipc_socket2.c:1504).
1499            /*
1500             * Used only for diagnostic assertions, so so_lock should be
1501             * stable at this point, hence on need for atomic_load_*.
1502             */
1503            lock = so1->so_lock;
1504            if (lock != so2->so_lock)
1505                    return false;
1506            return mutex_owned(lock);
1507    }
1508    
(gdb) disas solocked2
Dump of assembler code for function solocked2:
   0xffffffff80e842cb <+0>:     mov    (%rdi),%rax
   0xffffffff80e842ce <+3>:     mov    (%rsi),%rdi
   0xffffffff80e842d1 <+6>:     cmp    %rax,%rdi
   0xffffffff80e842d4 <+9>:     je     0xffffffff80e842d9 <solocked2+14>
   0xffffffff80e842d6 <+11>:    xor    %eax,%eax
   0xffffffff80e842d8 <+13>:    retq   
   0xffffffff80e842d9 <+14>:    push   %rbp
   0xffffffff80e842da <+15>:    mov    %rsp,%rbp
   0xffffffff80e842dd <+18>:    callq  0xffffffff80de89d6 <mutex_owned>
   0xffffffff80e842e2 <+23>:    test   %eax,%eax
   0xffffffff80e842e4 <+25>:    setne  %al
   0xffffffff80e842e7 <+28>:    pop    %rbp
   0xffffffff80e842e8 <+29>:    retq   

>How-To-Repeat:
ATF_TC(sockaddr_un_local_connwait);
ATF_TC_HEAD(sockaddr_un_local_connwait, tc)
{

	atf_tc_set_md_var(tc, "descr", "Check that LOCAL_CONNWAIT works");
}

ATF_TC_BODY(sockaddr_un_local_connwait, tc)
{
	/* too annoying to fit this into the test(...) framework above */
	struct sockaddr_un sun = {.sun_family = AF_UNIX, .sun_path = "sock"};
	int listener, conn, acc;
	const int one = 1;
	struct pollfd pfd;
	int nfd, error;
	socklen_t errorlen = sizeof(error);

	RL(listener = socket(PF_LOCAL, SOCK_STREAM|SOCK_NONBLOCK, 0));
	RL(bind(listener, (const struct sockaddr *)&sun, sizeof(sun)));
	RL(listen(listener, 5));

	/* accept would block */
	ATF_REQUIRE_ERRNO(EAGAIN, accept(listener, NULL, NULL) == -1);

	/* success without accept if we don't set LOCAL_CONNWAIT */
	RL(conn = socket(PF_LOCAL, SOCK_STREAM|SOCK_NONBLOCK, 0));
	RL(connect(conn, (const struct sockaddr *)&sun, sizeof(sun)));
	RL(close(conn));

	/* accept should happen now; chuck the client */
	RL(acc = accept(listener, NULL, NULL));
	RL(close(acc));

	/* fail EINPROGRESS without accept if we do set LOCAL_CONNWAIT */
	RL(conn = socket(PF_LOCAL, SOCK_STREAM|SOCK_NONBLOCK, 0));
	RL(setsockopt(conn, SOL_LOCAL, LOCAL_CONNWAIT, &one, sizeof(one)));
	ATF_REQUIRE_ERRNO(EINPROGRESS,
	    connect(conn, (const struct sockaddr *)&sun, sizeof(sun)) == -1);
	ATF_REQUIRE_ERRNO(EALREADY,
	    connect(conn, (const struct sockaddr *)&sun, sizeof(sun)) == -1);

	/* accept should happen now and connect should finish */
	RL(acc = accept(listener, NULL, NULL));
	pfd = (struct pollfd){.fd = conn, .events = POLLOUT};
	RL(nfd = poll(&pfd, 1, 0));
	ATF_REQUIRE_MSG(pfd.revents & POLLOUT, "revents=0x%x", pfd.revents);
	RL(getsockopt(conn, SOL_SOCKET, SO_ERROR, &error, &errorlen));
	ATF_REQUIRE_MSG(errorlen == sizeof(error), "errorlen=%d", errorlen);
	ATF_REQUIRE_MSG(error == 0, "error=%d", error);

	RL(close(acc));
	RL(close(conn));
	RL(close(listener));
}

>Fix:
Yes, please!

>Release-Note:

>Audit-Trail:
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59220 CVS commit: src/tests/net/net
Date: Thu, 27 Mar 2025 10:57:10 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Thu Mar 27 10:57:10 UTC 2025

 Modified Files:
 	src/tests/net/net: t_unix.c

 Log Message:
 t_unix: Sort includes.

 No functional change intended.

 Preparation for:

 PR kern/59220: accept(2): null pointer deref


 To generate a diff of this commit:
 cvs rdiff -u -r1.25 -r1.26 src/tests/net/net/t_unix.c

 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/59220 CVS commit: src/tests/net/net
Date: Thu, 27 Mar 2025 10:57:30 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Thu Mar 27 10:57:30 UTC 2025

 Modified Files:
 	src/tests/net/net: t_unix.c

 Log Message:
 t_unix: Make existing tests more reliable by exiting in child.

 Returning into atf in the child is not helpful.

 Preparation for adding a test for:

 PR kern/59220: accept(2): null pointer deref


 To generate a diff of this commit:
 cvs rdiff -u -r1.26 -r1.27 src/tests/net/net/t_unix.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: Thu, 27 Mar 2025 11:03:08 +0000
State-Changed-Why:
fixed in HEAD, needs pullup-9 and pullup-10


From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59220 CVS commit: src/sys/kern
Date: Thu, 27 Mar 2025 11:00:50 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Thu Mar 27 11:00:50 UTC 2025

 Modified Files:
 	src/sys/kern: uipc_usrreq.c

 Log Message:
 unp_accept: Fix assertion in SS_ISCONNECTING case.

 This null test matches the assertion inside soisconnected.

 In this context, so2 must also be locked, and we no longer assert so
 directly here -- but the assertions never worked as far as I can tell
 since they were added in 2008, and soisconnected already asserts
 solocked(so2), so this is fine.

 (Not doing this via xfail test because it crashes the whole test bed
 when the assertion fires, but the test that tripped this assertion is
 coming shortly.)

 PR kern/59220: accept(2): null pointer deref


 To generate a diff of this commit:
 cvs rdiff -u -r1.207 -r1.208 src/sys/kern/uipc_usrreq.c

 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/59220 CVS commit: src/tests/net/net
Date: Thu, 27 Mar 2025 11:01:08 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Thu Mar 27 11:01:07 UTC 2025

 Modified Files:
 	src/tests/net/net: t_unix.c

 Log Message:
 t_unix: Test LOCAL_CONNWAIT.

 PR kern/59220: accept(2): null pointer deref


 To generate a diff of this commit:
 cvs rdiff -u -r1.27 -r1.28 src/tests/net/net/t_unix.c

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

>Unformatted:

Home	PR Database Search