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

ratrace: fix race conditions and range check

Browse source 
the syscallno check in syscallfmt() was wrong. the unsigned
syscall number was cast to an signed integer. so negative
values would pass the check provoking bad memory access from
kernel. the check also has an off by one. one has to check
syscallno >= nsyscalls instead of syscallno > nsyscalls.

access to the p->syscalltrace string was not protected
from modification in devproc. you could awake the process
and cause it to free the string giving an opportunity for
the kernel to access bad memory. or someone could kill the
process (pexit would just free it).

now the string is protected by the usual p->debug qlock. we
also keep the string arround until it is overwritten again
or the process exists. this has the nice side effect that
one can inspect it after the process crashed.

another problem was that our validaddr() would error() instead
of pexiting the current process. the code was changed to only
access up->s.args after it was validated and copied instead of
accessing the user stack directly. this also prevents a sneaky
multithreaded process from chaning the arguments under us.

in case our validaddr() errors, we cannot assume valid user
stack after the waserror() if block. use up->s.arg[0] for the
noted() call to avoid bad access.
This commit is contained in:
cinap_lenrek 2012-11-23 20:27:09 +01:00
parent 2f416353df
commit 4b4070a8b9
7 changed files with 48 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
sys/src/9/pc/fns.h
View file

@ -161,8 +161,6 @@ void screeninit(void);
void (*screenputs)(char*, int);
void* sigsearch(char*);
void syncclock(void);
void syscallfmt(int syscallno, ulong pc, va_list list);
void sysretfmt(int syscallno, va_list list, long ret, uvlong start, uvlong stop);
void* tmpmap(Page*);
void tmpunmap(void*);
void touser(void*);

48
sys/src/9/pc/trap.c
View file

