ref: 15341e1116cd0fe31dcf3fb989d2004a8d3a3e8f
parent: d8cf26110d5c98a0568260caf772ed8c10b2a395
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);
}
--
⑨