From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Serge E. Hallyn" Subject: Re: [PATCH user-cr 2/2] add nsexeccwp to test clone-with-pids Date: Mon, 16 Nov 2009 12:26:55 -0600 Message-ID: <20091116182655.GA3777@us.ibm.com> References: <1258089886-10034-1-git-send-email-serue@us.ibm.com> <1258089886-10034-3-git-send-email-serue@us.ibm.com> <1258325156.4031.3.camel@localhost.localdomain> <20091116111249.GA32340@us.ibm.com> <1258328984.4031.21.camel@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1258328984.4031.21.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Nathan Lynch Cc: containers-qjLDD68F18O7TbgM5vRIOg@public.gmane.org List-Id: containers.vger.kernel.org Quoting Nathan Lynch (ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org): > On Mon, 2009-11-16 at 05:12 -0600, Serge E. Hallyn wrote: > > Quoting Nathan Lynch (ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org): > > > On Thu, 2009-11-12 at 23:24 -0600, serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org 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