devproc: fix proccrlmemio bugs

dont kill the calling process when demand load fails if fixfault()
is called from devproc. this happens when you delete the binary
of a running process and try to debug the process accessing uncached
pages thru /proc/$pid/mem file.

fixes to procctlmemio():

- fix missed unlock as txt2data() can error
- make sure the segment isnt freed by taking a reference (under p->seglock)
- access the page with segment locked (see comment)
- get rid of the segment stealer lock

other stuff:

- move txt2data() and data2txt() to segment.c
- add procpagecount() function
- make return type mcounseg() to ulong

7 files changed, 266 insertions, 223 deletions

diff --git a/sys/src/9/port/devproc.c b/sys/src/9/port/devproc.c
index 346da3c3a..c0ed27c92 100644
--- a/sys/src/9/port/devproc.c
+++ b/sys/src/9/port/devproc.c
@@ -154,9 +154,9 @@ static char *sname[]={ "Text", "Data", "Bss", "Stack", "Shared", "Phys", };
 void	procctlreq(Proc*, char*, int);
 int	procctlmemio(Proc*, uintptr, int, void*, int);
 Chan*	proctext(Chan*, Proc*);
-Segment* txt2data(Proc*, Segment*);
 int	procstopped(void*);
 void	mntscan(Mntwalk*, Proc*);
+ulong	procpagecount(Proc *);
 
 static Traceevent *tevents;
 static Lock tlock;
@@ -896,23 +896,7 @@ procread(Chan *c, void *va, long n, vlong off)
 			readnum(0, statbuf+j+NUMSIZE*i, NUMSIZE, l, NUMSIZE);
 		}
 
-		l = 0;
-		eqlock(&p->seglock);
-		if(waserror()){
-			qunlock(&p->seglock);
-			nexterror();
-		}
-		for(i=0; i<NSEG; i++){
-			if(s = p->seg[i]){
-				eqlock(s);
-				l += mcountseg(s);
-				qunlock(s);
-			}
-		}
-		poperror();
-		qunlock(&p->seglock);
-
-		readnum(0, statbuf+j+NUMSIZE*6, NUMSIZE, l*BY2PG/1024, NUMSIZE);
+		readnum(0, statbuf+j+NUMSIZE*6, NUMSIZE, procpagecount(p)*BY2PG/1024, NUMSIZE);
 		readnum(0, statbuf+j+NUMSIZE*7, NUMSIZE, p->basepri, NUMSIZE);
 		readnum(0, statbuf+j+NUMSIZE*8, NUMSIZE, p->priority, NUMSIZE);
 		memmove(a, statbuf+offset, n);
@@ -1540,6 +1524,32 @@ procstopped(void *a)
 	return p->state == Stopped;
 }
 
