shithub: riscv

Info  •  Files  •  Log  •  Branches

lib9p: reject reads on closed fids and writes on directories

mischief provided the following test that shows the issue:

ramfs -S crash

aux/9pcon /srv/crash <<EOF
Tversion 8192 9P2000
Tattach 0 -1 $user ''
Tcreate 0 dir 020000000777 0
Tattach 5 -1 $user ''
Twalk 5 6 dir
Tread 6 0 512
EOF

the problem is that lib9p wrongly allowed reads on closed fids,
due to the permission check only considering the lower 2 bits.
a closed fid has fid->omode == -1 and it would pass on read for:

(-1 & 3) == 3 == OEXEC

the following change explicitely checks for for the closed case
and also rejects writes on directories (they are rejected on
open/create, but a broken 9p client could still issue the request).

--- a/sys/src/lib9p/srv.c

+++ b/sys/src/lib9p/srv.c

@@ -214,8 +214,14 @@

 static void

 rauth(Req *r, char *error)

 {

-	if(error && r->afid)

+	if(r->afid == nil)

+		return;

+	if(error){

 		closefid(removefid(r->srv->fpool, r->afid->fid));

+		return;

+	}

+	if(r->afid->omode == -1)

+		r->afid->omode = ORDWR;

 }

 static void

@@ -402,7 +408,8 @@

 	r->ofcall.qid = r->fid->qid;

 	switch(r->ifcall.mode&3){

 	default:

-		assert(0);

+		respond(r, Ebotch);

+		return;

 	case OREAD:

 		p = AREAD;	

 		break;

@@ -443,21 +450,6 @@

 	else

 		respond(r, nil);

 }

-static void

-ropen(Req *r, char *error)

-{

-	char errbuf[ERRMAX];

-	if(error)

-		return;

-	if(chatty9p){

-		snprint(errbuf, sizeof errbuf, "fid mode is 0x%ux\n", r->ifcall.mode);

-		write(2, errbuf, strlen(errbuf));

-	}

-	r->fid->omode = r->ifcall.mode;

-	r->fid->qid = r->ofcall.qid;

-	if(r->ofcall.qid.type&QTDIR)

-		r->fid->diroffset = 0;

-}

 static void

 screate(Srv *srv, Req *r)

@@ -475,13 +467,18 @@

 	else

 		respond(r, Enocreate);

 }

+

 static void

-rcreate(Req *r, char *error)

+ropen(Req *r, char *error)

 {

 	if(error)

 		return;

-	r->fid->omode = r->ifcall.mode;

+	if(chatty9p)

+		fprint(2, "fid mode is %x\n", (int)r->ifcall.mode);

+	if(r->ofcall.qid.type&QTDIR)

+		r->fid->diroffset = 0;

 	r->fid->qid = r->ofcall.qid;

+	r->fid->omode = r->ifcall.mode;

 }

 static void

@@ -493,6 +490,20 @@

 		respond(r, Eunknownfid);

 		return;

 	}

+	o = r->fid->omode;

+	if(o == -1){

+		respond(r, Ebotch);

+		return;

+	}

+	switch(o & 3){

+	default:

+		respond(r, Ebotch);

+		return;

+	case OREAD:

+	case ORDWR:

+	case OEXEC:

+		break;

+	}

 	if((int)r->ifcall.count < 0){

 		respond(r, Ebotch);

 		return;

@@ -502,16 +513,10 @@

 		respond(r, Ebadoffset);

 		return;

 	}

-

 	if(r->ifcall.count > srv->msize - IOHDRSZ)

 		r->ifcall.count = srv->msize - IOHDRSZ;

 	r->rbuf = emalloc9p(r->ifcall.count);

 	r->ofcall.data = r->rbuf;

-	o = r->fid->omode & 3;

-	if(o != OREAD && o != ORDWR && o != OEXEC){

-		respond(r, Ebotch);

-		return;

-	}

 	if((r->fid->qid.type&QTDIR) && r->fid->file){

 		r->ofcall.count = readdirfile(r->fid->rdir, r->rbuf, r->ifcall.count, r->ifcall.offset);

 		respond(r, nil);

@@ -533,12 +538,28 @@

 swrite(Srv *srv, Req *r)

 {

 	int o;

-	char e[ERRMAX];

 	if((r->fid = lookupfid(srv->fpool, r->ifcall.fid)) == nil){

 		respond(r, Eunknownfid);

 		return;

 	}

+	o = r->fid->omode;

+	if(o == -1){

+		respond(r, Ebotch);

+		return;

+	}

+	switch(o & 3){

+	default:

+		respond(r, Ebotch);

+		return;

+	case OWRITE:

+	case ORDWR:

+		break;

+	}

+	if(r->fid->qid.type&QTDIR){

+		respond(r, Ebotch);

+		return;

+	}

 	if((int)r->ifcall.count < 0){

 		respond(r, Ebotch);

 		return;

@@ -549,12 +570,6 @@

 	}

 	if(r->ifcall.count > srv->msize - IOHDRSZ)

 		r->ifcall.count = srv->msize - IOHDRSZ;

-	o = r->fid->omode & 3;

-	if(o != OWRITE && o != ORDWR){

-		snprint(e, sizeof e, "write on fid with open mode 0x%ux", r->fid->omode);

-		respond(r, e);

-		return;

-	}

 	if(srv->write)

 		srv->write(r);

 	else

@@ -860,7 +875,7 @@

 	case Tattach:	rattach(r, error);	break;

 	case Twalk:	rwalk(r, error);	break;

 	case Topen:	ropen(r, error);	break;

-	case Tcreate:	rcreate(r, error);	break;

+	case Tcreate:	ropen(r, error);	break;

 	case Tread:	rread(r, error);	break;

 	case Twrite:	rwrite(r, error);	break;

 	case Tclunk:	rclunk(r, error);	break;