ref: 664dd820a26824ee2bda86fa7b0e9baa8cde9316
parent: b0d03479d85f43183cfbd113b96a7484c84e73d5
author: cinap_lenrek <cinap_lenrek@felloff.net>
date: Fri Aug 30 14:46:50 EDT 2024
gefs: fix Fid refcounting bugs The AuthRpc was attached to the Fid, but this doesnt work as it does not handle dupfid() properly. Instead attach the AuthRpc to the directory entry, which is refcounted. The dupfid() function retuns the new Fid* struct with an extra reference. If we don't use it, we have to putfid() it. Use ainc()/adec() consistently and dont mix it with agetl().
--- a/sys/src/cmd/gefs/blk.c
+++ b/sys/src/cmd/gefs/blk.c
@@ -806,7 +806,7 @@
p = agetp(&fs->limbo[ge]);
l->next = p;
if(acasp(&fs->limbo[ge], p, l)){- aincl(&fs->nlimbo, 1);
+ ainc(&fs->nlimbo);
break;
}
}
@@ -955,7 +955,7 @@
default:
abort();
}
- aincl(&fs->nlimbo, -1);
+ adec(&fs->nlimbo);
}
}
@@ -1082,7 +1082,7 @@
q = p;
if(waserror()){- aincl(&fs->rdonly, 1);
+ ainc(&fs->rdonly);
fprint(2, "error syncing: %s\n", errmsg());
return;
}
--- a/sys/src/cmd/gefs/check.c
+++ b/sys/src/cmd/gefs/check.c
@@ -255,7 +255,7 @@
Blk *b;
ok = 1;
- aincl(&fs->rdonly, 1);
+ ainc(&fs->rdonly);
epochwait();
if(waserror()){fprint(fd, "error checking %s\n", errmsg());
@@ -300,7 +300,7 @@
poperror();
}
btexit(&s);
- aincl(&fs->rdonly, -1);
+ adec(&fs->rdonly);
poperror();
return ok;
}
--- a/sys/src/cmd/gefs/dat.h
+++ b/sys/src/cmd/gefs/dat.h
@@ -629,7 +629,10 @@
char gone;
char trunc;
- char buf[Maxent];
+ union {+ char buf[Maxent];
+ void *auth;
+ };
};
struct Mount {@@ -685,7 +688,6 @@
Dent *dent; /* (pqid, name) ref, modified on rename */
Dent *dir;
Amsg *rclose;
- void *auth;
u32int fid;
vlong qpath;
--- a/sys/src/cmd/gefs/fs.c
+++ b/sys/src/cmd/gefs/fs.c
@@ -12,6 +12,8 @@
static void rerror(Fmsg*, char*, ...);
static void clunkfid(Conn*, Fid*, Amsg**);
+static void authfree(AuthRpc*);
+
int
walk1(Tree *t, vlong up, char *name, Qid *qid, vlong *len)
{@@ -228,7 +230,7 @@
return;
}
if(t->nlbl == 1 && t->nref <= 1 && t->succ == -1){- aincl(&t->memref, 1);
+ ainc(&t->memref);
*tp = t;
}
delsnap(t, t->succ, a->old);
@@ -676,8 +678,11 @@
if(de == nil)
return;
- if(de->qid.type & QTAUTH && adec(&de->ref) == 0){- free(de);
+ if(de->qid.type & QTAUTH){+ if(adec(&de->ref) == 0){+ authfree(de->auth);
+ free(de);
+ }
return;
}
lock(&mnt->dtablk);
@@ -797,8 +802,8 @@
f->dent->gone = 1;
wunlock(f->dent);
- aincl(&f->dent->ref, 1);
- aincl(&f->mnt->ref, 1);
+ ainc(&f->dent->ref);
+ ainc(&f->mnt->ref);
(*ao)->op = AOrclose;
(*ao)->mnt = f->mnt;
(*ao)->qpath = f->qpath;
@@ -837,7 +842,7 @@
free(m);
return -1;
}
- aincl(&c->ref, 1);
+ ainc(&c->ref);
m->conn = c;
m->sz = sz;
PBIT32(m->buf, sz);
@@ -870,7 +875,7 @@
respond(m, &r);
}
-void
+static void
authfree(AuthRpc *auth)
{AuthRpc *rpc;
@@ -911,7 +916,7 @@
AuthRpc *rpc;
User *u;
- if((rpc = f->auth) == nil)
+ if((f->dir->qid.type & QTAUTH) == 0 || (rpc = f->dir->auth) == nil)
error(Etype);
switch(auth_rpc(rpc, "read", nil, 0)){@@ -947,7 +952,7 @@
{AuthRpc *rpc;
- if((rpc = f->auth) == nil)
+ if((f->dir->qid.type & QTAUTH) == 0 || (rpc = f->dir->auth) == nil)
error(Etype);
if(auth_rpc(rpc, "write", data, count) != ARok)
error(Ebotch);
@@ -961,7 +966,7 @@
{Dent *de;
Fcall r;
- Fid f;
+ Fid f, *nf;
if(fs->noauth){rerror(m, Eauth);
@@ -976,6 +981,11 @@
return;
}
memset(de, 0, sizeof(Dent));
+ de->auth = authnew();
+ if(de->auth == nil){+ rerror(m, errmsg());
+ return;
+ }
de->ref = 0;
de->qid.type = QTAUTH;
de->qid.path = aincv(&fs->nextqid, 1);
@@ -997,9 +1007,10 @@
f.duid = -1;
f.dgid = -1;
f.dmode = 0600;
- f.auth = authnew();
- if(dupfid(m->conn, m->afid, &f) == nil){+ nf = dupfid(m->conn, m->afid, &f);
+ if(nf == nil){rerror(m, Efid);
+ authfree(de->auth);
free(de);
return;
}
@@ -1006,6 +1017,7 @@
r.type = Rauth;
r.aqid = de->qid;
respond(m, &r);
+ putfid(nf);
}
static int
@@ -1105,7 +1117,7 @@
Xdir d;
Kvp kv;
Key dk;
- Fid f, *af;
+ Fid f, *af, *nf;
int uid;
de = nil;
@@ -1188,12 +1200,13 @@
}
if(strcmp(aname, "dump") == 0)
f.fromdump = 1;
- if(dupfid(m->conn, m->fid, &f) == nil)
+ nf = dupfid(m->conn, m->fid, &f);
+ if(nf == nil)
error(Efid);
-
r.type = Rattach;
r.qid = d.qid;
respond(m, &r);
+ putfid(nf);
poperror();
@@ -1307,6 +1320,12 @@
if((f = dupfid(m->conn, m->newfid, o)) == nil)
error(Efid);
putfid(o);
+ poperror();
+ if(waserror()){+ rerror(m, errmsg());
+ putfid(f);
+ return;
+ }
}
if(i > 0 && i == m->nwname){lock(f);
@@ -1349,8 +1368,8 @@
unlock(f);
}
respond(m, &r);
- poperror();
putfid(f);
+ poperror();
}
static void
@@ -1457,8 +1476,8 @@
qlock(&de->trunclk);
de->trunc = 1;
qunlock(&de->trunclk);
- aincl(&de->ref, 1);
- aincl(&f->mnt->ref, 1);
+ ainc(&de->ref);
+ ainc(&f->mnt->ref);
(*ao)->op = AOclear;
(*ao)->mnt = f->mnt;
(*ao)->qpath = f->qpath;
@@ -1829,7 +1848,7 @@
}else{if(*ao == nil)
*ao = emalloc(sizeof(Amsg), 1);
- aincl(&f->mnt->ref, 1);
+ ainc(&f->mnt->ref);
(*ao)->op = AOclear;
(*ao)->mnt = f->mnt;
(*ao)->qpath = f->qpath;
@@ -1922,8 +1941,8 @@
qlock(&f->dent->trunclk);
f->dent->trunc = 1;
qunlock(&f->dent->trunclk);
- aincl(&f->dent->ref, 1);
- aincl(&f->mnt->ref, 1);
+ ainc(&f->dent->ref);
+ ainc(&f->mnt->ref);
(*ao)->op = AOclear;
(*ao)->mnt = f->mnt;
(*ao)->qpath = f->qpath;
@@ -2275,7 +2294,7 @@
Fid *f;
int i;
- if(aincl(&c->ref, -1))
+ if(adec(&c->ref) != 0)
return;
lock(&fs->connlk);
@@ -2297,16 +2316,12 @@
lock(&c->fidtablk[i]);
for(f = c->fidtab[i]; f != nil; f = f->next){lock(f);
- if(waserror()){- unlock(f);
- continue;
- }
a = nil;
clunkfid(c, f, &a);
unlock(f);
+ putfid(f);
if(a != nil)
chsend(fs->admchan, a);
- nexterror();
}
unlock(&c->fidtablk[i]);
}
@@ -2414,9 +2429,11 @@
rerror(m, Enofid);
continue;
}
+ lock(f);
clunkfid(m->conn, f, &a);
/* read only: ignore rclose */
f->rclose = nil;
+ unlock(f);
free(a);
putfid(f);
}
--- a/sys/src/cmd/gefs/main.c
+++ b/sys/src/cmd/gefs/main.c
@@ -119,7 +119,7 @@
{va_list ap;
- aincl(&fs->rdonly, 1);
+ ainc(&fs->rdonly);
va_start(ap, fmt);
errorv(fmt, ap, 1);
}
--
⑨