kernel: fix noteid change race condition from devproc while forking (thanks joe7)
devproc allows changing the noteid of another process which opens a race condition in sysrfork(), when deciding to inherit the noteid of "up" to the child and calling pidalloc() later to take the reference, the noteid could have been changed and the childs noteid could have been freed already in the process. this bug can only happen when one writes the /proc/n/noteid file of a another process than your own that is in the process of forking. the noteid changing functionality of devproc seems questinable and seems to be only used by ape's setpgrid() implementation.
This commit is contained in:
parent
641bd4512f
commit
9126ee3eea
2 changed files with 27 additions and 14 deletions
|
@ -1820,6 +1820,19 @@ procindex(ulong pid)
|
||||||
return -1;
|
return -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* for the following functions:
|
||||||
|
* setnoteid(), pidalloc() and pidfree()
|
||||||
|
*
|
||||||
|
* They need to be called with p->debug held
|
||||||
|
* to prevent devproc's changenoteid() from
|
||||||
|
* changing the noteid under us and make the
|
||||||
|
* update atomic.
|
||||||
|
*/
|
||||||
|
|
||||||
|
/*
|
||||||
|
* change the noteid of a process
|
||||||
|
*/
|
||||||
ulong
|
ulong
|
||||||
setnoteid(Proc *p, ulong noteid)
|
setnoteid(Proc *p, ulong noteid)
|
||||||
{
|
{
|
||||||
|
@ -1840,15 +1853,6 @@ setnoteid(Proc *p, ulong noteid)
|
||||||
return p->noteid = i->pid;
|
return p->noteid = i->pid;
|
||||||
}
|
}
|
||||||
|
|
||||||
static ulong
|
|
||||||
setparentpid(Proc *p, Proc *pp)
|
|
||||||
{
|
|
||||||
Pid *i;
|
|
||||||
|
|
||||||
i = pidadd(pp->pid);
|
|
||||||
return p->parentpid = i->pid;
|
|
||||||
}
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* allocate pid, noteid and parentpid to a process
|
* allocate pid, noteid and parentpid to a process
|
||||||
*/
|
*/
|
||||||
|
@ -1858,8 +1862,11 @@ pidalloc(Proc *p)
|
||||||
Pid *i;
|
Pid *i;
|
||||||
|
|
||||||
/* skip for the boot process */
|
/* skip for the boot process */
|
||||||
if(up != nil)
|
if(up != nil){
|
||||||
setparentpid(p, up);
|
i = pidadd(up->pid);
|
||||||
|
p->parentpid = i->pid;
|
||||||
|
} else
|
||||||
|
p->parentpid = 0;
|
||||||
|
|
||||||
i = pidadd(0);
|
i = pidadd(0);
|
||||||
i->procindex = (int)(p - procalloc.arena);
|
i->procindex = (int)(p - procalloc.arena);
|
||||||
|
|
|
@ -79,14 +79,18 @@ sysrfork(va_list list)
|
||||||
envcpy(up->egrp, oeg);
|
envcpy(up->egrp, oeg);
|
||||||
closeegrp(oeg);
|
closeegrp(oeg);
|
||||||
}
|
}
|
||||||
if(flag & RFNOTEG)
|
if(flag & RFNOTEG){
|
||||||
setnoteid(up, 0);
|
qlock(&up->debug);
|
||||||
|
setnoteid(up, 0); /* can't error() with 0 argument */
|
||||||
|
qunlock(&up->debug);
|
||||||
|
}
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
if((p = newproc()) == nil)
|
if((p = newproc()) == nil)
|
||||||
error("no procs");
|
error("no procs");
|
||||||
|
|
||||||
|
qlock(&up->debug);
|
||||||
qlock(&p->debug);
|
qlock(&p->debug);
|
||||||
|
|
||||||
p->scallnr = up->scallnr;
|
p->scallnr = up->scallnr;
|
||||||
|
@ -112,7 +116,8 @@ sysrfork(va_list list)
|
||||||
p->procctl = Proc_tracesyscall;
|
p->procctl = Proc_tracesyscall;
|
||||||
p->kp = 0;
|
p->kp = 0;
|
||||||
|
|
||||||
/* Craft a return frame which will cause the child to pop out of
|
/*
|
||||||
|
* Craft a return frame which will cause the child to pop out of
|
||||||
* the scheduler in user mode with the return register zero
|
* the scheduler in user mode with the return register zero
|
||||||
*/
|
*/
|
||||||
forkchild(p, up->dbgreg);
|
forkchild(p, up->dbgreg);
|
||||||
|
@ -132,6 +137,7 @@ sysrfork(va_list list)
|
||||||
pid = pidalloc(p);
|
pid = pidalloc(p);
|
||||||
|
|
||||||
qunlock(&p->debug);
|
qunlock(&p->debug);
|
||||||
|
qunlock(&up->debug);
|
||||||
|
|
||||||
/* Abort the child process on error */
|
/* Abort the child process on error */
|
||||||
if(waserror()){
|
if(waserror()){
|
||||||
|
|
Loading…
Reference in a new issue