kern/51356: Repeating ifconfig <ip>/ifconfig <ip> delete under network load causes a panic

[ozaki-r%netbsd.org@localhost]

>Number:         51356
>Category:       kern
>Synopsis:       Repeating ifconfig <ip>/ifconfig <ip> delete under network load causes a panic
>Confidential:   no
>Severity:       critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jul 25 04:55:00 +0000 2016
>Originator:     Ryota Ozaki
>Release:        -current, netbsd-7 (and maybe netbsd-6)
>Organization:
>Environment:
NetBSD kvm 7.99.34 NetBSD 7.99.34 (KVM) #215: Mon Jul 25 12:42:10 JST 2016  ozaki-r@rangeley:(hidden) amd64
NetBSD netbsd7 7.0.0_PATCH NetBSD 7.0.0_PATCH (GENERIC.201512132100Z) amd64
>Description:
Adding and deleting an IP address is executed without softnet_lock, but that's unsafe
and causes a panic on rtcache or rtentry manipulations. We have to hold softnet_lock
during such operations.
>How-To-Repeat:
Setup a machine with IP forwarding enabled:

  sysctl -w net.inet.ip.forwarding=1
  ifconfig wm0 10.0.1.1/24
  ifconfig wm1 10.0.2.1/24

Sending massive UDP packets over the machine and run the following script on the machine:

  while true; do ifconfig wm0 10.0.1.1/24 >/dev/null 2>&1; ifconfig wm0 10.0.1.1/24 delete >/dev/null 2>&1; done &
  while true; do ifconfig wm1 10.0.2.1/24 >/dev/null 2>&1; ifconfig wm1 10.0.2.1/24 delete >/dev/null 2>&1; done &

And wait several minutes.
>Fix:
Hold softnet_lock in in_control (and in6_control). We also need to tweak callout_halt for DAD.

diff --git a/sys/netinet/if_arp.c b/sys/netinet/if_arp.c
index 3ae4e4d..e347315 100644
--- a/sys/netinet/if_arp.c
+++ b/sys/netinet/if_arp.c
@@ -1502,7 +1502,7 @@ static void
 arp_dad_stoptimer(struct dadq *dp)
 {
 
-	callout_halt(&dp->dad_timer_ch, NULL);
+	callout_halt(&dp->dad_timer_ch, softnet_lock);
 }
 
 static void
diff --git a/sys/netinet/in.c b/sys/netinet/in.c
index 7619d6b..90354f8 100644
--- a/sys/netinet/in.c
+++ b/sys/netinet/in.c
@@ -360,8 +360,8 @@ in_len2mask(struct in_addr *mask, u_int len)
  * Ifp is 0 if not an interface-specific ioctl.
  */
 /* ARGSUSED */
-int
-in_control(struct socket *so, u_long cmd, void *data, struct ifnet *ifp)
+static int
+in_control0(struct socket *so, u_long cmd, void *data, struct ifnet *ifp)
 {
 	struct ifreq *ifr = (struct ifreq *)data;
 	struct in_ifaddr *ia = NULL;
@@ -667,6 +667,18 @@ in_control(struct socket *so, u_long cmd, void *data, struct ifnet *ifp)
 	return error;
 }
 
+int
+in_control(struct socket *so, u_long cmd, void *data, struct ifnet *ifp)
+{
+	int error;
+
+	mutex_enter(softnet_lock);
+	error = in_control0(so, cmd, data, ifp);
+	mutex_exit(softnet_lock);
+
+	return error;
+}
+
 /* Add ownaddr as loopback rtentry. */
 static void
 in_ifaddlocal(struct ifaddr *ifa)
diff --git a/sys/netinet6/in6.c b/sys/netinet6/in6.c
index ead9154..54e5823 100644
--- a/sys/netinet6/in6.c
+++ b/sys/netinet6/in6.c
@@ -751,7 +751,9 @@ in6_control(struct socket *so, u_long cmd, void *data, struct ifnet *ifp)
 	}
 
 	s = splnet();
+	mutex_enter(softnet_lock);
 	error = in6_control1(so , cmd, data, ifp);
+	mutex_exit(softnet_lock);
 	splx(s);
 	return error;
 }
diff --git a/sys/netinet6/nd6_nbr.c b/sys/netinet6/nd6_nbr.c
index a3ee9b9..6066460 100644
--- a/sys/netinet6/nd6_nbr.c
+++ b/sys/netinet6/nd6_nbr.c
@@ -1074,7 +1074,7 @@ static void
 nd6_dad_stoptimer(struct dadq *dp)
 {
 
-	callout_halt(&dp->dad_timer_ch, NULL);
+	callout_halt(&dp->dad_timer_ch, softnet_lock);
 }
 
 /*