Re: [RFC][PATCH 7/7][v2] Define clone_with_pids syscall

[Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>]

Sukadev Bhattiprolu wrote:
> Oren Laadan [orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org] wrote:
> 
> | > +
> | > +	if (num_pids < 0 || num_pids > task_pid(current)->level + 1)
> | > +		return ERR_PTR(-EINVAL);
> | 
> | What happens if (num_pids < task_pid(current->level) + 1) ?
> | Unless I missed something elsewhere, I suspect it will oops,
> 
> Yep. the kzalloc() below this check should use:
> 
> 	task_pid(current)->pid_ns->level+1 * sizeof(pid_t)
> 
> | because in patch #4, you had:
> | 
> |  	for (i = ns->level; i >= 0; i--) {
> | -		nr = alloc_pidmap(tmp, 0);
> | +		tpid = 0;
> | +		if (target_pids)
> | +			tpid = target_pids[i];
> | +
> | +		nr = alloc_pidmap(tmp, tpid);
> |  		if (nr < 0)
> |  			goto out_free;
> | 
> | In general, can a task figure out it's depth in the pid-ns hierarchy ?
> | 
> | I'm thinking of a case in which a checkpoint was taken of a (flat)
> | container which is in depth 2 of the global hierarchy, and then it
> | is restarted in depth 3, or in depth 1.
> | 
> | Perhaps the semantics of the syscall should be that target_pids will
> | indicate the desired pids _from the bottom up_. A value of "0" means
> 
> Hmm, I thought bottom up would be confusing, but I guess that would
> work better :-) Will fix and resend.

(Repeating...)

Perhaps bottom-up is a misleading: I don't care about the order...
I meant that a N size array should affect the last N levels of
the hierarchy (if it is deeper than N).

You could actually keep everything else as is, and change the code
that copies from user to always allocate a ns->level+1 size array,
and copy the user data to the end of it, zeroing the beginning.

Oren.

> 
> | "don't care" and let the kernel assign. The remaining levels (upwards)
> | will be treated as zeros.
> | 
> | [...]
> | 
> | Oren.
> | 
>