Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/netinet Fix that a fresh in_ifaddr is unexpectedly freed...



details:   https://anonhg.NetBSD.org/src/rev/0d6cf176516e
branches:  trunk
changeset: 824128:0d6cf176516e
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Thu May 25 02:43:43 2017 +0000

description:
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@

diffstat:

 sys/netinet/in.c |  11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diffs (39 lines):

diff -r dcff2bf8be90 -r 0d6cf176516e sys/netinet/in.c
--- a/sys/netinet/in.c  Thu May 25 02:28:07 2017 +0000
+++ b/sys/netinet/in.c  Thu May 25 02:43:43 2017 +0000
@@ -1,4 +1,4 @@
-/*     $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.
@@ -91,7 +91,7 @@
  */
 
 #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"
 
@@ -495,6 +495,11 @@
                        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;
                }
@@ -681,6 +686,8 @@
                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);



Home | Main Index | Thread Index | Old Index