Quoting Nathan Lynch ([email protected]):
> On Mon, 2009-11-16 at 05:12 -0600, Serge E. Hallyn wrote:
> > Quoting Nathan Lynch ([email protected]):
> > > On Thu, 2009-11-12 at 23:24 -0600, [email protected] wrote:
> > > > +       if (use_clone) {
> > > > +               int stacksize = 4*getpagesize();
> > > > +               void *stack = malloc(stacksize);
> > > > +
> > > > +               if (!stack) {
> > > > +                       perror("malloc");
> > > > +                       return -1;
> > > > +               }
> > > > +
> > > > +               printf("about to clone with %lx\n", flags);
> > > > +               if (chosen_pid)
> > > > +                       printf("Will choose pid %d\n", chosen_pid);
> > > > +               flags |= SIGCHLD;
> > > > +               pid = clone_with_pids(do_child, stack, flags, &pid_set,
> > > > +                                       (void *)argv);
> > > 
> > > The stack argument should be adjusted with the usual stack += stacksize
> > > - 1 or similar, right?
> > 
> > the clone_with_pids() helper in user-cr/clone_s390x.c (and IIRC the
> > x86 one by Suka also) does this implicitly, by doing:
> > 
> >     s = child_stack;
> >     *--s = arg;
> >     *--s = fn;
> >     child_stack -= 16
> 
> That's setting up arguments for the function to run in the child, and
> afaict that code assumes the value of child_stack is the _end_ of the
> stack region.

Yes.

> The code I quoted above is passing the beginning of the
> region (the return value from malloc).

Holy cow, that was a snafu in my switching to sending (stack_base,stack_size)
for the previous version, and then back again.  It was meant to send
stack_base+stack_size now.

I say 'holy cow' because it doesn't segfault on s390x.  And it certainly
should!

> On powerpc the segfaults went away when I made the following change.
> 
> diff --git a/nsexeccwp.c b/nsexeccwp.c
> index a71d9a4..92eb092 100644
> --- a/nsexeccwp.c
> +++ b/nsexeccwp.c
> @@ -309,8 +309,8 @@ int main(int argc, char *argv[])
>                 if (chosen_pid)
>                         printf("Will choose pid %d\n", chosen_pid);
>                 flags |= SIGCHLD;
> -               pid = clone_with_pids(do_child, stack, flags, &pid_set,
> -                                       (void *)argv);
> +               pid = clone_with_pids(do_child, stack + stacksize - 1,
> +                                     flags, &pid_set, (void *)argv);

Yes I don't think the -1 should be needed, but certainly the
+stacksize is.

thanks,
-serge
_______________________________________________
Containers mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/containers

_______________________________________________
Devel mailing list
[email protected]
https://openvz.org/mailman/listinfo/devel

Reply via email to