Throw an error on bad statchecks
authorBarret Rhoden <brho@cs.berkeley.edu>
Tue, 25 Sep 2018 03:38:53 +0000 (23:38 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Tue, 25 Sep 2018 03:55:17 +0000 (23:55 -0400)
statcheck() was returning -1, and both of its callers would throw an
error.  We might as well report the specific error and squelch the
warning.

Reported-by: syzbot+0068960e94fbc67ffab4@syzkaller.appspotmail.com
Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/include/ns.h
kern/src/ns/convM2D.c
kern/src/ns/sysfile.c

index 4309127..f861d46 100644 (file)
@@ -337,7 +337,7 @@ unsigned int convM2kdirent(uint8_t * buf, unsigned int nbuf, struct kdirent *kd,
                                                   char *strs);
 unsigned int convM2kstat(uint8_t * buf, unsigned int nbuf, struct kstat *ks);
 
-int statcheck(uint8_t * abuf, unsigned int nbuf);
+void statcheck(uint8_t *buf, size_t nbuf);
 unsigned int convM2D(uint8_t * unused_uint8_p_t, unsigned int unused_int,
                                         struct dir *, char *unused_char_p_t);
 unsigned int convD2M(struct dir *, uint8_t * unused_uint8_p_t, unsigned int);
index bbc027c..0024aab 100644 (file)
 #include <net/ip.h>
 
 /* It looks like the intent of this code is to check any stat that we, the
- * kernel or our userspace, send out. */
-int statcheck(uint8_t * buf, unsigned int nbuf)
+ * kernel or our userspace, send out.
+ *
+ * Throws on error. */
+void statcheck(uint8_t *buf, size_t nbuf)
 {
        uint8_t *ebuf;
        int i;
 
        ebuf = buf + nbuf;
 
-       if (nbuf < STAT_FIX_LEN_9P || nbuf != BIT16SZ + GBIT16(buf)) {
-               warn("nbuf %d, STAT_FIX_LEN_9P %d BIT16SZ %d, GBIT16(buf) %d ",
-                    nbuf, STAT_FIX_LEN_9P, BIT16SZ, GBIT16(buf));
-               return -1;
-       }
+       if (nbuf < STAT_FIX_LEN_9P)
+               error(EINVAL, "%s: legacy chunk too short (%lu < %u)", __func__,
+                     nbuf, STAT_FIX_LEN_9P);
+       if (nbuf != BIT16SZ + GBIT16(buf))
+               error(EINVAL, "%s: size mismatch (%lu != %u)", __func__, nbuf,
+                     BIT16SZ + GBIT16(buf));
 
        buf += STAT_FIX_LEN_9P - STAT_NR_STRINGS_9P * BIT16SZ;
 
        /* Check the legacy strings that all stats have. */
        for (i = 0; i < STAT_NR_STRINGS_9P; i++) {
                if (buf + BIT16SZ > ebuf)
-                       return -1;
+                       error(EINVAL, "%s: string %d (legacy) out of range",
+                             __func__, i);
                buf += BIT16SZ + GBIT16(buf);
        }
        /* Legacy 9p stats are OK
         * TODO: consider removing this.  We get them from userspace, e.g. mkdir. */
        if (buf == ebuf)
-               return 0;
+               return;
 
        for (i = STAT_NR_STRINGS_9P; i < STAT_NR_STRINGS_AK; i++) {
                if (buf + BIT16SZ > ebuf)
-                       return -1;
+                       error(EINVAL, "%s: string %d (akaros) out of range",
+                             __func__, i);
                buf += BIT16SZ + GBIT16(buf);
        }
 
        if (buf + __STAT_FIX_LEN_AK_NONSTRING > ebuf)
-               return -1;
+               error(EINVAL, "%s: akaros chunk too short (%ld < %u)", __func__,
+                     ebuf - buf, __STAT_FIX_LEN_AK_NONSTRING);
        buf += __STAT_FIX_LEN_AK_NONSTRING;
        if (buf != ebuf)
-               return -1;
-       return 0;
+               error(EINVAL, "%s: %lu extra bytes", __func__, ebuf - buf);
 }
 
 static char nullstring[] = "";
index a0d03d0..86c4fa9 100644 (file)
@@ -958,8 +958,7 @@ void validstat(uint8_t * s, int n, int slashok)
        int m;
        char buf[64];
 
-       if (statcheck(s, n) < 0)
-               error(EINVAL, ERROR_FIXME);
+       statcheck(s, n);
        /* verify that name entry is acceptable */
        s += STAT_FIX_LEN_9P - STAT_NR_STRINGS_9P * BIT16SZ;
        /*
@@ -1308,15 +1307,11 @@ static long dirpackage(uint8_t * buf, long ts, struct kdirent **d)
        n = 0;
        for (i = 0; i < ts; i += m) {
                m = BIT16SZ + GBIT16(&buf[i]);
-               if (statcheck(&buf[i], m) < 0)
-                       break;
+               statcheck(&buf[i], m);
                ss += m;
                n++;
        }
 
-       if (i != ts)
-               error(EFAIL, "bad directory format");
-
        *d = kzmalloc(n * sizeof(**d) + ss, 0);
        if (*d == NULL)
                error(ENOMEM, ERROR_FIXME);