shithub: riscv

Info  •  Files  •  Log  •  Branches

kernel: try to catch some (rare) mistakes

kstrcpy() did not null terminate for < 4 byte buffers. fixed,
but i dont think there is any case where this can happen in
practice.

always set malloctag in kstrdup(), cleanup.

always use ERRMAX bounded kstrcpy() to set up->errstr, q->err
and note[]->msg. paranoia.

instead of silently truncating interface name in netifinit(),
panic the kernel if interface name is too long as this case
is clearly a mistake.

panic kernel when filename is too long for addbootfile() in
devroot. this might happen if your kernel configuration is
messed up.

--- a/sys/src/9/port/chan.c

+++ b/sys/src/9/port/chan.c

@@ -114,7 +114,6 @@

 	unlock(r);

 	if(x < 0)

 		panic("decref pc=%#p", getcallerpc(&r));

-

 	return x;

 }

@@ -130,20 +129,19 @@

 	int nt;

 	nt = strlen(t);

-	if(nt+1 <= ns){

-		memmove(s, t, nt+1);

+	if(nt < ns){

+		memmove(s, t, nt);

+		s[nt] = '\0';

 		return;

 	}

-	/* too long */

-	if(ns < 4){

-		/* but very short! */

-		strncpy(s, t, ns);

-		return;

-	}

-	/* truncate with ... at character boundary (very rare case) */

-	memmove(s, t, ns-4);

+	/* too long, truncate */

+	nt = ns-1;

+	memmove(s, t, nt);

+	s[nt] = '\0';

+	/* append ... if there is space */

 	ns -= 4;

-	s[ns] = '\0';

+	if(ns < 0)

+		return;

 	/* look for first byte of UTF-8 sequence by skipping continuation bytes */

 	while(ns>0 && (s[--ns]&0xC0)==0x80)

 		;

@@ -169,17 +167,18 @@

 	int n;

 	char *t, *prev;

-	n = strlen(s)+1;

+	n = strlen(s);

 	/* if it's a user, we can wait for memory; if not, something's very wrong */

-	if(up){

-		t = smalloc(n);

-		setmalloctag(t, getcallerpc(&p));

-	}else{

-		t = malloc(n);

+	if(up != nil)

+		t = smalloc(n+1);

+	else{

+		t = malloc(n+1);

 		if(t == nil)

 			panic("kstrdup: no memory");

 	}

+	setmalloctag(t, getcallerpc(&p));

 	memmove(t, s, n);

+	t[n] = '\0';

 	prev = *p;

 	*p = t;

 	free(prev);

@@ -1002,7 +1001,7 @@

 				*nerror = nhave;

 			pathclose(path);

 			cclose(c);

-			strcpy(up->errstr, Enotdir);

+			kstrcpy(up->errstr, Enotdir, ERRMAX);

 			if(mh != nil)

 				putmhead(mh);

 			return -1;

@@ -1083,11 +1082,11 @@

 					if(wq->nqid==0 || (wq->qid[wq->nqid-1].type&QTDIR)){

 						if(nerror)

 							*nerror = nhave+wq->nqid+1;

-						strcpy(up->errstr, Edoesnotexist);

+						kstrcpy(up->errstr, Edoesnotexist, ERRMAX);

 					}else{

 						if(nerror)

 							*nerror = nhave+wq->nqid;

-						strcpy(up->errstr, Enotdir);

+						kstrcpy(up->errstr, Enotdir, ERRMAX);

 					}

 					free(wq);

 					if(mh != nil)

--- a/sys/src/9/port/devaoe.c

+++ b/sys/src/9/port/devaoe.c

@@ -2168,7 +2168,7 @@

 netrdaoeproc(void *v)

 {

 	int idx;

-	char name[Maxpath+1], *s;

+	char name[Maxpath], *s;

 	Aoehdr *h;

 	Block *b;

 	Netlink *nl;

--- a/sys/src/9/port/devroot.c

+++ b/sys/src/9/port/devroot.c

@@ -63,6 +63,8 @@

 		panic("too many root files");

 	l->data[l->ndir] = contents;

 	d = &l->dir[l->ndir];

+	if(strlen(name) >= sizeof d->name)

+		panic("root file name too long: %s", name);

 	strcpy(d->name, name);

 	d->length = len;

 	d->perm = perm;

--- a/sys/src/9/port/netif.c

+++ b/sys/src/9/port/netif.c

@@ -18,8 +18,9 @@

 void

 netifinit(Netif *nif, char *name, int nfile, ulong limit)

 {

-	strncpy(nif->name, name, KNAMELEN-1);

-	nif->name[KNAMELEN-1] = 0;

+	if(strlen(name) >= sizeof nif->name)

+		panic("netifinit: name too long: %s", name);

+	strcpy(nif->name, name);

 	nif->nfile = nfile;

 	nif->f = xalloc(nfile*sizeof(Netfile*));

 	if (nif->f == nil)

--- a/sys/src/9/port/proc.c

+++ b/sys/src/9/port/proc.c

@@ -913,7 +913,7 @@

 	ret = 0;

 	if(p->nnote < NNOTE && n != nil) {

-		strcpy(p->note[p->nnote].msg, n);

+		kstrcpy(p->note[p->nnote].msg, n, ERRMAX);

 		p->note[p->nnote++].flag = flag;

 		ret = 1;

 	}

--- a/sys/src/9/port/qio.c

+++ b/sys/src/9/port/qio.c

@@ -1391,7 +1391,7 @@

 	ilock(q);

 	q->state |= Qclosed;

 	q->state &= ~(Qflow|Qstarve);

-	strcpy(q->err, Ehungup);

+	kstrcpy(q->err, Ehungup, ERRMAX);

 	bfirst = q->bfirst;

 	q->bfirst = 0;

 	q->len = 0;

@@ -1417,12 +1417,9 @@

 	/* mark it */

 	ilock(q);

 	q->state |= Qclosed;

-	if(msg == 0 || *msg == 0)

-		strcpy(q->err, Ehungup);

-	else {

-		strncpy(q->err, msg, ERRMAX-1);

-		q->err[ERRMAX-1] = 0;

-	}

+	if(msg == 0 || *msg == '\0')

+		msg = Ehungup;

+	kstrcpy(q->err, msg, ERRMAX);

 	iunlock(q);

 	/* wake up readers/writers */