+ulong
+procpagecount(Proc *p)
+{
+	Segment *s;
+	ulong pages;
+	int i;
+
+	eqlock(&p->seglock);
+	if(waserror()){
+		qunlock(&p->seglock);
+		nexterror();
+	}
+	pages = 0;
+	for(i=0; i<NSEG; i++){
+		if((s = p->seg[i]) != nil){
+			eqlock(s);
+			pages += mcountseg(s);
+			qunlock(s);
+		}
+	}
+	qunlock(&p->seglock);
+	poperror();
+
+	return pages;
+}
+
 int
 procctlmemio(Proc *p, uintptr offset, int n, void *va, int read)
 {
@@ -1547,110 +1557,118 @@ procctlmemio(Proc *p, uintptr offset, int n, void *va, int read)
 	Pte *pte;
 	Page *pg;
 	Segment *s;
-	uintptr soff, l;
-	char *a = va, *b;
+	uintptr soff;
+	char *a, *b;
+	int i, l;
+
+	/* Only one page at a time */
+	l = BY2PG - (offset&(BY2PG-1));
+	if(n > l)
+		n = l;
+
+	/*
+	 * Make temporary copy to avoid fault while we have
+	 * segment locked as we would deadlock when trying
+	 * to read the calling procs memory.
+	 */
+	a = malloc(n);
+	if(a == nil)
+		error(Enomem);
+	if(waserror()) {
+		free(a);
+		nexterror();
+	}
+
+	if(!read)
+		memmove(a, va, n);	/* can fault */
 
 	for(;;) {
-		s = seg(p, offset, 1);
-		if(s == 0)
+		s = seg(p, offset, 0);
+		if(s == nil)
 			error(Ebadarg);
 
-		if(offset+n >= s->top)
-			n = s->top-offset;
+		eqlock(&p->seglock);
+		if(waserror()) {
+			qunlock(&p->seglock);
+			nexterror();
+		}
 
-		if(!read && (s->type&SG_TYPE) == SG_TEXT)
-			s = txt2data(p, s);
+		for(i = 0; i < NSEG; i++) {
+			if(p->seg[i] == s)
+				break;
+		}
+		if(i == NSEG)
+			error(Egreg);	/* segment gone */
 
-		s->steal++;
-		soff = offset-s->base;
-		if(waserror()) {
-			s->steal--;
+		eqlock(s);
+		if(waserror()){
+			qunlock(s);
 			nexterror();
 		}
-		if(fixfault(s, offset, read, 0) == 0)
-			break;
+		if(!read && (s->type&SG_TYPE) == SG_TEXT) {
+			s = txt2data(s);
+			p->seg[i] = s;
+		}
+		incref(s);
+		qunlock(&p->seglock);
+		poperror();
 		poperror();
-		s->steal--;
+		/* segment s still locked, fixfault() unlocks */
+		if(!waserror()){
+			if(fixfault(s, offset, read, 0) == 0)
+				break;
+			poperror();
+		}
+		putseg(s);
 	}
-	poperror();
+
+	/*
+	 * Only access the page while segment is locked
+	 * as the proc could segfree or relocate the pte
+	 * concurrently.
+	 */ 
+	eqlock(s);
+	if(waserror()){
+		qunlock(s);
+		nexterror();
+	}
+	if(offset+n >= s->top)
+		n = s->top-offset;
+	soff = offset-s->base;
 	pte = s->map[soff/PTEMAPMEM];
-	if(pte == 0)
-		panic("procctlmemio");
+	if(pte == nil)
+		error(Egreg);	/* page gone, should retry? */
 	pg = pte->pages[(soff&(PTEMAPMEM-1))/BY2PG];
 	if(pagedout(pg))
-		panic("procctlmemio1");
-
-	l = BY2PG - (offset&(BY2PG-1));
-	if(n > l)
-		n = l;
+		error(Egreg);	/* page gone, should retry?  */
 
+	/* Map and copy the page */
 	k = kmap(pg);
-	if(waserror()) {
-		s->steal--;
-		kunmap(k);
-		nexterror();
-	}
 	b = (char*)VA(k);
 	b += offset&(BY2PG-1);
-	if(read == 1)
-		memmove(a, b, n);	/* This can fault */
+	if(read)
+		memmove(a, b, n);
 	else
 		memmove(b, a, n);
 	kunmap(k);
-	poperror();
 
 	/* Ensure the process sees text page changes */
 	if(s->flushme)
 		memset(pg->cachectl, PG_TXTFLUSH, sizeof(pg->cachectl));
 
-	s->steal--;
-
-	if(read == 0)
+	if(!read)
 		p->newtlb = 1;
 
-	return n;
-}
-
-Segment*
-txt2data(Proc *p, Segment *s)
-{
-	int i;
-	Segment *ps;
-
-	ps = newseg(SG_DATA, s->base, s->size);
-	ps->image = s->image;
-	incref(ps->image);
-	ps->fstart = s->fstart;
-	ps->flen = s->flen;
-	ps->flushme = 1;
-
-	qlock(&p->seglock);
-	for(i = 0; i < NSEG; i++)
-		if(p->seg[i] == s)
-			break;
-	if(i == NSEG)
-		panic("segment gone");
-
 	qunlock(s);
+	poperror();
 	putseg(s);
-	qlock(ps);
-	p->seg[i] = ps;
-	qunlock(&p->seglock);
-
-	return ps;
-}
+	poperror();
 
-Segment*
-data2txt(Segment *s)
-{
-	Segment *ps;
+	if(read)
+		memmove(va, a, n);	/* can fault */
 
-	ps = newseg(SG_TEXT, s->base, s->size);
-	ps->image = s->image;
-	incref(ps->image);
-	ps->fstart = s->fstart;
-	ps->flen = s->flen;
-	ps->flushme = 1;
+	free(a);
+	poperror();
 
-	return ps;
+	return n;
 }
diff --git a/sys/src/9/port/fault.c b/sys/src/9/port/fault.c
index 54824e8ba..0fd6b2e97 100644
--- a/sys/src/9/port/fault.c
+++ b/sys/src/9/port/fault.c
@@ -58,7 +58,7 @@ fault(uintptr addr, int read)
 }
 
 static void
