From 47bb311d39f0cdd38067a31b51ec619af7584e56 Mon Sep 17 00:00:00 2001 From: cinap_lenrek Date: Sun, 19 Jul 2015 20:25:42 +0200 Subject: [PATCH] devmnt: do not use user buffer to update the mount cache using the user buffer has a race where the user can modify the buffer from another process before it is copied into the cache. this allows poisoning the cache for every file where the user has read access. instead, we update the cache from kernel memory. --- sys/src/9/port/devmnt.c | 70 ++++++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 29 deletions(-) diff --git a/sys/src/9/port/devmnt.c b/sys/src/9/port/devmnt.c index 2b6610afc..91d6ad2f9 100644 --- a/sys/src/9/port/devmnt.c +++ b/sys/src/9/port/devmnt.c @@ -634,32 +634,11 @@ static long mntread(Chan *c, void *buf, long n, vlong off) { uchar *p, *e; - int nc, cache, isdir, dirlen; - - isdir = 0; - cache = c->flag & CCACHE; - if(c->qid.type & QTDIR) { - cache = 0; - isdir = 1; - } + int dirlen; p = buf; - if(cache) { - nc = cread(c, buf, n, off); - if(nc > 0) { - n -= nc; - if(n == 0) - return nc; - p += nc; - off += nc; - } - n = mntrdwr(Tread, c, p, n, off); - cupdate(c, p, n, off); - return n + nc; - } - - n = mntrdwr(Tread, c, buf, n, off); - if(isdir) { + n = mntrdwr(Tread, c, p, n, off); + if(c->qid.type & QTDIR) { for(e = &p[n]; p+BIT16SZ < e; p += dirlen){ dirlen = BIT16SZ+GBIT16(p); if(p+dirlen > e) @@ -695,6 +674,14 @@ mntrdwr(int type, Chan *c, void *buf, long n, vlong off) if(c->qid.type & QTDIR) cache = 0; for(;;) { + if(cache && type == Tread) { + nr = cread(c, (uchar*)uba, n, off); + if(nr > 0) { + nreq = nr; + goto Next; + } + } + r = mntralloc(c, m->msize); if(waserror()) { mntfree(r); @@ -714,13 +701,39 @@ mntrdwr(int type, Chan *c, void *buf, long n, vlong off) if(nr > nreq) nr = nreq; + if(cache) { + /* + * note that we cannot update the cache from uba as + * the user could change its contents from another + * process before the data gets copied to the cached. + */ + if(type == Tread) { + ulong nc, nn; + Block *b; + + nc = 0; + for(b = r->b; b != nil; b = b->next) { + nn = BLEN(b); + if(nc+nn > nr) + nn = nr - nc; + cupdate(c, b->rp, nn, off + nc); + nc += nn; + if(nc >= nr) + break; + } + } else { + if(convM2S(r->rpc, r->rpclen, &r->request) == 0) + panic("convM2S"); + cwrite(c, (uchar*)r->request.data, nr, off); + } + } + if(type == Tread) r->b = bl2mem((uchar*)uba, r->b, nr); - else if(cache) - cwrite(c, (uchar*)uba, nr, off); - - poperror(); mntfree(r); + poperror(); + + Next: off += nr; uba += nr; cnt += nr; @@ -807,7 +820,6 @@ mountio(Mnt *m, Mntrpc *r) up->text, up->pid, n, r->request.tag, r->request.fid, r->request.type); error(Emountrpc); } - if(devtab[m->c->type]->write(m->c, r->rpc, n, 0) != n) error(Emountrpc);