ref: 12fa017f3fc2687d3afb166fd30d9b134abb1da5
parent: c45458b929e0f8e5eb26fd97c844977c893f6262
author: cinap_lenrek <[email protected]>
date: Sat Sep 21 12:36:40 EDT 2019
devproc: fix fishy locking in proctext(), check proc validity, static functions the locking in proctext() is wrong. we have to acquire Proc.seglock when reading segments from Proc.seg[] as segments do not have a private freelist and can therefore be reused for other data structures. once we have Proc.seglock acquired, check that the process pid is still valid so we wont accidentally read some other processes segments. (for both proctext() and procctlmemio()). this also should give better error message to distinguish the case when the process did segdetach() the segment in question before we could acquire Proc.seglock. declare private functions as static.
--- a/sys/src/9/port/devproc.c
+++ b/sys/src/9/port/devproc.c
@@ -154,11 +154,10 @@
#define PID(q) ((q).vers)
#define NOTEID(q) ((q).vers)
-void procctlreq(Proc*, char*, int);
-long procctlmemio(Chan*, Proc*, uintptr, void*, long, int);
-Chan* proctext(Chan*, Proc*);
-int procstopped(void*);
-ulong procpagecount(Proc *);
+static void procctlreq(Proc*, char*, int);
+static long procctlmemio(Chan*, Proc*, uintptr, void*, long, int);
+static Chan* proctext(Chan*, Proc*);
+static int procstopped(void*);
static Traceevent *tevents;
static Lock tlock;
@@ -382,10 +381,10 @@
case Qtext:
if(omode != OREAD)
error(Eperm);
- tc = proctext(c, p);
- tc->offset = 0;
qunlock(&p->debug);
poperror();
+ tc = proctext(c, p);
+ tc->offset = 0;
cclose(c);
return tc;
@@ -1297,7 +1296,7 @@
procwstat,
};
-Chan*
+static Chan*
proctext(Chan *c, Proc *p)
{
Chan *tc;
@@ -1304,43 +1303,32 @@
Image *i;
Segment *s;
- s = p->seg[TSEG];
- if(s == nil)
- error(Enonexist);
- if(p->state==Dead)
+ eqlock(&p->seglock);
+ if(waserror()){
+ qunlock(&p->seglock);
+ nexterror();
+ }
+ if(p->state == Dead || p->pid != PID(c->qid))
error(Eprocdied);
-
- i = s->image;
- if(i == nil)
- error(Eprocdied);
-
+ if((s = p->seg[TSEG]) == nil)
+ error(Enonexist);
+ if((i = s->image) == nil)
+ error(Enonexist);
lock(i);
- if(waserror()) {
+ tc = i->c;
+ if(i->notext || tc == nil || (tc->flag&COPEN) == 0 || tc->mode != OREAD){
unlock(i);
- nexterror();
+ error(Enonexist);
}
-
- tc = i->c;
- if(tc == nil)
- error(Eprocdied);
-
- if(incref(tc) == 1 || (tc->flag&COPEN) == 0 || tc->mode != OREAD) {
- cclose(tc);
- error(Eprocdied);
- }
-
- if(p->pid != PID(c->qid)) {
- cclose(tc);
- error(Eprocdied);
- }
-
+ incref(tc);
unlock(i);
+ qunlock(&p->seglock);
poperror();
return tc;
}
-void
+static void
procstopwait(Proc *p, int ctl)
{
char *state;
@@ -1375,7 +1363,7 @@
error(Eprocdied);
}
-void
+static void
procctlclosefiles(Proc *p, int all, int fd)
{
Fgrp *f;
@@ -1440,7 +1428,7 @@
return nil;
}
-void
+static void
procctlreq(Proc *p, char *va, int n)
{
Segment *s;
@@ -1636,13 +1624,13 @@
free(cb);
}
-int
+static int
procstopped(void *a)
{
return ((Proc*)a)->state == Stopped;
}
-long
+static long
procctlmemio(Chan *c, Proc *p, uintptr offset, void *a, long n, int read)
{
Segio *sio;
@@ -1657,11 +1645,9 @@
qunlock(&p->seglock);
nexterror();
}
- sio = c->aux;
- if(sio == nil){
- sio = smalloc(sizeof(Segio));
- c->aux = sio;
- }
+ if(p->state == Dead || p->pid != PID(c->qid))
+ error(Eprocdied);
+
for(i = 0; i < NSEG; i++) {
if(p->seg[i] == s)
break;
@@ -1668,6 +1654,7 @@
}
if(i == NSEG)
error(Egreg); /* segment gone */
+
eqlock(s);
if(waserror()){
qunlock(s);
@@ -1681,6 +1668,13 @@
incref(s); /* for us while we copy */
qunlock(s);
poperror();
+
+ sio = c->aux;
+ if(sio == nil){
+ sio = smalloc(sizeof(Segio));
+ c->aux = sio;
+ }
+
qunlock(&p->seglock);
poperror();