-faulterror(char *s, Chan *c, int freemem)
+faulterror(char *s, Chan *c, int isfatal)
 {
 	char buf[ERRMAX];
 
@@ -67,123 +67,15 @@ faulterror(char *s, Chan *c, int freemem)
 		s = buf;
 	}
 	if(up->nerrlab) {
-		postnote(up, 1, s, NDebug);
+		if(isfatal)
+			postnote(up, 1, s, NDebug);
 		error(s);
 	}
-	pexit(s, freemem);
+	pexit(s, 1);
 }
 
-void	(*checkaddr)(uintptr, Segment *, Page *);
-uintptr	addr2check;
-
-int
-fixfault(Segment *s, uintptr addr, int read, int doputmmu)
-{
-	int type;
-	Pte **p, *etp;
-	uintptr soff, mmuphys=0;
-	Page **pg, *old, *new;
-	Page *(*fn)(Segment*, uintptr);
-
-	addr &= ~(BY2PG-1);
-	soff = addr-s->base;
-	p = &s->map[soff/PTEMAPMEM];
-	if(*p == nil)
-		*p = ptealloc();
-
-	etp = *p;
-	pg = &etp->pages[(soff&(PTEMAPMEM-1))/BY2PG];
-	type = s->type&SG_TYPE;
-
-	if(pg < etp->first)
-		etp->first = pg;
-	if(pg > etp->last)
-		etp->last = pg;
-
-	switch(type) {
-	default:
-		panic("fault");
-		break;
-
-	case SG_TEXT: 			/* Demand load */
-		if(pagedout(*pg))
-			pio(s, addr, soff, pg);
-
-		mmuphys = PPN((*pg)->pa) | PTERONLY|PTEVALID;
-		(*pg)->modref = PG_REF;
-		break;
-
-	case SG_BSS:
-	case SG_SHARED:			/* Zero fill on demand */
-	case SG_STACK:
-		if(*pg == nil) {
-			new = newpage(1, &s, addr);
-			if(s == nil)
-				return -1;
-			*pg = new;
-		}
-		goto common;
-
-	case SG_DATA:
-	common:			/* Demand load/pagein/copy on write */
-		if(pagedout(*pg))
-			pio(s, addr, soff, pg);
-
-		/*
-		 *  It's only possible to copy on write if
-		 *  we're the only user of the segment.
-		 */
-		if(read && conf.copymode == 0 && s->ref == 1) {
-			mmuphys = PPN((*pg)->pa)|PTERONLY|PTEVALID;
-			(*pg)->modref |= PG_REF;
-			break;
-		}
-
-		old = *pg;
-		if(old->image == &swapimage && (old->ref + swapcount(old->daddr)) == 1)
-			uncachepage(old);
-		if(old->ref > 1 || old->image != nil) {
-			new = newpage(0, &s, addr);
-			if(s == nil)
-				return -1;
-			*pg = new;
-			copypage(old, *pg);
-			putpage(old);
-		}
-		mmuphys = PPN((*pg)->pa) | PTEWRITE | PTEVALID;
-		(*pg)->modref = PG_MOD|PG_REF;
-		break;
-
-	case SG_PHYSICAL:
-		if(*pg == nil) {
-			fn = s->pseg->pgalloc;
-			if(fn)
-				*pg = (*fn)(s, addr);
-			else {
-				new = smalloc(sizeof(Page));
-				new->va = addr;
-				new->pa = s->pseg->pa+(addr-s->base);
-				new->ref = 1;
-				*pg = new;
-			}
-		}
-
-		if (checkaddr && addr == addr2check)
-			(*checkaddr)(addr, s, *pg);
-		mmuphys = PPN((*pg)->pa) |PTEWRITE|PTEUNCACHED|PTEVALID;
-		(*pg)->modref = PG_MOD|PG_REF;
-		break;
-	}
-	qunlock(s);
-
-	if(doputmmu)
-		putmmu(addr, mmuphys, *pg);
-
-	return 0;
-}
-
-void
-pio(Segment *s, uintptr addr, uintptr soff, Page **p)
+static void
+pio(Segment *s, uintptr addr, uintptr soff, Page **p, int isfatal)
 {
 	Page *new;
 	KMap *k;
@@ -227,22 +119,21 @@ retry:
 	new = newpage(0, 0, addr);
 	k = kmap(new);
 	kaddr = (char*)VA(k);
-
 	while(waserror()) {
-		if(strcmp(up->errstr, Eintr) == 0)
+		if(isfatal && strcmp(up->errstr, Eintr) == 0)
 			continue;
 		kunmap(k);
 		putpage(new);
-		faulterror(Eioload, c, 0);
+		faulterror(Eioload, c, isfatal);
 	}
 	n = devtab[c->type]->read(c, kaddr, ask, daddr);
 	if(n != ask)
-		error(Eioload);
+		error(Eshort);
 	if(ask < BY2PG)
 		memset(kaddr+ask, 0, BY2PG-ask);
-
 	poperror();
 	kunmap(k);
+
 	qlock(s);
 	if(loadrec == nil) {	/* This is demand load */
 		/*
@@ -299,6 +190,115 @@ done:
 		memset((*p)->cachectl, PG_TXTFLUSH, sizeof((*p)->cachectl));
 }
 
+void	(*checkaddr)(uintptr, Segment *, Page *);
+uintptr	addr2check;
+
+int
+fixfault(Segment *s, uintptr addr, int read, int doputmmu)
+{
+	int type;
+	Pte **p, *etp;
+	uintptr soff, mmuphys=0;
+	Page **pg, *old, *new;
+	Page *(*fn)(Segment*, uintptr);
+
+	addr &= ~(BY2PG-1);
+	soff = addr-s->base;
+	p = &s->map[soff/PTEMAPMEM];
+	if(*p == nil)
+		*p = ptealloc();
+
+	etp = *p;
+	pg = &etp->pages[(soff&(PTEMAPMEM-1))/BY2PG];
+	type = s->type&SG_TYPE;
+
+	if(pg < etp->first)
+		etp->first = pg;
+	if(pg > etp->last)
+		etp->last = pg;
+
+	switch(type) {
+	default:
+		panic("fault");
+		break;
+
+	case SG_TEXT: 			/* Demand load */
+		if(pagedout(*pg))
+			pio(s, addr, soff, pg, doputmmu);
+
+		mmuphys = PPN((*pg)->pa) | PTERONLY|PTEVALID;
+		(*pg)->modref = PG_REF;
+		break;
+
+	case SG_BSS:
+	case SG_SHARED:			/* Zero fill on demand */
+	case SG_STACK:
+		if(*pg == nil) {
+			new = newpage(1, &s, addr);
+			if(s == nil)
+				return -1;
+			*pg = new;
+		}
+		goto common;
+
+	case SG_DATA:
+	common:			/* Demand load/pagein/copy on write */
+		if(pagedout(*pg))
+			pio(s, addr, soff, pg, doputmmu);
+
+		/*
+		 *  It's only possible to copy on write if
+		 *  we're the only user of the segment.
+		 */
+		if(read && conf.copymode == 0 && s->ref == 1) {
+			mmuphys = PPN((*pg)->pa)|PTERONLY|PTEVALID;
+			(*pg)->modref |= PG_REF;
+			break;
+		}
+
+		old = *pg;
+		if(old->image == &swapimage && (old->ref + swapcount(old->daddr)) == 1)
+			uncachepage(old);
+		if(old->ref > 1 || old->image != nil) {
+			new = newpage(0, &s, addr);
+			if(s == nil)
+				return -1;
+			*pg = new;
+			copypage(old, *pg);
+			putpage(old);
+		}
+		mmuphys = PPN((*pg)->pa) | PTEWRITE | PTEVALID;
+		(*pg)->modref = PG_MOD|PG_REF;
+		break;
+
+	case SG_PHYSICAL:
+		if(*pg == nil) {
+			fn = s->pseg->pgalloc;
+			if(fn)
+				*pg = (*fn)(s, addr);
+			else {
+				new = smalloc(sizeof(Page));
+				new->va = addr;
+				new->pa = s->pseg->pa+(addr-s->base);
+				new->ref = 1;
+				*pg = new;
+			}
+		}
+
+		if (checkaddr && addr == addr2check)
+			(*checkaddr)(addr, s, *pg);
+		mmuphys = PPN((*pg)->pa) |PTEWRITE|PTEUNCACHED|PTEVALID;
+		(*pg)->modref = PG_MOD|PG_REF;
+		break;
+	}
+	qunlock(s);
+
+	if(doputmmu)
+		putmmu(addr, mmuphys, *pg);
+
+	return 0;
+}
+
 /*
  * Called only in a system call
  */
