Thu May 25 02:43:43 2017 UTC ()

Fix that a fresh in_ifaddr is unexpectedly freed before activating it

An in_ifaddr object is initialized with refcnt=0 and the refcnt
is incremented when being enqueued to the lists. However before
enqueuing it, in_ifinit can hold and refelease a reference to
it, i.e., call ifaref and ifafree, resulting in that the object
is freed in ifafree because its refcnt is decremented to 0.

It can be reproduced by doing:
  ifconfig tun0 create
  ifconfig tun1 create
  ifconfig tun0 10.1 10.2
  ifconfig tun1 10.2 10.1
  ifconfig  # Cause a kernel panic (may depend on environmemts)

We need to initialize a created in_ifaddr object with refcnt=1
to make the object survive over in_ifinit.

The issue is found by ryo@


(ozaki-r)

diff -r1.201 -r1.202 src/sys/netinet/in.c

--- src/sys/netinet/in.c 2017/05/12 17:53:53	1.201
+++ src/sys/netinet/in.c 2017/05/25 02:43:43	1.202

 @@ -1,14 +1,14 @@
-/*	$NetBSD: in.c,v 1.201 2017/05/12 17:53:53 ryo Exp $	*/
+/*	$NetBSD: in.c,v 1.202 2017/05/25 02:43:43 ozaki-r Exp $	*/
 /*
  * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
  * All rights reserved.
+ *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
  *    documentation and/or other materials provided with the distribution.
 @@ -81,27 +81,27 @@
  * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
  * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
  * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
  * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
  * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
  * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
+ *
  *	@(#)in.c	8.4 (Berkeley) 1/9/95
  */
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: in.c,v 1.201 2017/05/12 17:53:53 ryo Exp $");
+__KERNEL_RCSID(0, "$NetBSD: in.c,v 1.202 2017/05/25 02:43:43 ozaki-r Exp $");
 #include "arp.h"
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
 #include "opt_inet_conf.h"
 #include "opt_mrouting.h"
 #include "opt_net_mpsafe.h"
 #endif
 #include <sys/param.h>
 #include <sys/ioctl.h>
 #include <sys/errno.h>
 @@ -485,26 +485,31 @@ in_control0(struct socket *so, u_long cm
 #endif /* IPSELSRC */
 			ia->ia_sockmask.sin_len = 8;
 			ia->ia_sockmask.sin_family = AF_INET;
 			if (ifp->if_flags & IFF_BROADCAST) {
 				ia->ia_broadaddr.sin_len = sizeof(ia->ia_addr);
 				ia->ia_broadaddr.sin_family = AF_INET;
+			}
 			ia->ia_ifp = ifp;
 			ia->ia_idsalt = cprng_fast32() % 65535;
 			LIST_INIT(&ia->ia_multiaddrs);
 			IN_ADDRHASH_ENTRY_INIT(ia);
 			IN_ADDRLIST_ENTRY_INIT(ia);
 			ifa_psref_init(&ia->ia_ifa);
 			/*
 			 * We need a reference to make ia survive over in_ifinit
 			 * that does ifaref and ifafree.
 			 */
 			ifaref(&ia->ia_ifa);
 			newifaddr = 1;
+		}
 		break;
 	case SIOCSIFBRDADDR:
 		if (kauth_authorize_network(curlwp->l_cred, KAUTH_NETWORK_INTERFACE,
 		    KAUTH_REQ_NETWORK_INTERFACE_SETPRIV, ifp, (void *)cmd,
 		    NULL) != 0) {
 			error = EPERM;
 			goto out;
+		}
 		/* FALLTHROUGH */
 @@ -671,26 +676,28 @@ in_control0(struct socket *so, u_long cm
 	/*
 	 * XXX insert regardless of error to make in_purgeaddr below work.
 	 * Need to improve.
 	 */
 	if (newifaddr) {
 		ifaref(&ia->ia_ifa);
 		ifa_insert(ifp, &ia->ia_ifa);
 		mutex_enter(&in_ifaddr_lock);
 		TAILQ_INSERT_TAIL(&in_ifaddrhead, ia, ia_list);
 		IN_ADDRLIST_WRITER_INSERT_TAIL(ia);
 		in_addrhash_insert_locked(ia);
 		/* Release a reference that is held just after creation. */
 		ifafree(&ia->ia_ifa);
 		mutex_exit(&in_ifaddr_lock);
 	} else if (need_reinsert) {
 		in_addrhash_insert(ia);
+	}
 	if (error == 0) {
 		if (run_hook)
 			pfil_run_addrhooks(if_pfil, cmd, &ia->ia_ifa);
 	} else if (newifaddr) {
 		KASSERT(ia != NULL);
 		in_purgeaddr(&ia->ia_ifa);
 		ia = NULL;
+	}