[PATCH] Throw an error on bad statchecks

[Barret Rhoden]

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+006896...@syzkaller.appspotmail.com
Signed-off-by: Barret Rhoden <br...@cs.berkeley.edu>
---
kern/include/ns.h | 2 +-
kern/src/ns/convM2D.c | 31 ++++++++++++++++++-------------
kern/src/ns/sysfile.c | 9 ++-------
3 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/kern/include/ns.h b/kern/include/ns.h
index 43091273e22f..f861d46638db 100644
--- a/kern/include/ns.h
+++ b/kern/include/ns.h
@@ -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);
diff --git a/kern/src/ns/convM2D.c b/kern/src/ns/convM2D.c
index bbc027c33600..0024aab19d84 100644
--- a/kern/src/ns/convM2D.c
+++ b/kern/src/ns/convM2D.c
@@ -39,45 +39,50 @@
#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[] = "";
diff --git a/kern/src/ns/sysfile.c b/kern/src/ns/sysfile.c
index a0d03d0e9156..86c4fa984112 100644
--- a/kern/src/ns/sysfile.c
+++ b/kern/src/ns/sysfile.c
@@ -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);
--
2.18.0