diff --git a/sys/src/9/port/portdat.h b/sys/src/9/port/portdat.h
index 58c82c6a8..0afe70ddf 100644
--- a/sys/src/9/port/portdat.h
+++ b/sys/src/9/port/portdat.h
@@ -389,8 +389,7 @@ struct Segment
 {
 	Ref;
 	QLock;
-	ushort	steal;		/* Page stealer lock */
-	ushort	type;		/* segment type */
+	int	type;		/* segment type */
 	uintptr	base;		/* virtual base */
 	uintptr	top;		/* virtual top */
 	ulong	size;		/* size in pages */
diff --git a/sys/src/9/port/portfns.h b/sys/src/9/port/portfns.h
index 626ec3eab..fae87632e 100644
--- a/sys/src/9/port/portfns.h
+++ b/sys/src/9/port/portfns.h
@@ -166,7 +166,7 @@ void*		malloc(ulong);
 void*		mallocalign(ulong, ulong, long, ulong);
 void		mallocsummary(void);
 Block*		mem2bl(uchar*, int);
-int		mcountseg(Segment*);
+ulong		mcountseg(Segment*);
 void		mfreeseg(Segment*, uintptr, int);
 void		microdelay(int);
 uvlong		mk64fract(uvlong, uvlong);
@@ -214,7 +214,6 @@ void		pexit(char*, int);
 void		pgrpcpy(Pgrp*, Pgrp*);
 void		pgrpnote(ulong, char*, long, int);
 int		pidalloc(Proc*);
-void		pio(Segment *, uintptr, uintptr, Page **);
 #define		poperror()		up->nerrlab--
 void		portcountpagerefs(ulong*, int);
 int		postnote(Proc*, int, char*, int);
@@ -380,6 +379,7 @@ Segment*	data2txt(Segment*);
 Segment*	dupseg(Segment**, int, int);
 Segment*	newseg(int, uintptr, ulong);
 Segment*	seg(Proc*, uintptr, int);
+Segment*	txt2data(Segment*);
 void		hnputv(void*, uvlong);
 void		hnputl(void*, uint);
 void		hnputs(void*, ushort);
diff --git a/sys/src/9/port/proc.c b/sys/src/9/port/proc.c
index bd152c4c8..58fbc7a8b 100644
--- a/sys/src/9/port/proc.c
+++ b/sys/src/9/port/proc.c
@@ -1510,7 +1510,7 @@ killbig(char *why)
 			s = p->seg[i];
 			if(s == nil || !canqlock(s))
 				continue;
-			l += (ulong)mcountseg(s);
+			l += mcountseg(s);
 			qunlock(s);
 		}
 		qunlock(&p->seglock);
