[CRIU] [PATCH 07/17] dump: move the may_dump() check in seize_task()

[Andrew Vagin]

On Thu, Nov 06, 2014 at 03:38:12PM +0400, Pavel Emelyanov wrote:
> 
> > @@ -78,26 +79,31 @@ int seize_task(pid_t pid, pid_t ppid)
> >  	 * we might nead at that early point.
> >  	 */
> >  
> > -	ret2 = parse_pid_stat_small(pid, &ps);
> > -	if (ret2 < 0)
> > -		return -1;
> > +	ret2 = parse_pid_status(pid, &cr);
> > +	if (ret2)
> > +		goto err;
> > +
> > +	if (!may_dump(&cr)) {
> > +		pr_err("Check uid (pid: %d) failed\n", pid);
> > +		goto err;
> > +	}
> 
> Don't we have a race here? Consider you're spawning a siud application
> and dump it. You get the may_dump() check while the process you spawned
> still belongs to you and the may_dump() check succeeds. Then the 
> application calls setiud() raising the priviledges, then you go and
> seize one.

No, we don't. The processes is stopped in this moment and it can't do
anything.

> 
> >  	if (ret < 0) {
> > -		if (ps.state != 'Z') {
> > +		if (cr.state != 'Z') {
> >  			if (pid == getpid())
> >  				pr_err("The criu itself is within dumped tree.\n");
> >  			else
> >  				pr_err("Unseizable non-zombie %d found, state %c, err %d/%d\n",
> > -						pid, ps.state, ret, ptrace_errno);
> > +						pid, cr.state, ret, ptrace_errno);
> >  			return -1;
> >  		}
> >  
> >  		return TASK_DEAD;
> >  	}
> >  
> > -	if ((ppid != -1) && (ps.ppid != ppid)) {
> > +	if ((ppid != -1) && (cr.ppid != ppid)) {
> >  		pr_err("Task pid reused while suspending (%d: %d -> %d)\n",
> > -				pid, ppid, ps.ppid);
> > +				pid, ppid, cr.ppid);
> >  		goto err;
> >  	}
> >  
> > 
>