Re: [PATCH 2/2] clone_with_pids: define the s390 syscall

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

Quoting Martin Schwidefsky (schwidefsky-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org):
> Hi Serge,
> 
> On Mon, 15 Jun 2009 12:16:45 -0500
> "Serge E. Hallyn" <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> wrote:
> 
> > diff --git a/arch/s390/kernel/compat_wrapper.S b/arch/s390/kernel/compat_wrapper.S
> > index c2228b2..bf13315 100644
> > --- a/arch/s390/kernel/compat_wrapper.S
> > +++ b/arch/s390/kernel/compat_wrapper.S
> > @@ -1837,3 +1837,13 @@ sys_restore_wrapper:
> >  	lgfr	%r3,%r3			# int
> >  	llgfr	%r4,%r4			# unsigned long
> >  	jg	compat_sys_restore
> > +
> > +	.globl sys_clone_with_pids_wrapper
> > +sys_clone_with_pids_wrapper:
> > +	llgfr	%r2,%r2			# unsigned long
> > +	llgfr	%r3,%r3			# unsigned long
> > +	llgtr	%r4,%r4			# int *
> > +	llgtr	%r5,%r5			# int *
> > +	llgtr	%r6,%r6			# void *
> > +	llgtr	%r7,%r7			# void *
> > +	jg	compat_sys_clone_with_pids
> 
> This is incorrect. If you have a system call that takes 6 parameters
> you need to load/store the 6th parameter from the stack. Check out the
> futex system call wrapper. But before you do that see the next comment.
> 
> > diff --git a/arch/s390/kernel/process.c b/arch/s390/kernel/process.c
> > index a3acd8e..fa187bf 100644
> > --- a/arch/s390/kernel/process.c
> > +++ b/arch/s390/kernel/process.c
> > @@ -246,6 +246,25 @@ SYSCALL_DEFINE0(clone)
> >  		       parent_tidptr, child_tidptr);
> >  }
> > 
> > +SYSCALL_DEFINE0(clone_with_pids)
> > +{
> > +	struct pt_regs *regs = task_pt_regs(current);
> > +	unsigned long clone_flags;
> > +	unsigned long newsp;
> > +	int __user *parent_tidptr, *child_tidptr;
> > +	void __user *upid_setp;
> > +
> > +	clone_flags = regs->gprs[3];
> > +	newsp = regs->orig_gpr2;
> > +	parent_tidptr = (int __user *) regs->gprs[4];
> > +	child_tidptr = (int __user *) regs->gprs[5];
> > +	upid_setp = (void __user *) regs->gprs[7];
> > +	if (!newsp)
> > +		newsp = regs->gprs[15];
> > +	return do_fork_with_pids(clone_flags, newsp, regs, 0, parent_tidptr,
> > +			child_tidptr, upid_setp);
> > +}
> > +
> >  /*
> >   * This is trivial, and on the face of it looks like it
> >   * could equally well be done in user mode.
> 
> clone_with_pids is declared as system call with no paramters. In this
> case the system call wrapper is not needed (empty) and you have to do
> the compat conversion inside the system call. See sys32_clone.

Ah, I see, thanks.

So in that case, is it ok for me to just use gprs[7] to pass in the
upid_setp variable?

thanks,
-serge