shithub: riscv

Info  •  Files  •  Log  •  Branches

devmnt: fix mount device leak and allocation error handling in mntversion()

the fist problem is that qopen() might return nil and that kstrdup() will
sleep, so we should try to avoid holding the mntalloc lock. so we move
the kstrdup() and qopen() calls before the Mnt allocation, and properly
recover the memory if we fail later.

the second problem was that we error(Eshort) after we already created the Mnt
when returnlen < sizeof(f.version). this check has to happen *before* we
even attempt to allocate the Mnt structures. note that we only copy the
version string once everything is in the clear, so the semantics of the
user buffer not being modified in case of error is not changed.

a little cleanup in muxclose(), getting rid of mntptfree()...

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

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

@@ -61,7 +61,6 @@

 void	mntflushfree(Mnt*, Mntrpc*);

 void	mntfree(Mntrpc*);

 void	mntgate(Mnt*);

-void	mntpntfree(Mnt*);

 void	mntqrm(Mnt*, Mntrpc*);

 Mntrpc*	mntralloc(Chan*, ulong);

 long	mntrdwr(int, Chan*, void*, long, vlong);

@@ -101,6 +100,7 @@

 	uchar *msg;

 	Mnt *m;

 	char *v;

+	Queue *q;

 	long k, l;

 	uvlong oo;

 	char buf[128];

@@ -199,7 +199,17 @@

 	k = strlen(f.version);

 	if(strncmp(f.version, v, k) != 0)

 		error("bad 9P version returned from server");

+	if(returnlen > 0 && returnlen < k)

+		error(Eshort);

+	v = nil;

+	kstrdup(&v, f.version);

+	q = qopen(10*MAXRPC, 0, nil, nil);

+	if(q == nil){

+		free(v);

+		exhausted("mount queues");

+	}

+

 	/* now build Mnt associated with this connection */

 	lock(&mntalloc);

 	m = mntalloc.mntfree;

@@ -208,6 +218,8 @@

 	else {

 		m = malloc(sizeof(Mnt));

 		if(m == 0) {

+			qfree(q);

+			free(v);

 			unlock(&mntalloc);

 			exhausted("mount devices");

 		}

@@ -214,18 +226,14 @@

 	}

 	m->list = mntalloc.list;

 	mntalloc.list = m;

-	m->version = nil;

-	kstrdup(&m->version, f.version);

+	m->version = v;

 	m->id = mntalloc.id++;

-	m->q = qopen(10*MAXRPC, 0, nil, nil);

+	m->q = q;

 	m->msize = f.msize;

 	unlock(&mntalloc);

-	if(returnlen > 0){

-		if(returnlen < k)

-			error(Eshort);

-		memmove(version, f.version, k);

-	}

+	if(returnlen > 0)

+		memmove(version, f.version, k);	/* length was checked above */

 	poperror();	/* msg */

 	free(msg);

@@ -564,24 +572,19 @@

 void

 muxclose(Mnt *m)

 {

-	Mntrpc *q, *r;

+	Mnt *f, **l;

+	Mntrpc *r;

-	for(q = m->queue; q; q = r) {

-		r = q->list;

-		mntfree(q);

+	while((r = m->queue) != nil){

+		m->queue = r->list;

+		mntfree(r);

 	}

 	m->id = 0;

 	free(m->version);

 	m->version = nil;

-	mntpntfree(m);

-}

+	qfree(m->q);

+	m->q = nil;

-void

-mntpntfree(Mnt *m)

-{

-	Mnt *f, **l;

-	Queue *q;

-

 	lock(&mntalloc);

 	l = &mntalloc.list;

 	for(f = *l; f; f = f->list) {

@@ -593,10 +596,7 @@

 	}

 	m->list = mntalloc.mntfree;

 	mntalloc.mntfree = m;

-	q = m->q;

 	unlock(&mntalloc);

-

-	qfree(q);

 }

 static void

-- 

⑨