[Qemu-devel] [PATCH v2 1/8] linux-user: Disallow setting newsp for fork

[Richard Henderson]

Or really, just clone devolving into fork.  This should not ever happen
in practice.  We do want to reserve calling cpu_clone_regs for the case
in which we are actually performing a clone.

Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 linux-user/syscall.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 96cd4bf86d..f7d0754c8d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5553,10 +5553,14 @@ static int do_fork(CPUArchState *env, unsigned int 
flags, abi_ulong newsp,
 pthread_mutex_destroy(&info.mutex);
 pthread_mutex_unlock(&clone_lock);
 } else {
-/* if no CLONE_VM, we consider it is a fork */
+/* If no CLONE_VM, we consider it is a fork.  */
 if (flags & CLONE_INVALID_FORK_FLAGS) {
 return -TARGET_EINVAL;
 }
+/* As a fork, setting a new sp does not make sense.  */
+if (newsp) {
+return -TARGET_EINVAL;
+}
 
 /* We can't support custom termination signals */
 if ((flags & CSIGNAL) != TARGET_SIGCHLD) {
@@ -5571,7 +5575,6 @@ static int do_fork(CPUArchState *env, unsigned int flags, 
abi_ulong newsp,
 ret = fork();
 if (ret == 0) {
 /* Child Process.  */
-cpu_clone_regs(env, newsp);
 fork_end(1);
 /* There is a race condition here.  The parent process could
theoretically read the TID in the child process before the child
-- 
2.17.1

Re: [Qemu-devel] [PATCH v2 1/8] linux-user: Disallow setting newsp for fork

[Richard Henderson]

On 5/9/19 8:27 PM, Richard Henderson wrote:
> Or really, just clone devolving into fork.  This should not ever happen
> in practice.  We do want to reserve calling cpu_clone_regs for the case
> in which we are actually performing a clone.
> 
> Reviewed-by: Alex Bennée 
> Signed-off-by: Richard Henderson 
> ---
>  linux-user/syscall.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 96cd4bf86d..f7d0754c8d 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5553,10 +5553,14 @@ static int do_fork(CPUArchState *env, unsigned int 
> flags, abi_ulong newsp,
>  pthread_mutex_destroy(&info.mutex);
>  pthread_mutex_unlock(&clone_lock);
>  } else {
> -/* if no CLONE_VM, we consider it is a fork */
> +/* If no CLONE_VM, we consider it is a fork.  */
>  if (flags & CLONE_INVALID_FORK_FLAGS) {
>  return -TARGET_EINVAL;
>  }
> +/* As a fork, setting a new sp does not make sense.  */
> +if (newsp) {
> +return -TARGET_EINVAL;
> +}

This causes failures for aarch64 and riscv.

We have to allow no-op setting of sp as well.
Other targets set newsp to 0 for in vfork.S in glibc.


r~