xfnw/plan9fox
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

sshfs: fix race condition between sendproc() and recvproc()

Browse source 
there was a race between the sendproc putting the request on
the sreqrd[] id and the recvproc handling the response, and
potentially freeing the request before the sendproc() was
finished with the request (or the fid).

so we defer allocating a request id and putting it on the
sreqrd[] stage after we have completely generated the
request in vpack(). this prevents the handling of the request
before it is even sent.

this also means that the SReq should not be touched after
calling sendpkt(), submitreq(), submitsreq().

secondly, putsfid() needs to acquire the RWLock to make sure
sendproc() is finished with the request. the scenario is that
recvproc() can call respond() on the request before sendproc()
has unlocked the SFid.
This commit is contained in:
cinap_lenrek 2019-10-07 11:52:14 +02:00
parent af23bb343a
commit f763dc1640
1 changed files with 75 additions and 66 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

141
sys/src/cmd/sshfs.c
View file

@ -103,7 +103,6 @@ struct SFid {
struct SReq { struct SReq {
Req *req; Req *req;
SFid *closefid; SFid *closefid;
int reqid;
SReq *next; SReq *next;
}; };
@ -193,10 +192,30 @@ namelookup(IDEnt **tab, char *name)
return -1; return -1;
} }
int
allocsreqid(SReq *r)
{
int i;
qlock(&sreqidlock);
for(;;){
for(i = 0; i < MAXREQID; i++)
if(sreqrd[i] == nil){
sreqrd[i] = r;
goto out;
}
rsleep(&sreqidrend);
}
out:
qunlock(&sreqidlock);
return i;
}
int int
vpack(uchar *p, int n, char *fmt, va_list a) vpack(uchar *p, int n, char *fmt, va_list a)
{ {
uchar *p0 = p, *e = p+n; uchar *p0 = p, *e = p+n;
SReq *sr = nil;
u32int u; u32int u;
u64int v; u64int v;
void *s; void *s;
@ -205,6 +224,10 @@ vpack(uchar *p, int n, char *fmt, va_list a)
for(;;){ for(;;){
switch(c = *fmt++){ switch(c = *fmt++){
case '\0': case '\0':
if(sr != nil){
u = allocsreqid(sr);
PUT4(p0+1, u);
}
return p - p0; return p - p0;
case '_': case '_':
if(++p > e) goto err; if(++p > e) goto err;
@ -228,6 +251,11 @@ vpack(uchar *p, int n, char *fmt, va_list a)
memmove(p, s, u); memmove(p, s, u);
p += u; p += u;
break; break;
case 'q':
p += 4;
if(p != p0+5 || p > e) goto err;
sr = va_arg(a, SReq*);
break;
case 'u': case 'u':
u = va_arg(a, int); u = va_arg(a, int);
if(p+4 > e) goto err; if(p+4 > e) goto err;
@ -389,11 +417,14 @@ freedir(SFid *s)
s->dirpos = 0; s->dirpos = 0;
} }
void void
putsfid(SFid *s) putsfid(SFid *s)
{ {
if(s == nil) return; if(s == nil) return;
wlock(s);
wunlock(s);
free(s->fn); free(s->fn);
free(s->hand); free(s->hand);
freedir(s); freedir(s);
@ -403,14 +434,6 @@ putsfid(SFid *s)
void void
putsreq(SReq *s) putsreq(SReq *s)
{ {
if(s == nil) return;
if(s->reqid != -1){
qlock(&sreqidlock);
sreqrd[s->reqid] = nil;
rwakeup(&sreqidrend);
qunlock(&sreqidlock);
}
putsfid(s->closefid);
free(s); free(s);
} }
@ -424,14 +447,12 @@ submitsreq(SReq *s)
qunlock(&sreqwrlock); qunlock(&sreqwrlock);
} }
void void
submitreq(Req *r) submitreq(Req *r)
{ {
SReq *s; SReq *s;
s = emalloc9p(sizeof(SReq)); s = emalloc9p(sizeof(SReq));
s->reqid = -1;
s->req = r; s->req = r;
submitsreq(s); submitsreq(s);
} }
@ -697,20 +718,18 @@ readprocess(Req *r)
void void
sshfsread(Req *r) sshfsread(Req *r)
{ {
SFid *sf;
if((r->fid->qid.type & QTDIR) == 0){ if((r->fid->qid.type & QTDIR) == 0){
submitreq(r); submitreq(r);
return; return;
} }
sf = r->fid->aux;
if(r->ifcall.offset == 0){ if(r->ifcall.offset == 0){
SFid *sf = r->fid->aux;
wlock(sf); wlock(sf);
freedir(sf); freedir(sf);
if(sf->dirreads > 0){ if(sf->dirreads > 0){
wunlock(sf);
r->aux = (void*)-1; r->aux = (void*)-1;
submitreq(r); submitreq(r);
wunlock(sf);
return; return;
} }
wunlock(sf); wunlock(sf);
@ -764,7 +783,6 @@ sendproc(void *)
{ {
SReq *r; SReq *r;
SFid *sf; SFid *sf;
int i;
int x, y; int x, y;
char *s, *t; char *s, *t;
@ -778,41 +796,33 @@ sendproc(void *)
sreqwr = r->next; sreqwr = r->next;
if(sreqwr == nil) sreqlast = &sreqwr; if(sreqwr == nil) sreqlast = &sreqwr;
qunlock(&sreqwrlock); qunlock(&sreqwrlock);
qlock(&sreqidlock);
idagain:
for(i = 0; i < MAXREQID; i++)
if(sreqrd[i] == nil){
sreqrd[i] = r;
r->reqid = i;
break;
}
if(i == MAXREQID){
rsleep(&sreqidrend);
goto idagain;
}
qunlock(&sreqidlock);
if(r->closefid != nil){ sf = r->closefid;
sendpkt("bus", SSH_FXP_CLOSE, r->reqid, r->closefid->hand, r->closefid->handn); if(sf != nil){
rlock(sf);
sendpkt("bqs", SSH_FXP_CLOSE, r, sf->hand, sf->handn);
runlock(sf);
continue; continue;
} }
if(r->req == nil) if(r->req == nil)
sysfatal("nil request in queue"); sysfatal("nil request in queue");
sf = r->req->fid != nil ? r->req->fid->aux : nil; sf = r->req->fid->aux;
switch(r->req->ifcall.type){ switch(r->req->ifcall.type){
case Tattach: case Tattach:
sendpkt("bus", SSH_FXP_STAT, r->reqid, sf->fn, strlen(sf->fn)); rlock(sf);
sendpkt("bqs", SSH_FXP_STAT, r, sf->fn, strlen(sf->fn));
runlock(sf);
break; break;
case Twalk: case Twalk:
sendpkt("bus", SSH_FXP_STAT, r->reqid, r->req->aux, strlen(r->req->aux)); sendpkt("bqs", SSH_FXP_STAT, r, r->req->aux, strlen(r->req->aux));
break; break;
case Topen: case Topen:
rlock(sf); if((r->req->ofcall.qid.type & QTDIR) != 0){
if((r->req->ofcall.qid.type & QTDIR) != 0) rlock(sf);
sendpkt("bus", SSH_FXP_OPENDIR, r->reqid, sf->fn, strlen(sf->fn)); sendpkt("bqs", SSH_FXP_OPENDIR, r, sf->fn, strlen(sf->fn));
else{ runlock(sf);
}else{
x = r->req->ifcall.mode; x = r->req->ifcall.mode;
y = 0; y = 0;
switch(x & 3){ switch(x & 3){
@ -822,15 +832,15 @@ sendproc(void *)
} }
if(readonly && (y & SSH_FXF_WRITE) != 0){ if(readonly && (y & SSH_FXF_WRITE) != 0){
respond(r->req, "mounted read-only"); respond(r->req, "mounted read-only");
runlock(sf);
putsreq(r); putsreq(r);
break; break;
} }
if((x & OTRUNC) != 0) if((x & OTRUNC) != 0)
y |= SSH_FXF_TRUNC; y |= SSH_FXF_TRUNC;
sendpkt("busuu", SSH_FXP_OPEN, r->reqid, sf->fn, strlen(sf->fn), y, 0); rlock(sf);
sendpkt("bqsuu", SSH_FXP_OPEN, r, sf->fn, strlen(sf->fn), y, 0);
runlock(sf);
} }
runlock(sf);
break; break;
case Tcreate: case Tcreate:
rlock(sf); rlock(sf);
@ -838,12 +848,12 @@ sendproc(void *)
runlock(sf); runlock(sf);
if((r->req->ifcall.perm & DMDIR) != 0){ if((r->req->ifcall.perm & DMDIR) != 0){
if(r->req->aux == nil){ if(r->req->aux == nil){
sendpkt("busuu", SSH_FXP_MKDIR, r->reqid, s, strlen(s),
SSH_FILEXFER_ATTR_PERMISSIONS, r->req->ifcall.perm & 0777);
r->req->aux = (void*)-1; r->req->aux = (void*)-1;
sendpkt("bqsuu", SSH_FXP_MKDIR, r, s, strlen(s),
SSH_FILEXFER_ATTR_PERMISSIONS, r->req->ifcall.perm & 0777);
}else{ }else{
sendpkt("bus", SSH_FXP_OPENDIR, r->reqid, s, strlen(s));
r->req->aux = (void*)-2; r->req->aux = (void*)-2;
sendpkt("bqs", SSH_FXP_OPENDIR, r, s, strlen(s));
} }
free(s); free(s);
break; break;
@ -855,7 +865,7 @@ sendproc(void *)
case OWRITE: y |= SSH_FXF_WRITE; break; case OWRITE: y |= SSH_FXF_WRITE; break;
case ORDWR: y |= SSH_FXF_READ | SSH_FXF_WRITE; break; case ORDWR: y |= SSH_FXF_READ | SSH_FXF_WRITE; break;
} }
sendpkt("busuuu", SSH_FXP_OPEN, r->reqid, s, strlen(s), y, sendpkt("bqsuuu", SSH_FXP_OPEN, r, s, strlen(s), y,
SSH_FILEXFER_ATTR_PERMISSIONS, r->req->ifcall.perm & 0777); SSH_FILEXFER_ATTR_PERMISSIONS, r->req->ifcall.perm & 0777);
free(s); free(s);
break; break;
@ -863,22 +873,22 @@ sendproc(void *)
if((r->req->fid->qid.type & QTDIR) != 0){ if((r->req->fid->qid.type & QTDIR) != 0){
wlock(sf); wlock(sf);
if(r->req->aux == (void*)-1){ if(r->req->aux == (void*)-1){
sendpkt("bus", SSH_FXP_CLOSE, r->reqid, sf->hand, sf->handn); sendpkt("bqs", SSH_FXP_CLOSE, r, sf->hand, sf->handn);
free(sf->hand); free(sf->hand);
sf->hand = nil; sf->hand = nil;
sf->handn = 0; sf->handn = 0;
sf->direof = 0; sf->direof = 0;
sf->dirreads = 0; sf->dirreads = 0;
}else if(r->req->aux == (void*)-2){ }else if(r->req->aux == (void*)-2){
sendpkt("bus", SSH_FXP_OPENDIR, r->reqid, sf->fn, strlen(sf->fn)); sendpkt("bqs", SSH_FXP_OPENDIR, r, sf->fn, strlen(sf->fn));
}else{ }else{
sf->dirreads++; sf->dirreads++;
sendpkt("bus", SSH_FXP_READDIR, r->reqid, sf->hand, sf->handn); sendpkt("bqs", SSH_FXP_READDIR, r, sf->hand, sf->handn);
} }
wunlock(sf); wunlock(sf);
}else{ }else{
rlock(sf); rlock(sf);
sendpkt("busvuu", SSH_FXP_READ, r->reqid, sf->hand, sf->handn, sendpkt("bqsvuu", SSH_FXP_READ, r, sf->hand, sf->handn,
r->req->ifcall.offset, r->req->ifcall.count); r->req->ifcall.offset, r->req->ifcall.count);
runlock(sf); runlock(sf);
} }
@ -886,33 +896,33 @@ sendproc(void *)
case Twrite: case Twrite:
x = r->req->ifcall.count - r->req->ofcall.count; x = r->req->ifcall.count - r->req->ofcall.count;
if(x >= MAXWRITE) x = MAXWRITE; if(x >= MAXWRITE) x = MAXWRITE;
r->req->ofcall.offset = x;
rlock(sf); rlock(sf);
sendpkt("busvs", SSH_FXP_WRITE, r->reqid, sf->hand, sf->handn, sendpkt("bqsvs", SSH_FXP_WRITE, r, sf->hand, sf->handn,
r->req->ifcall.offset + r->req->ofcall.count, r->req->ifcall.offset + r->req->ofcall.count,
r->req->ifcall.data + r->req->ofcall.count, r->req->ifcall.data + r->req->ofcall.count,
x); x);
runlock(sf); runlock(sf);
r->req->ofcall.offset = x;
break; break;
case Tstat: case Tstat:
rlock(sf); rlock(sf);
r->req->d.name = finalelem(sf->fn); r->req->d.name = finalelem(sf->fn);
r->req->d.qid = sf->qid; r->req->d.qid = sf->qid;
if(sf->handn > 0 && (sf->qid.type & QTDIR) == 0) if(sf->handn > 0 && (sf->qid.type & QTDIR) == 0)
sendpkt("bus", SSH_FXP_FSTAT, r->reqid, sf->hand, sf->handn); sendpkt("bqs", SSH_FXP_FSTAT, r, sf->hand, sf->handn);
else else
sendpkt("bus", SSH_FXP_STAT, r->reqid, sf->fn, strlen(sf->fn)); sendpkt("bqs", SSH_FXP_STAT, r, sf->fn, strlen(sf->fn));
runlock(sf); runlock(sf);
break; break;
case Twstat: case Twstat:
if(r->req->aux == (void *) -1){ if(r->req->aux == (void*)-1){
rlock(sf); rlock(sf);
s = parentdir(sf->fn); s = parentdir(sf->fn);
t = pathcat(s, r->req->d.name); t = pathcat(s, r->req->d.name);
free(s);
r->req->aux = t; r->req->aux = t;
sendpkt("buss", SSH_FXP_RENAME, r->reqid, sf->fn, strlen(sf->fn), t, strlen(t)); sendpkt("bqss", SSH_FXP_RENAME, r, sf->fn, strlen(sf->fn), t, strlen(t));
runlock(sf); runlock(sf);
free(s);
break; break;
} }
x = dir2attrib(&r->req->d, (uchar **) &s); x = dir2attrib(&r->req->d, (uchar **) &s);
@ -923,18 +933,18 @@ sendproc(void *)
} }
rlock(sf); rlock(sf);
if(sf->handn > 0) if(sf->handn > 0)
sendpkt("bus[", SSH_FXP_FSETSTAT, r->reqid, sf->hand, sf->handn, s, x); sendpkt("bqs[", SSH_FXP_FSETSTAT, r, sf->hand, sf->handn, s, x);
else else
sendpkt("bus[", SSH_FXP_SETSTAT, r->reqid, sf->fn, strlen(sf->fn), s, x); sendpkt("bqs[", SSH_FXP_SETSTAT, r, sf->fn, strlen(sf->fn), s, x);
runlock(sf); runlock(sf);
free(s); free(s);
break; break;
case Tremove: case Tremove:
rlock(sf); rlock(sf);
if((sf->qid.type & QTDIR) != 0) if((sf->qid.type & QTDIR) != 0)
sendpkt("bus", SSH_FXP_RMDIR, r->reqid, sf->fn, strlen(sf->fn)); sendpkt("bqs", SSH_FXP_RMDIR, r, sf->fn, strlen(sf->fn));
else else
sendpkt("bus", SSH_FXP_REMOVE, r->reqid, sf->fn, strlen(sf->fn)); sendpkt("bqs", SSH_FXP_REMOVE, r, sf->fn, strlen(sf->fn));
runlock(sf); runlock(sf);
break; break;
default: default:
@ -983,7 +993,6 @@ recvproc(void *)
r = sreqrd[id]; r = sreqrd[id];
if(r != nil){ if(r != nil){
sreqrd[id] = nil; sreqrd[id] = nil;
r->reqid = -1;
rwakeup(&sreqidrend); rwakeup(&sreqidrend);
} }
qunlock(&sreqidlock); qunlock(&sreqidlock);
@ -992,13 +1001,14 @@ recvproc(void *)
continue; continue;
} }
if(r->closefid != nil){ if(r->closefid != nil){
putsfid(r->closefid);
putsreq(r); putsreq(r);
continue; continue;
} }
if(r->req == nil) if(r->req == nil)
sysfatal("recvproc: r->req == nil"); sysfatal("recvproc: r->req == nil");
sf = r->req->fid != nil ? r->req->fid->aux : nil; sf = r->req->fid->aux;
okresp = rxlen >= 9 && t == SSH_FXP_STATUS && GET4(rxpkt+5) == SSH_FX_OK; okresp = rxlen >= 9 && t == SSH_FXP_STATUS && GET4(rxpkt+5) == SSH_FX_OK;
switch(r->req->ifcall.type){ switch(r->req->ifcall.type){
case Tattach: case Tattach:
@ -1094,7 +1104,7 @@ recvproc(void *)
break; break;
} }
if(r->req->aux == nil){ if(r->req->aux == nil){
r->req->aux = (void *) -1; r->req->aux = (void*)-1;
submitreq(r->req); submitreq(r->req);
}else{ }else{
wlock(sf); wlock(sf);
@ -1199,7 +1209,6 @@ sshfsdestroyfid(Fid *f)
return; return;
if(sf->hand != nil){ if(sf->hand != nil){
sr = emalloc9p(sizeof(SReq)); sr = emalloc9p(sizeof(SReq));
sr->reqid = -1;
sr->closefid = sf; sr->closefid = sf;
submitsreq(sr); submitsreq(sr);
}else }else