net: Fix double-free snoop bug
[akaros.git] / kern / src / net / ipifc.c
index df85c0c..2ab62be 100644 (file)
@@ -132,26 +132,26 @@ struct medium *ipfindmedium(char *name)
  *  attach a device (or pkt driver) to the interface.
  *  called with c locked
  */
-static char *ipifcbind(struct conv *c, char **argv, int argc)
+static void ipifcbind(struct conv *c, char **argv, int argc)
 {
        ERRSTACK(1);
        struct Ipifc *ifc;
        struct medium *m;
 
        if (argc < 2)
-               return errno_to_string(EINVAL);
+               error(EINVAL, "Too few args (%d) to %s", argc, __func__);
 
        ifc = (struct Ipifc *)c->ptcl;
 
        /* bind the device to the interface */
        m = ipfindmedium(argv[1]);
        if (m == NULL)
-               return "unknown interface type";
+               error(EFAIL, "unknown interface type");
 
        wlock(&ifc->rwlock);
        if (ifc->m != NULL) {
                wunlock(&ifc->rwlock);
-               return "interface already bound";
+               error(EFAIL, "interfacr already bound");
        }
        if (waserror()) {
                wunlock(&ifc->rwlock);
@@ -193,8 +193,6 @@ static char *ipifcbind(struct conv *c, char **argv, int argc)
 
        wunlock(&ifc->rwlock);
        poperror();
-
-       return NULL;
 }
 
 /*
@@ -239,7 +237,7 @@ static void ipifcunbind(struct Ipifc *ifc)
 }
 
 char sfixedformat[] =
-       "device %s maxtu %d sendra %d recvra %d mflag %d oflag %d maxraint %d minraint %d linkmtu %d reachtime %d rxmitra %d ttl %d routerlt %d pktin %lu pktout %lu errin %lu errout %lu\n";
+       "device %s maxtu %d sendra %d recvra %d mflag %d oflag %d maxraint %d minraint %d linkmtu %d reachtime %d rxmitra %d ttl %d routerlt %d pktin %lu pktout %lu errin %lu errout %lu tracedrop %lu\n";
 
 char slineformat[] = " %-40I %-10M %-40I %-12lu %-12lu\n";
 
@@ -256,7 +254,7 @@ static int ipifcstate(struct conv *c, char *state, int n)
                                 ifc->rp.mflag, ifc->rp.oflag, ifc->rp.maxraint,
                                 ifc->rp.minraint, ifc->rp.linkmtu, ifc->rp.reachtime,
                                 ifc->rp.rxmitra, ifc->rp.ttl, ifc->rp.routerlt,
-                                ifc->in, ifc->out, ifc->inerr, ifc->outerr);
+                                ifc->in, ifc->out, ifc->inerr, ifc->outerr, ifc->tracedrop);
 
        rlock(&ifc->rwlock);
        for (lifc = ifc->lifc; lifc && n > m; lifc = lifc->next)
@@ -338,7 +336,7 @@ static void ipifccreate(struct conv *c)
        struct Ipifc *ifc;
 
        c->rq = qopen(QMAX, 0, 0, 0);
-       c->sq = qopen(2 * QMAX, 0, 0, 0);
+       c->sq = qopen(2 * QMAX, Qmsg | Qcoalesce, 0, 0);
        c->wq = qopen(QMAX, Qkick, ipifckick, c);
        ifc = (struct Ipifc *)c->ptcl;
        ifc->conv = c;
@@ -866,6 +864,17 @@ void ipifcinit(struct Fs *f)
 }
 
 /*
+ * TODO: Change this to a proper release.
+ * At the moment, this is difficult since link removal
+ * requires access to more than just the kref/struct Iplink.
+ * E.g., the self and interface pointers.
+ */
+static void link_release(struct kref *kref)
+{
+       (void)kref;
+}
+
+/*
  *  add to self routing cache
  *     called with c locked
  */
@@ -906,7 +915,7 @@ addselfcache(struct Fs *f, struct Ipifc *ifc,
        /* allocate a lifc-to-local link and link to both */
        if (lp == NULL) {
                lp = kzmalloc(sizeof(*lp), 0);
-               kref_init(&lp->ref, fake_release, 1);
+               kref_init(&lp->ref, link_release, 1);
                lp->lifc = lifc;
                lp->self = p;
                lp->selflink = p->link;
@@ -983,7 +992,7 @@ static void ipselffree(struct Ipself *p)
  *     called with c locked
  */
 static void
-remselfcache(struct Fs *f, struct Ipifc *ifc, struct Iplifc *lifc, uint8_t * a)
+remselfcache(struct Fs *f, struct Ipifc *ifc, struct Iplifc *lifc, uint8_t *a)
 {
        struct Ipself *p, **l;
        struct Iplink *link, **l_self, **l_lifc;
@@ -1029,7 +1038,25 @@ remselfcache(struct Fs *f, struct Ipifc *ifc, struct Iplifc *lifc, uint8_t * a)
        if (link == NULL)
                panic("remselfcache");
 
-       if (kref_refcnt(&link->ref) > 1)
+       /*
+        * TODO: The check for 'refcnt > 0' here is awkward.  It
+        * exists so that 'remselfcache' can be called concurrently.
+        * In the original plan 9 code, the 'goto out' branch was
+        * taken if the decremented reference count was exactly zero.
+        * In other threads this could become -1, which plan 9 didn't
+        * care about since the logic would be skipped over, and since
+        * 'iplinkfree' won't _actually_ free the link for five seconds
+        * (see comments in that function), and since all of the actual
+        * link manipulation happens in the thread where the reference
+        * is exactly equal to zero.  But on Akaros, a negative kref
+        * will panic; hence checking for a positive ref count before
+        * decrementing.  This entire mechanism is dubious.  But Since
+        * this function is protected by a lock this is probably OK for
+        * the time being.
+        *
+        * However, it is a terrible design and we should fix it.
+        */
+       if (kref_refcnt(&link->ref) > 0 && kref_put(&link->ref) != 0)
                goto out;
 
        if ((p->type & Rmulti) && ifc->m->remmulti != NULL)
@@ -1485,6 +1512,22 @@ void ipifcaddmulti(struct conv *c, uint8_t * ma, uint8_t * ia)
        }
 }
 
+/* Trace a block @b that crosses the interface @ifc. */
+void ipifc_trace_block(struct Ipifc *ifc, struct block *bp)
+{
+       struct block *newb;
+
+       if (!atomic_read(&ifc->conv->snoopers))
+               return;
+       newb = copyblock(bp, MEM_ATOMIC);
+       if (!newb) {
+               ifc->tracedrop++;
+               return;
+       }
+       if (qpass(ifc->conv->sq, newb) < 0)
+               ifc->tracedrop++;
+}
+
 /*
  *  remove a multicast address from an interface, called with c locked
  */