Re: [PATCH 1/1] implement s390 clone_with_pids syscall

["Serge E. Hallyn" <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>]

Quoting Nathan Lynch (nathanl-V7BBcbaFuwjMbYB6QlFGEg@public.gmane.org):
> On Wed, 2009-11-11 at 08:46 -0600, Serge E. Hallyn wrote:
> > Quoting Nathan Lynch (nathanl-V7BBcbaFuwjMbYB6QlFGEg@public.gmane.org):
> > > 
> > > > +	parent_tid_ptr = (int *)kca.parent_tid_ptr;
> > > > +	child_tid_ptr =  (int *)kca.child_tid_ptr;
> > > > +
> > > > +	stack_size = (unsigned long)kca.child_stack_size;
> > > > +	child_stack = (unsigned long)kca.child_stack_base;
> > > > +	if (child_stack)
> > > > +		child_stack += stack_size;
> > > 
> > > Should this calculation not be of the form:
> > > child_stack = arch_dependent_alignment(child_stack + stack_size - 1)
> > > ?
> > > 
> > > Is overflow a concern?
> > > 
> > > Same questions apply to the x86 version.
> > 
> > Hmm...  if the stack isn't valid, the task will just segfault, so
> > it's not dangerous for the kernel, right?  Note that for instance
> > arch/s390/kernel/process.c:SYS_clone() doesn't check the validity
> > of the new stack pointer passed in either.
> 
> clone expects the stack argument to be the desired value of the stack
> pointer in the child.

And doesn't verify it.

> cwp is different in that the clone_args struct
> specifies the base and size of the region the child is to use for stack,
> meaning that the kernel must derive from these a sane value for the
> child's stack pointer (on every arch where the stack grows down).

And with regular clone, the kernel must expect userspace to do that
calculation correctly!  Userspace always still has to do
	base = malloc(size);
	base += size - 1;

> Your current calculation results in an unaligned SP outside of the

Can you send the patch to align it properly? 

> region that the caller has presumably allocated for the child stack.
> How is that useful behavior?

It's useful because stack_size still gets passed through copy_process
to the arch-dependent copy_thread().  That then mostly ignores the
size, but in theory we could start tracking it.

Anyway I'm not totally opposed to expecting stack_top in most
architectures, I just object to your assertion that somehow it
makes more sense for the kernel to expect userspace to do a
valid stack_top calculation than for the kernel to take the
stack_base and stack_size and comput it itself.

-serge
>