NetBSD Problem Report #53075

From martin@duskware.de  Tue Mar  6 19:23:19 2018
Return-Path: <martin@duskware.de>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id A975D7A187
	for <gnats-bugs@gnats.NetBSD.org>; Tue,  6 Mar 2018 19:23:19 +0000 (UTC)
From: martin@NetBSD.org
Reply-To: martin@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: nd6_dad_duplicated gets called with NULL argument
X-Send-Pr-Version: 3.95

>Number:         53075
>Category:       kern
>Synopsis:       nd6_dad_duplicated gets called with NULL argument
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    ozaki-r
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Mar 06 19:25:00 +0000 2018
>Closed-Date:    Wed Mar 07 17:33:06 +0000 2018
>Last-Modified:  Mon Apr 02 08:55:00 +0000 2018
>Originator:     Martin Husemann
>Release:        NetBSD 8.99.12
>Organization:
The NetBSD Foundation, Inc.
>Environment:
System: NetBSD night-owl.duskware.de 8.99.12 NetBSD 8.99.12 (NIGHT-OWL) #586: Tue Mar 6 20:09:36 CET 2018 martin@night-owl.duskware.de:/usr/src/sys/arch/amd64/compile/NIGHT-OWL amd64
Architecture: x86_64
Machine: amd64
>Description:

If I enable DAD (which is the default), I get an ~instant crash as soon
as my machine connects to a certain wlan.

I used to have DAD globally disabled for PR 48450 and only just enabled
it again to test Roy's DAD changes. This issue is older though.

>How-To-Repeat:
see above

>Fix:
This patch avoids the crash, but I haven't checked if this is a legitimate
call or caused by some other bug. And I don't know if just ignoring the
call is the right thing to do.


Index: nd6_nbr.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/nd6_nbr.c,v
retrieving revision 1.150
diff -u -p -r1.150 nd6_nbr.c
--- nd6_nbr.c	6 Mar 2018 11:21:31 -0000	1.150
+++ nd6_nbr.c	6 Mar 2018 19:14:03 -0000
@@ -1385,11 +1385,15 @@ done:
 static void
 nd6_dad_duplicated(struct dadq *dp)
 {
-	struct ifaddr *ifa = dp->dad_ifa;
+	struct ifaddr *ifa;
 	struct in6_ifaddr *ia;
 	struct ifnet *ifp;
 	char ip6buf[INET6_ADDRSTRLEN];

+	if (dp == NULL)
+		return;
+
+	ifa = dp->dad_ifa;
 	KASSERT(mutex_owned(&nd6_dad_lock));
 	KASSERT(ifa != NULL);


>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: kern-bug-people->ozaki-r
Responsible-Changed-By: ozaki-r@NetBSD.org
Responsible-Changed-When: Wed, 07 Mar 2018 01:39:27 +0000
Responsible-Changed-Why:
mine


State-Changed-From-To: open->feedback
State-Changed-By: ozaki-r@NetBSD.org
State-Changed-When: Wed, 07 Mar 2018 01:39:27 +0000
State-Changed-Why:
Fixed in -current. Could you test?


From: "Ryota Ozaki" <ozaki-r@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/53075 CVS commit: src/sys/netinet6
Date: Wed, 7 Mar 2018 01:37:24 +0000

 Module Name:	src
 Committed By:	ozaki-r
 Date:		Wed Mar  7 01:37:24 UTC 2018

 Modified Files:
 	src/sys/netinet6: nd6_nbr.c

 Log Message:
 Avoid passing NULL to nd6_dad_duplicated

 Fix PR kern/53075


 To generate a diff of this commit:
 cvs rdiff -u -r1.150 -r1.151 src/sys/netinet6/nd6_nbr.c

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

State-Changed-From-To: feedback->closed
State-Changed-By: martin@NetBSD.org
State-Changed-When: Wed, 07 Mar 2018 17:33:06 +0000
State-Changed-Why:
Works fine, thanks for the quick fix!


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/53075 CVS commit: [netbsd-8] src/sys
Date: Mon, 2 Apr 2018 08:54:35 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Mon Apr  2 08:54:35 UTC 2018

 Modified Files:
 	src/sys/netinet [netbsd-8]: if_arp.c
 	src/sys/netinet6 [netbsd-8]: nd6_nbr.c

 Log Message:
 Pull up following revision(s) (requested by ozaki-r in ticket #686):

 	sys/netinet/if_arp.c: revision 1.271
 	sys/netinet6/nd6_nbr.c: revision 1.151,1.152

 Avoid passing NULL to nd6_dad_duplicated
 Fix PR kern/53075

 Fix a race condition on DAD destructions (again)

 The previous fix to DAD timers was wrong; it avoided a use-after-free but
 instead introduced a memory leak.  The destruction method had delegated
 a destruction of a DAD timer to the timer itself and told that by setting NULL
 to dp->dad_ifa.  However, the previous fix made DAD timers do nothing on
 the sign.

 Fixing the issue with using callout_stop isn't easy.  One approach is to have
 a refcount on dp but it introduces extra complexity that we want to avoid.
 The new fix falls back to using callout_halt, which was abandoned because of
 softnet_lock.  Fortunately now the network stack is protected by KERNEL_LOCK
 so we can remove softnet_lock from DAD timers (callout) and use callout_halt
 safely.


 To generate a diff of this commit:
 cvs rdiff -u -r1.250.2.7 -r1.250.2.8 src/sys/netinet/if_arp.c
 cvs rdiff -u -r1.138.6.5 -r1.138.6.6 src/sys/netinet6/nd6_nbr.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