changelog shortlog tags branches files raw gz bz2 help

Mercurial > hg > plan9front / changeset: devproc: fix fishy locking in proctext(), check proc validity, static functions

changeset 7392: 52b0fd4ff543
parent 7391: 6555144c3581
child 7393: 6033964e95d2
author: cinap_lenrek@felloff.net
date: Sat, 21 Sep 2019 16:36:40 +0200
files: sys/src/9/port/devproc.c
description: 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.
     1.1--- a/sys/src/9/port/devproc.c
     1.2+++ b/sys/src/9/port/devproc.c
     1.3@@ -154,11 +154,10 @@ static char *sname[]={ "Text", "Data", "
     1.4 #define	PID(q)		((q).vers)
     1.5 #define	NOTEID(q)	((q).vers)
     1.6 
     1.7-void	procctlreq(Proc*, char*, int);
     1.8-long	procctlmemio(Chan*, Proc*, uintptr, void*, long, int);
     1.9-Chan*	proctext(Chan*, Proc*);
    1.10-int	procstopped(void*);
    1.11-ulong	procpagecount(Proc *);
    1.12+static void	procctlreq(Proc*, char*, int);
    1.13+static long	procctlmemio(Chan*, Proc*, uintptr, void*, long, int);
    1.14+static Chan*	proctext(Chan*, Proc*);
    1.15+static int	procstopped(void*);
    1.16 
    1.17 static Traceevent *tevents;
    1.18 static Lock tlock;
    1.19@@ -382,10 +381,10 @@ procopen(Chan *c, int omode0)
    1.20 	case Qtext:
    1.21 		if(omode != OREAD)
    1.22 			error(Eperm);
    1.23+		qunlock(&p->debug);
    1.24+		poperror();
    1.25 		tc = proctext(c, p);
    1.26 		tc->offset = 0;
    1.27-		qunlock(&p->debug);
    1.28-		poperror();
    1.29 		cclose(c);
    1.30 		return tc;
    1.31 
    1.32@@ -1297,50 +1296,39 @@ Dev procdevtab = {
    1.33 	procwstat,
    1.34 };
    1.35 
    1.36-Chan*
    1.37+static Chan*
    1.38 proctext(Chan *c, Proc *p)
    1.39 {
    1.40 	Chan *tc;
    1.41 	Image *i;
    1.42 	Segment *s;
    1.43 
    1.44-	s = p->seg[TSEG];
    1.45-	if(s == nil)
    1.46-		error(Enonexist);
    1.47-	if(p->state==Dead)
    1.48-		error(Eprocdied);
    1.49-
    1.50-	i = s->image;
    1.51-	if(i == nil)
    1.52-		error(Eprocdied);
    1.53-
    1.54-	lock(i);
    1.55-	if(waserror()) {
    1.56-		unlock(i);
    1.57+	eqlock(&p->seglock);
    1.58+	if(waserror()){
    1.59+		qunlock(&p->seglock);
    1.60 		nexterror();
    1.61 	}
    1.62-		
    1.63-	tc = i->c;
    1.64-	if(tc == nil)
    1.65-		error(Eprocdied);
    1.66-
    1.67-	if(incref(tc) == 1 || (tc->flag&COPEN) == 0 || tc->mode != OREAD) {
    1.68-		cclose(tc);
    1.69+	if(p->state == Dead || p->pid != PID(c->qid))
    1.70 		error(Eprocdied);
    1.71+	if((s = p->seg[TSEG]) == nil)
    1.72+		error(Enonexist);
    1.73+	if((i = s->image) == nil)
    1.74+		error(Enonexist);
    1.75+	lock(i);
    1.76+	tc = i->c;
    1.77+	if(i->notext || tc == nil || (tc->flag&COPEN) == 0 || tc->mode != OREAD){
    1.78+		unlock(i);
    1.79+		error(Enonexist);
    1.80 	}
    1.81-
    1.82-	if(p->pid != PID(c->qid)) {
    1.83-		cclose(tc);
    1.84-		error(Eprocdied);
    1.85-	}
    1.86-
    1.87+	incref(tc);
    1.88 	unlock(i);
    1.89+	qunlock(&p->seglock);
    1.90 	poperror();
    1.91 
    1.92 	return tc;
    1.93 }
    1.94 
    1.95-void
    1.96+static void
    1.97 procstopwait(Proc *p, int ctl)
    1.98 {
    1.99 	char *state;
   1.100@@ -1375,7 +1363,7 @@ procstopwait(Proc *p, int ctl)
   1.101 		error(Eprocdied);
   1.102 }
   1.103 
   1.104-void
   1.105+static void
   1.106 procctlclosefiles(Proc *p, int all, int fd)
   1.107 {
   1.108 	Fgrp *f;
   1.109@@ -1440,7 +1428,7 @@ parsetime(vlong *rt, char *s)
   1.110 	return nil;
   1.111 }
   1.112 
   1.113-void
   1.114+static void
   1.115 procctlreq(Proc *p, char *va, int n)
   1.116 {
   1.117 	Segment *s;
   1.118@@ -1636,13 +1624,13 @@ procctlreq(Proc *p, char *va, int n)
   1.119 	free(cb);
   1.120 }
   1.121 
   1.122-int
   1.123+static int
   1.124 procstopped(void *a)
   1.125 {
   1.126 	return ((Proc*)a)->state == Stopped;
   1.127 }
   1.128 
   1.129-long
   1.130+static long
   1.131 procctlmemio(Chan *c, Proc *p, uintptr offset, void *a, long n, int read)
   1.132 {
   1.133 	Segio *sio;
   1.134@@ -1657,17 +1645,16 @@ procctlmemio(Chan *c, Proc *p, uintptr o
   1.135 		qunlock(&p->seglock);
   1.136 		nexterror();
   1.137 	}
   1.138-	sio = c->aux;
   1.139-	if(sio == nil){
   1.140-		sio = smalloc(sizeof(Segio));
   1.141-		c->aux = sio;
   1.142-	}
   1.143+	if(p->state == Dead || p->pid != PID(c->qid))
   1.144+		error(Eprocdied);
   1.145+
   1.146 	for(i = 0; i < NSEG; i++) {
   1.147 		if(p->seg[i] == s)
   1.148 			break;
   1.149 	}
   1.150 	if(i == NSEG)
   1.151 		error(Egreg);	/* segment gone */
   1.152+
   1.153 	eqlock(s);
   1.154 	if(waserror()){
   1.155 		qunlock(s);
   1.156@@ -1681,6 +1668,13 @@ procctlmemio(Chan *c, Proc *p, uintptr o
   1.157 	incref(s);		/* for us while we copy */
   1.158 	qunlock(s);
   1.159 	poperror();
   1.160+
   1.161+	sio = c->aux;
   1.162+	if(sio == nil){
   1.163+		sio = smalloc(sizeof(Segio));
   1.164+		c->aux = sio;
   1.165+	}
   1.166+
   1.167 	qunlock(&p->seglock);
   1.168 	poperror();
   1.169