[C/R ARM][PATCH 2/3] ARM: Add the eclone system call
From: Russell King - ARM Linux <hidden>
Date: 2010-03-23 23:57:08
Also in:
lkml
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.
quoted hunk ↗ jump to hunk
+ sys_sigreturn_wrapper: add r0, sp, #S_OFF b sys_sigreturndiff --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?