diff --git a/sys/src/9/port/segment.c b/sys/src/9/port/segment.c
index 75098a590..17efff622 100644
--- a/sys/src/9/port/segment.c
+++ b/sys/src/9/port/segment.c
@@ -452,10 +452,11 @@ ibrk(uintptr addr, int seg)
 /*
  *  called with s locked
  */
-int
+ulong
 mcountseg(Segment *s)
 {
-	int i, j, pages;
+	ulong pages;
+	int i, j;
 	Page *pg;
 
 	pages = 0;
@@ -736,3 +737,33 @@ segclock(uintptr pc)
 	}
 }
 
+Segment*
+txt2data(Segment *s)
+{
+	Segment *ps;
+
+	ps = newseg(SG_DATA, s->base, s->size);
+	ps->image = s->image;
+	incref(ps->image);
+	ps->fstart = s->fstart;
+	ps->flen = s->flen;
+	ps->flushme = 1;
+	qunlock(s);
+	putseg(s);
+	qlock(ps);
+	return ps;
+}
+
+Segment*
+data2txt(Segment *s)
+{
+	Segment *ps;
+
+	ps = newseg(SG_TEXT, s->base, s->size);
+	ps->image = s->image;
+	incref(ps->image);
+	ps->fstart = s->fstart;
+	ps->flen = s->flen;
+	ps->flushme = 1;
+	return ps;
+}
diff --git a/sys/src/9/port/swap.c b/sys/src/9/port/swap.c
index ff2694f25..355c37f5f 100644
--- a/sys/src/9/port/swap.c
+++ b/sys/src/9/port/swap.c
@@ -222,11 +222,6 @@ pageout(Proc *p, Segment *s)
 	if(!canqlock(s))	/* We cannot afford to wait, we will surely deadlock */
 		return;
 
-	if(s->steal) {		/* Protected by /dev/proc */
-		qunlock(s);
-		return;
-	}
-
 	if(!canflush(p, s)) {	/* Able to invalidate all tlbs with references */
 		qunlock(s);
 		putseg(s);