@ -734,26 +734,6 @@ syscall(Ureg* ureg)
scallnr = ureg->ax;
up->scallnr = scallnr;
if(up->procctl == Proc_tracesyscall){
/*
* Redundant validaddr. Do we care?
* Tracing syscalls is not exactly a fast path...
* Beware, validaddr currently does a pexit rather
* than an error if there's a problem; that might
* change in the future.
*/
if(sp < (USTKTOP-BY2PG) || sp > (USTKTOP-sizeof(Sargs)-BY2WD))
validaddr(sp, sizeof(Sargs)+BY2WD, 0);
syscallfmt(scallnr, ureg->pc, (va_list)(sp+BY2WD));
up->procctl = Proc_stopme;
procctl(up);
if(up->syscalltrace)
free(up->syscalltrace);
up->syscalltrace = nil;
startns = todget(nil);
}
if(scallnr == RFORK && up->fpstate == FPactive){
fpsave(&up->fpsave);
up->fpstate = FPinactive;
@ -763,17 +743,26 @@ syscall(Ureg* ureg)
up->nerrlab = 0;
ret = -1;
if(!waserror()){
if(sp<(USTKTOP-BY2PG) || sp>(USTKTOP-sizeof(Sargs)-BY2WD))
validaddr(sp, sizeof(Sargs)+BY2WD, 0);
up->s = *((Sargs*)(sp+BY2WD));
if(up->procctl == Proc_tracesyscall){
syscallfmt(scallnr, ureg->pc, (va_list)up->s.args);
s = splhi();
up->procctl = Proc_stopme;
procctl(up);
splx(s);
startns = todget(nil);
}
if(scallnr >= nsyscall || systab[scallnr] == 0){
pprint("bad sys call number %lud pc %lux\n",
scallnr, ureg->pc);
postnote(up, 1, "sys: bad sys call", NDebug);
error(Ebadarg);
}
if(sp<(USTKTOP-BY2PG) || sp>(USTKTOP-sizeof(Sargs)-BY2WD))
validaddr(sp, sizeof(Sargs)+BY2WD, 0);
up->s = *((Sargs*)(sp+BY2WD));
up->psstate = sysctab[scallnr];
ret = systab[scallnr](up->s.args);
@ -804,21 +793,18 @@ syscall(Ureg* ureg)
if(up->procctl == Proc_tracesyscall){
stopns = todget(nil);
up->procctl = Proc_stopme;
sysretfmt(scallnr, (va_list)(sp+BY2WD), ret, startns, stopns);
sysretfmt(scallnr, (va_list)up->s.args, ret, startns, stopns);
s = splhi();
up->procctl = Proc_stopme;
procctl(up);
splx(s);
if(up->syscalltrace)
free(up->syscalltrace);
up->syscalltrace = nil;
}
up->insyscall = 0;
up->psstate = 0;
if(scallnr == NOTED)
noted(ureg, *(ulong*)(sp+BY2WD));
noted(ureg, up->s.args[0]);
if(scallnr!=RFORK && (up->procctl || up->nnote)){
splhi();

9
sys/src/9/port/devproc.c
View file

@ -737,9 +737,12 @@ procread(Chan *c, void *va, long n, vlong off)
memmove(a, &up->genbuf[offset], n);
return n;
case Qsyscall:
if(!p->syscalltrace)
return 0;
n = readstr(offset, a, n, p->syscalltrace);
eqlock(&p->debug);
if(p->syscalltrace)
n = readstr(offset, a, n, p->syscalltrace);
else
n = 0;
qunlock(&p->debug);
return n;
case Qmem:

2
sys/src/9/port/portfns.h
View file

@ -326,6 +326,8 @@ void shrrenameuser(char*, char*);
int swapcount(ulong);
int swapfull(void);
void swapinit(void);
void syscallfmt(ulong syscallno, ulong pc, va_list list);
void sysretfmt(ulong syscallno, va_list list, long ret, uvlong start, uvlong stop);
void timeradd(Timer*);
void timerdel(Timer*);
void timersinit(void);

11
sys/src/9/port/proc.c
View file

@ -622,10 +622,7 @@ newproc(void)
p->pdbg = 0;
p->fpstate = FPinit;
p->kp = 0;
if(up && up->procctl == Proc_tracesyscall)
p->procctl = Proc_tracesyscall;
else
p->procctl = 0;
p->procctl = 0;
p->syscalltrace = 0;
p->notepending = 0;
p->ureg = 0;
@ -1078,8 +1075,6 @@ pexit(char *exitstr, int freemem)
Chan *dot;
void (*pt)(Proc*, int, vlong);
if(up->syscalltrace)
free(up->syscalltrace);
up->alarm = 0;
if (up->tt)
timerdel(up);
@ -1194,6 +1189,10 @@ pexit(char *exitstr, int freemem)
wakeup(&up->pdbg->sleep);
up->pdbg = 0;
}
if(up->syscalltrace) {
free(up->syscalltrace);
up->syscalltrace = 0;
}
qunlock(&up->debug);
/* Sched must not loop for these locks */

28
sys/src/9/port/syscallfmt.c
View file

@ -22,6 +22,7 @@ fmtrwdata(Fmt* f, char* a, int n, char* suffix)
}
validaddr((ulong)a, n, 0);
t = smalloc(n+1);
t[n] = 0;
for(i = 0; i < n; i++)
if(a[i] > 0x20 && a[i] < 0x7f) /* printable ascii? */
t[i] = a[i];
@ -52,7 +53,7 @@ fmtuserstring(Fmt* f, char* a, char* suffix)
}
void
syscallfmt(int syscallno, ulong pc, va_list list)
syscallfmt(ulong syscallno, ulong pc, va_list list)
{
long l;
Fmt fmt;
@ -65,16 +66,13 @@ syscallfmt(int syscallno, ulong pc, va_list list)
fmtstrinit(&fmt);
fmtprint(&fmt, "%uld %s ", up->pid, up->text);
if(syscallno > nsyscall)
fmtprint(&fmt, " %d ", syscallno);
if(syscallno >= nsyscall)
fmtprint(&fmt, " %uld ", syscallno);
else
fmtprint(&fmt, "%s ", sysctab[syscallno]?
sysctab[syscallno]: "huh?");
fmtprint(&fmt, "%ulx ", pc);
if(up->syscalltrace != nil)
free(up->syscalltrace);
switch(syscallno){
case SYSR1:
p = va_arg(list, uintptr);
@ -301,11 +299,15 @@ syscallfmt(int syscallno, ulong pc, va_list list)
break;
}
up->syscalltrace = fmtstrflush(&fmt);
a = fmtstrflush(&fmt);
qlock(&up->debug);
free(up->syscalltrace);
up->syscalltrace = a;
qunlock(&up->debug);
}
void
sysretfmt(int syscallno, va_list list, long ret, uvlong start, uvlong stop)
sysretfmt(ulong syscallno, va_list list, long ret, uvlong start, uvlong stop)
{
long l;
void* v;
@ -316,9 +318,6 @@ sysretfmt(int syscallno, va_list list, long ret, uvlong start, uvlong stop)
fmtstrinit(&fmt);
if(up->syscalltrace)
free(up->syscalltrace);
errstr = "\"\"";
switch(syscallno){
default:
@ -402,5 +401,10 @@ sysretfmt(int syscallno, va_list list, long ret, uvlong start, uvlong stop)
break;
}
fmtprint(&fmt, " %s %#llud %#llud\n", errstr, start, stop);
up->syscalltrace = fmtstrflush(&fmt);
a = fmtstrflush(&fmt);
qlock(&up->debug);
free(up->syscalltrace);
up->syscalltrace = a;
qunlock(&up->debug);
}

2
sys/src/9/port/sysproc.c
View file

@ -162,6 +162,8 @@ sysrfork(ulong *arg)
}
p->hang = up->hang;
p->procmode = up->procmode;
if(up->procctl == Proc_tracesyscall)
p->procctl = Proc_tracesyscall;
/* Craft a return frame which will cause the child to pop out of
* the scheduler in user mode with the return register zero