On Sun, Mar 21, 2010 at 09:06:04PM -0400, Christoffer Dall wrote:
> In addition to doing everything that clone() system call does, the
> eclone() system call:
Some comments...
> +sys_eclone_wrapper:
> + add ip, sp, #S_OFF
> + str ip, [sp, #0]
> + b sys_eclone
> +ENDPROC(sys_eclone_wrapper)
I'm curious why, if you want the entire set of registers, you don't just
do:
add r0, sp, #S_OFF
b sys_eclone
and load the syscall arguments out of regs->ARM_foo. This avoids the need
for additional stores.
> +
> sys_sigreturn_wrapper:
> add r0, sp, #S_OFF
> b sys_sigreturn
> diff --git a/arch/arm/kernel/sys_arm.c b/arch/arm/kernel/sys_arm.c
> index ae4027b..fd8199d 100644
> --- a/arch/arm/kernel/sys_arm.c
> +++ b/arch/arm/kernel/sys_arm.c
> @@ -183,6 +183,45 @@ asmlinkage int sys_clone(unsigned long clone_flags,
> unsigned long newsp,
> return do_fork(clone_flags, newsp, regs, 0, parent_tidptr,
> child_tidptr);
> }
>
> +asmlinkage int sys_eclone(unsigned flags_low, struct clone_args __user *uca,
> + int args_size, pid_t __user *pids,
> + struct pt_regs *regs)
> +{
> + int rc;
> + struct clone_args kca;
> + unsigned long flags;
> + int __user *parent_tidp;
> + int __user *child_tidp;
> + unsigned long __user stack;
__user on an integer type doesn't make any sense; integer types do not
have address spaces.
> + unsigned long stack_size;
> +
> + rc = fetch_clone_args_from_user(uca, args_size, &kca);
> + if (rc)
> + return rc;
> +
> + /*
> + * TODO: Convert 'clone-flags' to 64-bits on all architectures.
> + * TODO: When ->clone_flags_high is non-zero, copy it in to the
> + * higher word(s) of 'flags':
> + *
> + * flags = (kca.clone_flags_high << 32) | flags_low;
> + */
> + flags = flags_low;
> + parent_tidp = (int *)(unsigned long)kca.parent_tid_ptr;
> + child_tidp = (int *)(unsigned long)kca.child_tid_ptr;
This will produce sparse errors. Is there a reason why 'clone_args'
tid pointers aren't already pointers marked with __user ?
> +
> + stack_size = (unsigned long)kca.child_stack_size;
Shouldn't this already be of integer type?
> + if (stack_size)
> + return -EINVAL;
So the stack must have a zero size? Is this missing a '!' ?
> +
> + stack = (unsigned long)kca.child_stack;
> + if (!stack)
> + stack = regs->ARM_sp;
> +
> + return do_fork_with_pids(flags, stack, regs, stack_size, parent_tidp,
> + child_tidp, kca.nr_pids, pids);
Hmm, so let me get this syscall interface right. We have some arguments
passed in registers and others via a (variable sized?) structure. It seems
really weird to have, eg, a pointer to the pids and the number of pids
passed in two separate ways.
The grouping between what's passed in registers and via this clone_args
structure seems to be random. Can it be sanitized?
_______________________________________________
Containers mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Devel mailing list
[email protected]
https://openvz.org/mailman/listinfo/devel
