svn commit: r326145 - in head/sys: amd64/amd64 amd64/ia32 amd64/linux32 arm/cloudabi32 arm64/arm64 arm64/cloudabi64 compat/linux i386/i386 kern powerpc/powerpc riscv/riscv sparc64/sparc64
Ed Schouten
ed at FreeBSD.org
Fri Nov 24 07:35:11 UTC 2017
Author: ed
Date: Fri Nov 24 07:35:08 2017
New Revision: 326145
URL: https://svnweb.freebsd.org/changeset/base/326145
Log:
Don't let cpu_set_syscall_retval() clobber exec_setregs().
Upon successful completion, the execve() system call invokes
exec_setregs() to initialize the registers of the initial thread of the
newly executed process. What is weird is that when execve() returns, it
still goes through the normal system call return path, clobbering the
registers with the system call's return value (td->td_retval).
Though this doesn't seem to be problematic for x86 most of the times (as
the value of eax/rax doesn't matter upon startup), this can be pretty
frustrating for architectures where function argument and return
registers overlap (e.g., ARM). On these systems, exec_setregs() also
needs to initialize td_retval.
Even worse are architectures where cpu_set_syscall_retval() sets
registers to values not derived from td_retval. On these architectures,
there is no way cpu_set_syscall_retval() can set registers to the way it
wants them to be upon the start of execution.
To get rid of this madness, let sys_execve() return EJUSTRETURN. This
will cause cpu_set_syscall_retval() to leave registers intact. This
makes process execution easier to understand. It also eliminates the
difference between execution of the initial process and successive ones.
The initial call to sys_execve() is not performed through a system call
context.
Reviewed by: kib, jhibbits
Differential Revision: https://reviews.freebsd.org/D13180
Modified:
head/sys/amd64/amd64/machdep.c
head/sys/amd64/ia32/ia32_signal.c
head/sys/amd64/linux32/linux32_sysvec.c
head/sys/arm/cloudabi32/cloudabi32_sysvec.c
head/sys/arm64/arm64/machdep.c
head/sys/arm64/cloudabi64/cloudabi64_sysvec.c
head/sys/compat/linux/linux_emul.c
head/sys/i386/i386/machdep.c
head/sys/kern/init_main.c
head/sys/kern/kern_exec.c
head/sys/powerpc/powerpc/exec_machdep.c
head/sys/riscv/riscv/machdep.c
head/sys/sparc64/sparc64/machdep.c
Modified: head/sys/amd64/amd64/machdep.c
==============================================================================
--- head/sys/amd64/amd64/machdep.c Fri Nov 24 05:01:00 2017 (r326144)
+++ head/sys/amd64/amd64/machdep.c Fri Nov 24 07:35:08 2017 (r326145)
@@ -604,7 +604,6 @@ exec_setregs(struct thread *td, struct image_params *i
regs->tf_fs = _ufssel;
regs->tf_gs = _ugssel;
regs->tf_flags = TF_HASSEGS;
- td->td_retval[1] = 0;
/*
* Reset the hardware debug registers if they were in use.
Modified: head/sys/amd64/ia32/ia32_signal.c
==============================================================================
--- head/sys/amd64/ia32/ia32_signal.c Fri Nov 24 05:01:00 2017 (r326144)
+++ head/sys/amd64/ia32/ia32_signal.c Fri Nov 24 07:35:08 2017 (r326145)
@@ -967,5 +967,4 @@ ia32_setregs(struct thread *td, struct image_params *i
/* Return via doreti so that we can change to a different %cs */
set_pcb_flags(pcb, PCB_32BIT | PCB_FULL_IRET);
- td->td_retval[1] = 0;
}
Modified: head/sys/amd64/linux32/linux32_sysvec.c
==============================================================================
--- head/sys/amd64/linux32/linux32_sysvec.c Fri Nov 24 05:01:00 2017 (r326144)
+++ head/sys/amd64/linux32/linux32_sysvec.c Fri Nov 24 07:35:08 2017 (r326145)
@@ -832,7 +832,6 @@ exec_linux_setregs(struct thread *td, struct image_par
/* Do full restore on return so that we can change to a different %cs */
set_pcb_flags(pcb, PCB_32BIT | PCB_FULL_IRET);
- td->td_retval[1] = 0;
}
/*
Modified: head/sys/arm/cloudabi32/cloudabi32_sysvec.c
==============================================================================
--- head/sys/arm/cloudabi32/cloudabi32_sysvec.c Fri Nov 24 05:01:00 2017 (r326144)
+++ head/sys/arm/cloudabi32/cloudabi32_sysvec.c Fri Nov 24 07:35:08 2017 (r326145)
@@ -61,7 +61,7 @@ cloudabi32_proc_setregs(struct thread *td, struct imag
* tpidrurw to the TCB.
*/
regs = td->td_frame;
- regs->tf_r0 = td->td_retval[0] =
+ regs->tf_r0 =
stack + roundup(sizeof(cloudabi32_tcb_t), sizeof(register_t));
(void)cpu_set_user_tls(td, (void *)stack);
}
Modified: head/sys/arm64/arm64/machdep.c
==============================================================================
--- head/sys/arm64/arm64/machdep.c Fri Nov 24 05:01:00 2017 (r326144)
+++ head/sys/arm64/arm64/machdep.c Fri Nov 24 07:35:08 2017 (r326145)
@@ -311,12 +311,7 @@ exec_setregs(struct thread *td, struct image_params *i
memset(tf, 0, sizeof(struct trapframe));
- /*
- * We need to set x0 for init as it doesn't call
- * cpu_set_syscall_retval to copy the value. We also
- * need to set td_retval for the cases where we do.
- */
- tf->tf_x[0] = td->td_retval[0] = stack;
+ tf->tf_x[0] = stack;
tf->tf_sp = STACKALIGN(stack);
tf->tf_lr = imgp->entry_addr;
tf->tf_elr = imgp->entry_addr;
Modified: head/sys/arm64/cloudabi64/cloudabi64_sysvec.c
==============================================================================
--- head/sys/arm64/cloudabi64/cloudabi64_sysvec.c Fri Nov 24 05:01:00 2017 (r326144)
+++ head/sys/arm64/cloudabi64/cloudabi64_sysvec.c Fri Nov 24 07:35:08 2017 (r326145)
@@ -61,7 +61,7 @@ cloudabi64_proc_setregs(struct thread *td, struct imag
* tpidr_el0 to the TCB.
*/
regs = td->td_frame;
- regs->tf_x[0] = td->td_retval[0] =
+ regs->tf_x[0] =
stack + roundup(sizeof(cloudabi64_tcb_t), sizeof(register_t));
(void)cpu_set_user_tls(td, (void *)stack);
}
Modified: head/sys/compat/linux/linux_emul.c
==============================================================================
--- head/sys/compat/linux/linux_emul.c Fri Nov 24 05:01:00 2017 (r326144)
+++ head/sys/compat/linux/linux_emul.c Fri Nov 24 07:35:08 2017 (r326145)
@@ -186,7 +186,7 @@ linux_common_execve(struct thread *td, struct image_ar
error = kern_execve(td, eargs, NULL);
post_execve(td, error, oldvmspace);
- if (error != 0)
+ if (error != EJUSTRETURN)
return (error);
/*
@@ -213,7 +213,7 @@ linux_common_execve(struct thread *td, struct image_ar
free(em, M_TEMP);
free(pem, M_LINUX);
}
- return (0);
+ return (EJUSTRETURN);
}
void
Modified: head/sys/i386/i386/machdep.c
==============================================================================
--- head/sys/i386/i386/machdep.c Fri Nov 24 05:01:00 2017 (r326144)
+++ head/sys/i386/i386/machdep.c Fri Nov 24 07:35:08 2017 (r326145)
@@ -1126,6 +1126,7 @@ exec_setregs(struct thread *td, struct image_params *i
set_fsbase(td, 0);
set_gsbase(td, 0);
+ /* Make sure edx is 0x0 on entry. Linux binaries depend on it. */
bzero((char *)regs, sizeof(struct trapframe));
regs->tf_eip = imgp->entry_addr;
regs->tf_esp = stack;
@@ -1168,13 +1169,6 @@ exec_setregs(struct thread *td, struct image_params *i
* clean FP state if it uses the FPU again.
*/
fpstate_drop(td);
-
- /*
- * XXX - Linux emulator
- * Make sure sure edx is 0x0 on entry. Linux binaries depend
- * on it.
- */
- td->td_retval[1] = 0;
}
void
Modified: head/sys/kern/init_main.c
==============================================================================
--- head/sys/kern/init_main.c Fri Nov 24 05:01:00 2017 (r326144)
+++ head/sys/kern/init_main.c Fri Nov 24 07:35:08 2017 (r326145)
@@ -797,7 +797,7 @@ start_init(void *dummy)
* Otherwise, return via fork_trampoline() all the way
* to user mode as init!
*/
- if ((error = sys_execve(td, &args)) == 0) {
+ if ((error = sys_execve(td, &args)) == EJUSTRETURN) {
mtx_unlock(&Giant);
return;
}
Modified: head/sys/kern/kern_exec.c
==============================================================================
--- head/sys/kern/kern_exec.c Fri Nov 24 05:01:00 2017 (r326144)
+++ head/sys/kern/kern_exec.c Fri Nov 24 07:35:08 2017 (r326145)
@@ -318,7 +318,7 @@ post_execve(struct thread *td, int error, struct vmspa
* If success, we upgrade to SINGLE_EXIT state to
* force other threads to suicide.
*/
- if (error == 0)
+ if (error == EJUSTRETURN)
thread_single(p, SINGLE_EXIT);
else
thread_single_end(p, SINGLE_BOUNDARY);
@@ -962,7 +962,13 @@ exec_fail:
ktrprocctor(p);
#endif
- return (error);
+ /*
+ * We don't want cpu_set_syscall_retval() to overwrite any of
+ * the register values put in place by exec_setregs().
+ * Implementations of cpu_set_syscall_retval() will leave
+ * registers unmodified when returning EJUSTRETURN.
+ */
+ return (error == 0 ? EJUSTRETURN : error);
}
int
Modified: head/sys/powerpc/powerpc/exec_machdep.c
==============================================================================
--- head/sys/powerpc/powerpc/exec_machdep.c Fri Nov 24 05:01:00 2017 (r326144)
+++ head/sys/powerpc/powerpc/exec_machdep.c Fri Nov 24 07:35:08 2017 (r326145)
@@ -520,22 +520,11 @@ exec_setregs(struct thread *td, struct image_params *i
* - ps_strings is a NetBSD extention, and will be
* ignored by executables which are strictly
* compliant with the SVR4 ABI.
- *
- * XXX We have to set both regs and retval here due to different
- * XXX calling convention in trap.c and init_main.c.
*/
/* Collect argc from the user stack */
argc = fuword((void *)stack);
- /*
- * XXX PG: these get overwritten in the syscall return code.
- * execve() should return EJUSTRETURN, like it does on NetBSD.
- * Emulate by setting the syscall return value cells. The
- * registers still have to be set for init's fork trampoline.
- */
- td->td_retval[0] = argc;
- td->td_retval[1] = stack + sizeof(register_t);
tf->fixreg[3] = argc;
tf->fixreg[4] = stack + sizeof(register_t);
tf->fixreg[5] = stack + (2 + argc)*sizeof(register_t);
@@ -572,8 +561,6 @@ ppc32_setregs(struct thread *td, struct image_params *
argc = fuword32((void *)stack);
- td->td_retval[0] = argc;
- td->td_retval[1] = stack + sizeof(uint32_t);
tf->fixreg[3] = argc;
tf->fixreg[4] = stack + sizeof(uint32_t);
tf->fixreg[5] = stack + (2 + argc)*sizeof(uint32_t);
Modified: head/sys/riscv/riscv/machdep.c
==============================================================================
--- head/sys/riscv/riscv/machdep.c Fri Nov 24 05:01:00 2017 (r326144)
+++ head/sys/riscv/riscv/machdep.c Fri Nov 24 07:35:08 2017 (r326145)
@@ -279,12 +279,7 @@ exec_setregs(struct thread *td, struct image_params *i
memset(tf, 0, sizeof(struct trapframe));
- /*
- * We need to set a0 for init as it doesn't call
- * cpu_set_syscall_retval to copy the value. We also
- * need to set td_retval for the cases where we do.
- */
- tf->tf_a[0] = td->td_retval[0] = stack;
+ tf->tf_a[0] = stack;
tf->tf_sp = STACKALIGN(stack);
tf->tf_ra = imgp->entry_addr;
tf->tf_sepc = imgp->entry_addr;
Modified: head/sys/sparc64/sparc64/machdep.c
==============================================================================
--- head/sys/sparc64/sparc64/machdep.c Fri Nov 24 05:01:00 2017 (r326144)
+++ head/sys/sparc64/sparc64/machdep.c Fri Nov 24 07:35:08 2017 (r326145)
@@ -1009,9 +1009,6 @@ exec_setregs(struct thread *td, struct image_params *i
* header, it turns out that just always using TSO performs best.
*/
tf->tf_tstate = TSTATE_IE | TSTATE_PEF | TSTATE_MM_TSO;
-
- td->td_retval[0] = tf->tf_out[0];
- td->td_retval[1] = tf->tf_out[1];
}
int
More information about the svn-src-all
mailing list