iplib: Fix thread-unsafeness in myipaddr
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 22 Dec 2016 15:53:04 +0000 (10:53 -0500)
committerBarret Rhoden <brho@cs.berkeley.edu>
Tue, 10 Jan 2017 00:01:40 +0000 (19:01 -0500)
readipifc() has this nasty style of "pass in your old one and we'll free
it", presumably so the caller doesn't need to just free whatever they got.
Of course, this style leads to thread-unsafe practices.  You gotta store
that pointer somewhere!

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
user/iplib/include/iplib/iplib.h
user/iplib/myipaddr.c
user/iplib/readipifc.c

index 8c2a35f..157e639 100644 (file)
@@ -195,7 +195,8 @@ int myetheraddr(uint8_t*, char*);
 int    equivip4(uint8_t*, uint8_t*);
 int    equivip6(uint8_t*, uint8_t*);
 
-struct ipifc*  readipifc(char*, struct ipifc*, int);
+struct ipifc *readipifc(char *net, struct ipifc *to_free, int index);
+void free_ipifc(struct ipifc *ifc);
 
 void   hnputv(void*, uint64_t);
 void   hnputl(void*, unsigned int);
index 1e564b7..05873ab 100644 (file)
@@ -33,10 +33,10 @@ int myipaddr(uint8_t *ip, char *net)
 {
        struct ipifc *nifc;
        struct iplifc *lifc;
-       static struct ipifc *ifc;
+       struct ipifc *ifc;
        uint8_t mynet[IPaddrlen];
 
-       ifc = readipifc(net, ifc, -1);
+       ifc = readipifc(net, NULL, -1);
        for (nifc = ifc; nifc; nifc = nifc->next)
                for (lifc = nifc->lifc; lifc; lifc = lifc->next) {
                        maskip(lifc->ip, loopbackmask, mynet);
@@ -45,9 +45,11 @@ int myipaddr(uint8_t *ip, char *net)
                        }
                        if (ipcmp(lifc->ip, IPnoaddr) != 0) {
                                ipmove(ip, lifc->ip);
+                               free_ipifc(ifc);
                                return 0;
                        }
                }
        ipmove(ip, IPnoaddr);
+       free_ipifc(ifc);
        return -1;
 }
index 45b0a97..f25b438 100644 (file)
@@ -156,7 +156,7 @@ static struct ipifc **_readipifc(char *file, struct ipifc **l, int index)
        return l;
 }
 
-static void _freeifc(struct ipifc *ifc)
+void free_ipifc(struct ipifc *ifc)
 {
        struct ipifc *next;
        struct iplifc *lnext, *lifc;
@@ -173,6 +173,9 @@ static void _freeifc(struct ipifc *ifc)
        }
 }
 
+/* This will free @ifc when passed in.  Some old Plan 9 programs might rely on
+ * it still (like our ping, netstat, ipconfig).  It usually ends up be a
+ * thread-unsafe disaster. */
 struct ipifc *readipifc(char *net, struct ipifc *ifc, int index)
 {
        int fd, i, n;
@@ -181,7 +184,7 @@ struct ipifc *readipifc(char *net, struct ipifc *ifc, int index)
        char buf[128];
        struct ipifc **l;
 
-       _freeifc(ifc);
+       free_ipifc(ifc);
 
        l = &ifc;
        ifc = NULL;