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()...
This commit is contained in:
parent
fe073f852a
commit
25bc4e84e9
1 changed files with 24 additions and 24 deletions
|
@ -61,7 +61,6 @@ Mntrpc* mntflushalloc(Mntrpc*, ulong);
|
|||
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 @@ mntversion(Chan *c, char *version, int msize, int returnlen)
|
|||
uchar *msg;
|
||||
Mnt *m;
|
||||
char *v;
|
||||
Queue *q;
|
||||
long k, l;
|
||||
uvlong oo;
|
||||
char buf[128];
|
||||
|
@ -199,6 +199,16 @@ mntversion(Chan *c, char *version, int msize, int returnlen)
|
|||
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);
|
||||
|
@ -208,24 +218,22 @@ mntversion(Chan *c, char *version, int msize, int returnlen)
|
|||
else {
|
||||
m = malloc(sizeof(Mnt));
|
||||
if(m == 0) {
|
||||
qfree(q);
|
||||
free(v);
|
||||
unlock(&mntalloc);
|
||||
exhausted("mount devices");
|
||||
}
|
||||
}
|
||||
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,23 +572,18 @@ mntclunk(Chan *c, int t)
|
|||
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);
|
||||
}
|
||||
|
||||
void
|
||||
mntpntfree(Mnt *m)
|
||||
{
|
||||
Mnt *f, **l;
|
||||
Queue *q;
|
||||
qfree(m->q);
|
||||
m->q = nil;
|
||||
|
||||
lock(&mntalloc);
|
||||
l = &mntalloc.list;
|
||||
|
@ -593,10 +596,7 @@ mntpntfree(Mnt *m)
|
|||
}
|
||||
m->list = mntalloc.mntfree;
|
||||
mntalloc.mntfree = m;
|
||||
q = m->q;
|
||||
unlock(&mntalloc);
|
||||
|
||||
qfree(q);
|
||||
}
|
||||
|
||||
static void
|
||||
|
|
Loading…
Reference in a new issue