Source-Changes-HG archive

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

[src/trunk]: src/sys/net Avoid if_dl and if_sadl to be NULL



details:   https://anonhg.NetBSD.org/src/rev/20088ac39d6f
branches:  trunk
changeset: 351473:20088ac39d6f
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Wed Feb 15 01:48:44 2017 +0000

description:
Avoid if_dl and if_sadl to be NULL

Calling if_deactivate_sadl and then if_sadl_setrefs exposes NULL-ed if_dl
and if_sadl to users for a moment. It's harmful because users expect that
they're always non-NULL. Fix it.

Note that a race condition still remains; if_dl and if_sald aren't updated
atomically so a user can see different data from if_dl and if_sadl.
Fortunately none uses both if_dl and if_sadl at the same time, so the race
condition doesn't hurt nobody for now. (In the first place exposing one
data with two ways is problematic?)

diffstat:

 sys/net/if.c |  27 ++++++++++++++++++++++-----
 1 files changed, 22 insertions(+), 5 deletions(-)

diffs (57 lines):

diff -r cab8366a7f81 -r 20088ac39d6f sys/net/if.c
--- a/sys/net/if.c      Tue Feb 14 18:38:07 2017 +0000
+++ b/sys/net/if.c      Wed Feb 15 01:48:44 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if.c,v 1.377 2017/02/10 20:56:21 christos Exp $        */
+/*     $NetBSD: if.c,v 1.378 2017/02/15 01:48:44 ozaki-r Exp $ */
 
 /*-
  * Copyright (c) 1999, 2000, 2001, 2008 The NetBSD Foundation, Inc.
@@ -90,7 +90,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.377 2017/02/10 20:56:21 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.378 2017/02/15 01:48:44 ozaki-r Exp $");
 
 #if defined(_KERNEL_OPT)
 #include "opt_inet.h"
@@ -518,6 +518,23 @@
        ifafree(ifa);
 }
 
+static void
+if_replace_sadl(struct ifnet *ifp, struct ifaddr *ifa)
+{
+       struct ifaddr *old;
+
+       KASSERT(ifp->if_dl != NULL);
+
+       old = ifp->if_dl;
+
+       ifaref(ifa);
+       /* XXX Update if_dl and if_sadl atomically */
+       ifp->if_dl = ifa;
+       ifp->if_sadl = satosdl(ifa->ifa_addr);
+
+       ifafree(old);
+}
+
 void
 if_activate_sadl(struct ifnet *ifp, struct ifaddr *ifa0,
     const struct sockaddr_dl *sdl)
@@ -526,11 +543,11 @@
        struct ifaddr *ifa;
        int bound = curlwp_bind();
 
+       KASSERT(ifa_held(ifa0));
+
        s = splsoftnet();
 
-       if_deactivate_sadl(ifp);
-
-       if_sadl_setrefs(ifp, ifa0);
+       if_replace_sadl(ifp, ifa0);
 
        ss = pserialize_read_enter();
        IFADDR_READER_FOREACH(ifa, ifp) {



Home | Main Index | Thread Index | Old Index