* [PATCH v2 1/5] target/i386: Renumber EXCP_SYSCALL
2020-01-16 19:43 [PATCH v2 0/5] linux-user: Implement x86_64 vsyscalls Richard Henderson
@ 2020-01-16 19:43 ` Richard Henderson
2020-01-16 19:43 ` [PATCH v2 2/5] linux-user/i386: Split out gen_signal Richard Henderson
` (5 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Richard Henderson @ 2020-01-16 19:43 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, alex.bennee, laurent, Philippe Mathieu-Daudé
We are not short of numbers for EXCP_*. There is no need to confuse things
by having EXCP_VMEXIT and EXCP_SYSCALL overlap, even though the former is
only used for system mode and the latter is only used for user mode.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/i386/cpu.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 594326a794..164d038d1f 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -998,9 +998,8 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
#define EXCP11_ALGN 17
#define EXCP12_MCHK 18
-#define EXCP_SYSCALL 0x100 /* only happens in user only emulation
- for syscall instruction */
-#define EXCP_VMEXIT 0x100
+#define EXCP_VMEXIT 0x100 /* only for system emulation */
+#define EXCP_SYSCALL 0x101 /* only for user emulation */
/* i386-specific interrupt pending bits. */
#define CPU_INTERRUPT_POLL CPU_INTERRUPT_TGT_EXT_1
--
2.20.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/5] linux-user/i386: Split out gen_signal
2020-01-16 19:43 [PATCH v2 0/5] linux-user: Implement x86_64 vsyscalls Richard Henderson
2020-01-16 19:43 ` [PATCH v2 1/5] target/i386: Renumber EXCP_SYSCALL Richard Henderson
@ 2020-01-16 19:43 ` Richard Henderson
2020-01-16 19:43 ` [PATCH v2 3/5] linux-user/i386: Emulate x86_64 vsyscalls Richard Henderson
` (4 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Richard Henderson @ 2020-01-16 19:43 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, alex.bennee, laurent, Philippe Mathieu-Daudé
This is a bit tidier than open-coding the 5 lines necessary
to initialize the target_siginfo_t. In addition, this zeros
the remaining bytes of the target_siginfo_t, rather than
passing in garbage.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/i386/cpu_loop.c | 93 ++++++++++++++------------------------
1 file changed, 33 insertions(+), 60 deletions(-)
diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
index 024b6f4d58..e217cca5ee 100644
--- a/linux-user/i386/cpu_loop.c
+++ b/linux-user/i386/cpu_loop.c
@@ -81,13 +81,23 @@ static void set_idt(int n, unsigned int dpl)
}
#endif
+static void gen_signal(CPUX86State *env, int sig, int code, abi_ptr addr)
+{
+ target_siginfo_t info = {
+ .si_signo = sig,
+ .si_code = code,
+ ._sifields._sigfault._addr = addr
+ };
+
+ queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+}
+
void cpu_loop(CPUX86State *env)
{
CPUState *cs = env_cpu(env);
int trapnr;
abi_ulong pc;
abi_ulong ret;
- target_siginfo_t info;
for(;;) {
cpu_exec_start(cs);
@@ -134,70 +144,45 @@ void cpu_loop(CPUX86State *env)
#endif
case EXCP0B_NOSEG:
case EXCP0C_STACK:
- info.si_signo = TARGET_SIGBUS;
- info.si_errno = 0;
- info.si_code = TARGET_SI_KERNEL;
- info._sifields._sigfault._addr = 0;
- queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+ gen_signal(env, TARGET_SIGBUS, TARGET_SI_KERNEL, 0);
break;
case EXCP0D_GPF:
/* XXX: potential problem if ABI32 */
#ifndef TARGET_X86_64
if (env->eflags & VM_MASK) {
handle_vm86_fault(env);
- } else
-#endif
- {
- info.si_signo = TARGET_SIGSEGV;
- info.si_errno = 0;
- info.si_code = TARGET_SI_KERNEL;
- info._sifields._sigfault._addr = 0;
- queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+ break;
}
+#endif
+ gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0);
break;
case EXCP0E_PAGE:
- info.si_signo = TARGET_SIGSEGV;
- info.si_errno = 0;
- if (!(env->error_code & 1))
- info.si_code = TARGET_SEGV_MAPERR;
- else
- info.si_code = TARGET_SEGV_ACCERR;
- info._sifields._sigfault._addr = env->cr[2];
- queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+ gen_signal(env, TARGET_SIGSEGV,
+ (env->error_code & 1 ?
+ TARGET_SEGV_ACCERR : TARGET_SEGV_MAPERR),
+ env->cr[2]);
break;
case EXCP00_DIVZ:
#ifndef TARGET_X86_64
if (env->eflags & VM_MASK) {
handle_vm86_trap(env, trapnr);
- } else
-#endif
- {
- /* division by zero */
- info.si_signo = TARGET_SIGFPE;
- info.si_errno = 0;
- info.si_code = TARGET_FPE_INTDIV;
- info._sifields._sigfault._addr = env->eip;
- queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+ break;
}
+#endif
+ gen_signal(env, TARGET_SIGFPE, TARGET_FPE_INTDIV, env->eip);
break;
case EXCP01_DB:
case EXCP03_INT3:
#ifndef TARGET_X86_64
if (env->eflags & VM_MASK) {
handle_vm86_trap(env, trapnr);
- } else
+ break;
+ }
#endif
- {
- info.si_signo = TARGET_SIGTRAP;
- info.si_errno = 0;
- if (trapnr == EXCP01_DB) {
- info.si_code = TARGET_TRAP_BRKPT;
- info._sifields._sigfault._addr = env->eip;
- } else {
- info.si_code = TARGET_SI_KERNEL;
- info._sifields._sigfault._addr = 0;
- }
- queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+ if (trapnr == EXCP01_DB) {
+ gen_signal(env, TARGET_SIGTRAP, TARGET_TRAP_BRKPT, env->eip);
+ } else {
+ gen_signal(env, TARGET_SIGTRAP, TARGET_SI_KERNEL, 0);
}
break;
case EXCP04_INTO:
@@ -205,31 +190,19 @@ void cpu_loop(CPUX86State *env)
#ifndef TARGET_X86_64
if (env->eflags & VM_MASK) {
handle_vm86_trap(env, trapnr);
- } else
-#endif
- {
- info.si_signo = TARGET_SIGSEGV;
- info.si_errno = 0;
- info.si_code = TARGET_SI_KERNEL;
- info._sifields._sigfault._addr = 0;
- queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+ break;
}
+#endif
+ gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0);
break;
case EXCP06_ILLOP:
- info.si_signo = TARGET_SIGILL;
- info.si_errno = 0;
- info.si_code = TARGET_ILL_ILLOPN;
- info._sifields._sigfault._addr = env->eip;
- queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+ gen_signal(env, TARGET_SIGILL, TARGET_ILL_ILLOPN, env->eip);
break;
case EXCP_INTERRUPT:
/* just indicate that signals should be handled asap */
break;
case EXCP_DEBUG:
- info.si_signo = TARGET_SIGTRAP;
- info.si_errno = 0;
- info.si_code = TARGET_TRAP_BRKPT;
- queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+ gen_signal(env, TARGET_SIGTRAP, TARGET_TRAP_BRKPT, 0);
break;
case EXCP_ATOMIC:
cpu_exec_step_atomic(cs);
--
2.20.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 3/5] linux-user/i386: Emulate x86_64 vsyscalls
2020-01-16 19:43 [PATCH v2 0/5] linux-user: Implement x86_64 vsyscalls Richard Henderson
2020-01-16 19:43 ` [PATCH v2 1/5] target/i386: Renumber EXCP_SYSCALL Richard Henderson
2020-01-16 19:43 ` [PATCH v2 2/5] linux-user/i386: Split out gen_signal Richard Henderson
@ 2020-01-16 19:43 ` Richard Henderson
2020-01-20 11:48 ` Alex Bennée
2020-01-16 19:43 ` [PATCH v2 4/5] linux-user: Add x86_64 vsyscall page to /proc/self/maps Richard Henderson
` (3 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Richard Henderson @ 2020-01-16 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, alex.bennee, laurent
Notice the magic page during translate, much like we already
do for the arm32 commpage. At runtime, raise an exception to
return cpu_loop for emulation.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/i386/cpu.h | 1 +
linux-user/i386/cpu_loop.c | 105 +++++++++++++++++++++++++++++++++++++
target/i386/translate.c | 16 +++++-
3 files changed, 121 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 164d038d1f..3fb2d2a986 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1000,6 +1000,7 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
#define EXCP_VMEXIT 0x100 /* only for system emulation */
#define EXCP_SYSCALL 0x101 /* only for user emulation */
+#define EXCP_VSYSCALL 0x102 /* only for user emulation */
/* i386-specific interrupt pending bits. */
#define CPU_INTERRUPT_POLL CPU_INTERRUPT_TGT_EXT_1
diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
index e217cca5ee..f9bf6cec27 100644
--- a/linux-user/i386/cpu_loop.c
+++ b/linux-user/i386/cpu_loop.c
@@ -92,6 +92,106 @@ static void gen_signal(CPUX86State *env, int sig, int code, abi_ptr addr)
queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
}
+#ifdef TARGET_X86_64
+static bool write_ok_or_segv(CPUX86State *env, abi_ptr addr, size_t len)
+{
+ /*
+ * For all the vsyscalls, NULL means "don't write anything" not
+ * "write it at address 0".
+ */
+ if (addr == 0 || access_ok(VERIFY_WRITE, addr, len)) {
+ return true;
+ }
+
+ env->error_code = PG_ERROR_W_MASK | PG_ERROR_U_MASK;
+ gen_signal(env, TARGET_SIGSEGV, TARGET_SEGV_MAPERR, addr);
+ return false;
+}
+
+/*
+ * Since v3.1, the kernel traps and emulates the vsyscall page.
+ * Entry points other than the official generate SIGSEGV.
+ */
+static void emulate_vsyscall(CPUX86State *env)
+{
+ int syscall;
+ abi_ulong ret;
+ uint64_t caller;
+
+ /*
+ * Validate the entry point. We have already validated the page
+ * during translation, now verify the offset.
+ */
+ switch (env->eip & ~TARGET_PAGE_MASK) {
+ case 0x000:
+ syscall = TARGET_NR_gettimeofday;
+ break;
+ case 0x400:
+ syscall = TARGET_NR_time;
+ break;
+ case 0x800:
+ syscall = TARGET_NR_getcpu;
+ break;
+ default:
+ sigsegv:
+ /* Like force_sig(SIGSEGV). */
+ gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0);
+ return;
+ }
+
+ /*
+ * Validate the return address.
+ * Note that the kernel treats this the same as an invalid entry point.
+ */
+ if (get_user_u64(caller, env->regs[R_ESP])) {
+ goto sigsegv;
+ }
+
+ /*
+ * Validate the the pointer arguments.
+ */
+ switch (syscall) {
+ case TARGET_NR_gettimeofday:
+ if (!write_ok_or_segv(env, env->regs[R_EDI],
+ sizeof(struct target_timeval)) ||
+ !write_ok_or_segv(env, env->regs[R_ESI],
+ sizeof(struct target_timezone))) {
+ return;
+ }
+ break;
+ case TARGET_NR_time:
+ if (!write_ok_or_segv(env, env->regs[R_EDI], sizeof(abi_long))) {
+ return;
+ }
+ break;
+ case TARGET_NR_getcpu:
+ if (!write_ok_or_segv(env, env->regs[R_EDI], sizeof(uint32_t)) ||
+ !write_ok_or_segv(env, env->regs[R_ESI], sizeof(uint32_t))) {
+ return;
+ }
+ break;
+ default:
+ g_assert_not_reached();
+ }
+
+ /*
+ * Perform the syscall. None of the vsyscalls should need restarting,
+ * and all faults should have been caught above.
+ */
+ ret = do_syscall(env, syscall, env->regs[R_EDI], env->regs[R_ESI],
+ env->regs[R_EDX], env->regs[10], env->regs[8],
+ env->regs[9], 0, 0);
+ g_assert(ret != -TARGET_ERESTARTSYS);
+ g_assert(ret != -TARGET_QEMU_ESIGRETURN);
+ g_assert(ret != -TARGET_EFAULT);
+ env->regs[R_EAX] = ret;
+
+ /* Emulate a ret instruction to leave the vsyscall page. */
+ env->eip = caller;
+ env->regs[R_ESP] += 8;
+}
+#endif
+
void cpu_loop(CPUX86State *env)
{
CPUState *cs = env_cpu(env);
@@ -141,6 +241,11 @@ void cpu_loop(CPUX86State *env)
env->regs[R_EAX] = ret;
}
break;
+#endif
+#ifdef TARGET_X86_64
+ case EXCP_VSYSCALL:
+ emulate_vsyscall(env);
+ break;
#endif
case EXCP0B_NOSEG:
case EXCP0C_STACK:
diff --git a/target/i386/translate.c b/target/i386/translate.c
index 7c99ef1385..391b4ef149 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -8555,7 +8555,21 @@ static bool i386_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cpu,
static void i386_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
{
DisasContext *dc = container_of(dcbase, DisasContext, base);
- target_ulong pc_next = disas_insn(dc, cpu);
+ target_ulong pc_next;
+
+#if defined(TARGET_X86_64) && \
+ defined(CONFIG_USER_ONLY) && \
+ defined(CONFIG_LINUX)
+ /*
+ * Detect entry into the vsyscall page and invoke the syscall.
+ */
+ if ((dc->base.pc_next & TARGET_PAGE_MASK) == 0xffffffffff600000ull) {
+ gen_exception(dc, EXCP_VSYSCALL, dc->base.pc_next);
+ return;
+ }
+#endif
+
+ pc_next = disas_insn(dc, cpu);
if (dc->tf || (dc->base.tb->flags & HF_INHIBIT_IRQ_MASK)) {
/* if single step mode, we generate only one instruction and
--
2.20.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/5] linux-user/i386: Emulate x86_64 vsyscalls
2020-01-16 19:43 ` [PATCH v2 3/5] linux-user/i386: Emulate x86_64 vsyscalls Richard Henderson
@ 2020-01-20 11:48 ` Alex Bennée
2020-01-21 3:38 ` Richard Henderson
0 siblings, 1 reply; 18+ messages in thread
From: Alex Bennée @ 2020-01-20 11:48 UTC (permalink / raw)
To: Richard Henderson; +Cc: Paolo Bonzini, qemu-devel, laurent
Richard Henderson <richard.henderson@linaro.org> writes:
> Notice the magic page during translate, much like we already
> do for the arm32 commpage. At runtime, raise an exception to
> return cpu_loop for emulation.
>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/i386/cpu.h | 1 +
> linux-user/i386/cpu_loop.c | 105 +++++++++++++++++++++++++++++++++++++
> target/i386/translate.c | 16 +++++-
> 3 files changed, 121 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 164d038d1f..3fb2d2a986 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1000,6 +1000,7 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
>
> #define EXCP_VMEXIT 0x100 /* only for system emulation */
> #define EXCP_SYSCALL 0x101 /* only for user emulation */
> +#define EXCP_VSYSCALL 0x102 /* only for user emulation */
>
> /* i386-specific interrupt pending bits. */
> #define CPU_INTERRUPT_POLL CPU_INTERRUPT_TGT_EXT_1
> diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
> index e217cca5ee..f9bf6cec27 100644
> --- a/linux-user/i386/cpu_loop.c
> +++ b/linux-user/i386/cpu_loop.c
> @@ -92,6 +92,106 @@ static void gen_signal(CPUX86State *env, int sig, int code, abi_ptr addr)
> queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
> }
>
> +#ifdef TARGET_X86_64
> +static bool write_ok_or_segv(CPUX86State *env, abi_ptr addr, size_t len)
> +{
> + /*
> + * For all the vsyscalls, NULL means "don't write anything" not
> + * "write it at address 0".
> + */
> + if (addr == 0 || access_ok(VERIFY_WRITE, addr, len)) {
> + return true;
> + }
> +
> + env->error_code = PG_ERROR_W_MASK | PG_ERROR_U_MASK;
> + gen_signal(env, TARGET_SIGSEGV, TARGET_SEGV_MAPERR, addr);
> + return false;
> +}
> +
> +/*
> + * Since v3.1, the kernel traps and emulates the vsyscall page.
> + * Entry points other than the official generate SIGSEGV.
> + */
> +static void emulate_vsyscall(CPUX86State *env)
> +{
> + int syscall;
> + abi_ulong ret;
> + uint64_t caller;
> +
> + /*
> + * Validate the entry point. We have already validated the page
> + * during translation, now verify the offset.
> + */
> + switch (env->eip & ~TARGET_PAGE_MASK) {
> + case 0x000:
> + syscall = TARGET_NR_gettimeofday;
> + break;
> + case 0x400:
> + syscall = TARGET_NR_time;
> + break;
> + case 0x800:
> + syscall = TARGET_NR_getcpu;
> + break;
> + default:
> + sigsegv:
this label looks a little extraneous.
Otherwise:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/5] linux-user/i386: Emulate x86_64 vsyscalls
2020-01-20 11:48 ` Alex Bennée
@ 2020-01-21 3:38 ` Richard Henderson
2020-01-21 10:13 ` Alex Bennée
0 siblings, 1 reply; 18+ messages in thread
From: Richard Henderson @ 2020-01-21 3:38 UTC (permalink / raw)
To: Alex Bennée; +Cc: Paolo Bonzini, qemu-devel, laurent
On 1/20/20 1:48 AM, Alex Bennée wrote:
>> + default:
>> + sigsegv:
>
> this label looks a little extraneous.
>
> Otherwise:
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>
Look a little further down:
> + default:
> + sigsegv:
> + /* Like force_sig(SIGSEGV). */
> + gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0);
> + return;
> + }
> +
> + /*
> + * Validate the return address.
> + * Note that the kernel treats this the same as an invalid entry point.
> + */
> + if (get_user_u64(caller, env->regs[R_ESP])) {
> + goto sigsegv;
> + }
r~
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/5] linux-user/i386: Emulate x86_64 vsyscalls
2020-01-21 3:38 ` Richard Henderson
@ 2020-01-21 10:13 ` Alex Bennée
2020-01-21 15:51 ` Richard Henderson
0 siblings, 1 reply; 18+ messages in thread
From: Alex Bennée @ 2020-01-21 10:13 UTC (permalink / raw)
To: Richard Henderson; +Cc: Paolo Bonzini, qemu-devel, laurent
Richard Henderson <richard.henderson@linaro.org> writes:
> On 1/20/20 1:48 AM, Alex Bennée wrote:
>>> + default:
>>> + sigsegv:
>>
>> this label looks a little extraneous.
>>
>> Otherwise:
>>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>>
>
> Look a little further down:
>
>> + default:
>> + sigsegv:
>> + /* Like force_sig(SIGSEGV). */
>> + gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0);
>> + return;
>> + }
>> +
>> + /*
>> + * Validate the return address.
>> + * Note that the kernel treats this the same as an invalid entry point.
>> + */
>> + if (get_user_u64(caller, env->regs[R_ESP])) {
>> + goto sigsegv;
>> + }
Wouldn't this read better:
/*
* Validate the entry point. We have already validated the page
* during translation, now verify the offset.
*/
switch (env->eip & ~TARGET_PAGE_MASK) {
case 0x000:
syscall = TARGET_NR_gettimeofday;
break;
case 0x400:
syscall = TARGET_NR_time;
break;
case 0x800:
syscall = TARGET_NR_getcpu;
break;
default:
syscall = -1;
break;
}
/*
* If we have an invalid entry point or an invalid return address we
* generate a SIGSEG.
*/
if (syscall < 0 || get_user_u64(caller, env->regs[R_ESP])) {
gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0);
return;
}
--
Alex Bennée
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/5] linux-user/i386: Emulate x86_64 vsyscalls
2020-01-21 10:13 ` Alex Bennée
@ 2020-01-21 15:51 ` Richard Henderson
2020-01-21 16:15 ` Alex Bennée
0 siblings, 1 reply; 18+ messages in thread
From: Richard Henderson @ 2020-01-21 15:51 UTC (permalink / raw)
To: Alex Bennée; +Cc: Paolo Bonzini, qemu-devel, laurent
On 1/21/20 12:13 AM, Alex Bennée wrote:
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> On 1/20/20 1:48 AM, Alex Bennée wrote:
>>>> + default:
>>>> + sigsegv:
>>>
>>> this label looks a little extraneous.
>>>
>>> Otherwise:
>>>
>>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>>>
>>
>> Look a little further down:
>>
>>> + default:
>>> + sigsegv:
>>> + /* Like force_sig(SIGSEGV). */
>>> + gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0);
>>> + return;
>>> + }
>>> +
>>> + /*
>>> + * Validate the return address.
>>> + * Note that the kernel treats this the same as an invalid entry point.
>>> + */
>>> + if (get_user_u64(caller, env->regs[R_ESP])) {
>>> + goto sigsegv;
>>> + }
>
> Wouldn't this read better:
>
> /*
> * Validate the entry point. We have already validated the page
> * during translation, now verify the offset.
> */
> switch (env->eip & ~TARGET_PAGE_MASK) {
> case 0x000:
> syscall = TARGET_NR_gettimeofday;
> break;
> case 0x400:
> syscall = TARGET_NR_time;
> break;
> case 0x800:
> syscall = TARGET_NR_getcpu;
> break;
> default:
> syscall = -1;
> break;
> }
>
> /*
> * If we have an invalid entry point or an invalid return address we
> * generate a SIGSEG.
> */
> if (syscall < 0 || get_user_u64(caller, env->regs[R_ESP])) {
> gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0);
> return;
> }
>
Only if you have a violent goto allergy.
r~
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/5] linux-user/i386: Emulate x86_64 vsyscalls
2020-01-21 15:51 ` Richard Henderson
@ 2020-01-21 16:15 ` Alex Bennée
2020-01-21 16:23 ` Paolo Bonzini
0 siblings, 1 reply; 18+ messages in thread
From: Alex Bennée @ 2020-01-21 16:15 UTC (permalink / raw)
To: Richard Henderson; +Cc: Paolo Bonzini, qemu-devel, laurent
Richard Henderson <richard.henderson@linaro.org> writes:
> On 1/21/20 12:13 AM, Alex Bennée wrote:
>>
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>>> On 1/20/20 1:48 AM, Alex Bennée wrote:
>>>>> + default:
>>>>> + sigsegv:
>>>>
>>>> this label looks a little extraneous.
>>>>
>>>> Otherwise:
>>>>
>>>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>>>>
>>>
>>> Look a little further down:
>>>
>>>> + default:
>>>> + sigsegv:
>>>> + /* Like force_sig(SIGSEGV). */
>>>> + gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0);
>>>> + return;
>>>> + }
>>>> +
>>>> + /*
>>>> + * Validate the return address.
>>>> + * Note that the kernel treats this the same as an invalid entry point.
>>>> + */
>>>> + if (get_user_u64(caller, env->regs[R_ESP])) {
>>>> + goto sigsegv;
>>>> + }
>>
>> Wouldn't this read better:
>>
>> /*
>> * Validate the entry point. We have already validated the page
>> * during translation, now verify the offset.
>> */
>> switch (env->eip & ~TARGET_PAGE_MASK) {
>> case 0x000:
>> syscall = TARGET_NR_gettimeofday;
>> break;
>> case 0x400:
>> syscall = TARGET_NR_time;
>> break;
>> case 0x800:
>> syscall = TARGET_NR_getcpu;
>> break;
>> default:
>> syscall = -1;
>> break;
>> }
>>
>> /*
>> * If we have an invalid entry point or an invalid return address we
>> * generate a SIGSEG.
>> */
>> if (syscall < 0 || get_user_u64(caller, env->regs[R_ESP])) {
>> gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0);
>> return;
>> }
>>
>
> Only if you have a violent goto allergy.
gotos have their place but jumping backwards is confusing to eye. If the
compiler want to mess with layout after then it is free to do so.
--
Alex Bennée
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/5] linux-user/i386: Emulate x86_64 vsyscalls
2020-01-21 16:15 ` Alex Bennée
@ 2020-01-21 16:23 ` Paolo Bonzini
0 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2020-01-21 16:23 UTC (permalink / raw)
To: Alex Bennée, Richard Henderson; +Cc: qemu-devel, laurent
On 21/01/20 17:15, Alex Bennée wrote:
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> On 1/21/20 12:13 AM, Alex Bennée wrote:
>>>
>>> Richard Henderson <richard.henderson@linaro.org> writes:
>>>
>>>> On 1/20/20 1:48 AM, Alex Bennée wrote:
>>>>>> + default:
>>>>>> + sigsegv:
>>>>>
>>>>> this label looks a little extraneous.
>>>>>
>>>>> Otherwise:
>>>>>
>>>>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>>>>>
>>>>
>>>> Look a little further down:
>>>>
>>>>> + default:
>>>>> + sigsegv:
>>>>> + /* Like force_sig(SIGSEGV). */
>>>>> + gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0);
>>>>> + return;
>>>>> + }
>>>>> +
>>>>> + /*
>>>>> + * Validate the return address.
>>>>> + * Note that the kernel treats this the same as an invalid entry point.
>>>>> + */
>>>>> + if (get_user_u64(caller, env->regs[R_ESP])) {
>>>>> + goto sigsegv;
>>>>> + }
>>>
>>> Wouldn't this read better:
>>>
>>> /*
>>> * Validate the entry point. We have already validated the page
>>> * during translation, now verify the offset.
>>> */
>>> switch (env->eip & ~TARGET_PAGE_MASK) {
>>> case 0x000:
>>> syscall = TARGET_NR_gettimeofday;
>>> break;
>>> case 0x400:
>>> syscall = TARGET_NR_time;
>>> break;
>>> case 0x800:
>>> syscall = TARGET_NR_getcpu;
>>> break;
>>> default:
>>> syscall = -1;
>>> break;
>>> }
>>>
>>> /*
>>> * If we have an invalid entry point or an invalid return address we
>>> * generate a SIGSEG.
>>> */
>>> if (syscall < 0 || get_user_u64(caller, env->regs[R_ESP])) {
>>> gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0);
>>> return;
>>> }
>>>
>>
>> Only if you have a violent goto allergy.
>
> gotos have their place but jumping backwards is confusing to eye. If the
> compiler want to mess with layout after then it is free to do so.
I agree, if anything I'd place the sigsegv label at the end of the
function but Alex's version is just fine too.
Paolo
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 4/5] linux-user: Add x86_64 vsyscall page to /proc/self/maps
2020-01-16 19:43 [PATCH v2 0/5] linux-user: Implement x86_64 vsyscalls Richard Henderson
` (2 preceding siblings ...)
2020-01-16 19:43 ` [PATCH v2 3/5] linux-user/i386: Emulate x86_64 vsyscalls Richard Henderson
@ 2020-01-16 19:43 ` Richard Henderson
2020-01-17 6:39 ` Philippe Mathieu-Daudé
2020-01-20 14:41 ` Alex Bennée
2020-01-16 19:43 ` [PATCH v2 5/5] linux-user: Flush out implementation of gettimeofday Richard Henderson
` (2 subsequent siblings)
6 siblings, 2 replies; 18+ messages in thread
From: Richard Henderson @ 2020-01-16 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee, laurent
The page isn't (necessarily) present in the host /proc/self/maps,
and even if it might be it isn't present in page_flags, and even
if it was it might not have the same set of page permissions.
The easiest thing to do, particularly when it comes to the
"[vsyscall]" note at the end of line, is to special case it.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/syscall.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 171c0caef3..eb867a5296 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7005,6 +7005,15 @@ static int open_self_maps(void *cpu_env, int fd)
}
}
+#ifdef TARGET_X86_64
+ /*
+ * We only support execution from the vsyscall page.
+ * This is as if CONFIG_LEGACY_VSYSCALL_XONLY=y from v5.3.
+ */
+ dprintf(fd, "ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0"
+ " [vsyscall]\n");
+#endif
+
free(line);
fclose(fp);
--
2.20.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/5] linux-user: Add x86_64 vsyscall page to /proc/self/maps
2020-01-16 19:43 ` [PATCH v2 4/5] linux-user: Add x86_64 vsyscall page to /proc/self/maps Richard Henderson
@ 2020-01-17 6:39 ` Philippe Mathieu-Daudé
2020-01-20 14:41 ` Alex Bennée
1 sibling, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-17 6:39 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: alex.bennee, laurent
On 1/16/20 8:43 PM, Richard Henderson wrote:
> The page isn't (necessarily) present in the host /proc/self/maps,
> and even if it might be it isn't present in page_flags, and even
> if it was it might not have the same set of page permissions.
>
> The easiest thing to do, particularly when it comes to the
> "[vsyscall]" note at the end of line, is to special case it.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/syscall.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 171c0caef3..eb867a5296 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7005,6 +7005,15 @@ static int open_self_maps(void *cpu_env, int fd)
> }
> }
>
> +#ifdef TARGET_X86_64
> + /*
> + * We only support execution from the vsyscall page.
> + * This is as if CONFIG_LEGACY_VSYSCALL_XONLY=y from v5.3.
> + */
> + dprintf(fd, "ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0"
> + " [vsyscall]\n");
Can we add a definition for 0xffffffffff600000ull in the previous patch,
and use it with TARGET_PAGE_MASK here?
> +#endif
> +
> free(line);
> fclose(fp);
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/5] linux-user: Add x86_64 vsyscall page to /proc/self/maps
2020-01-16 19:43 ` [PATCH v2 4/5] linux-user: Add x86_64 vsyscall page to /proc/self/maps Richard Henderson
2020-01-17 6:39 ` Philippe Mathieu-Daudé
@ 2020-01-20 14:41 ` Alex Bennée
1 sibling, 0 replies; 18+ messages in thread
From: Alex Bennée @ 2020-01-20 14:41 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, laurent
Richard Henderson <richard.henderson@linaro.org> writes:
> The page isn't (necessarily) present in the host /proc/self/maps,
> and even if it might be it isn't present in page_flags, and even
> if it was it might not have the same set of page permissions.
>
> The easiest thing to do, particularly when it comes to the
> "[vsyscall]" note at the end of line, is to special case it.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/syscall.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 171c0caef3..eb867a5296 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7005,6 +7005,15 @@ static int open_self_maps(void *cpu_env, int fd)
> }
> }
>
> +#ifdef TARGET_X86_64
> + /*
> + * We only support execution from the vsyscall page.
> + * This is as if CONFIG_LEGACY_VSYSCALL_XONLY=y from v5.3.
> + */
> + dprintf(fd, "ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0"
> + " [vsyscall]\n");
> +#endif
> +
I'm not sure what whitespace is wrong - tabs maybe? But looking at a
real vsyscall system vs emulated things line up better:
Welcome to Buildroot
buildroot login: root
# uname -a
Linux buildroot 5.4.13 #1 SMP Mon Jan 20 14:36:20 GMT 2020 x86_64 GNU/Linux
# cat /proc/self/maps
00400000-004ab000 r-xp 00000000 00:02 7635 /bin/busybox
004ab000-004ac000 r-xp 000aa000 00:02 7635 /bin/busybox
004ac000-004ad000 rwxp 000ab000 00:02 7635 /bin/busybox
004ad000-004ae000 rwxp 00000000 00:00 0
7fcc91566000-7fcc91567000 rwxp 00000000 00:00 0
7fcc91567000-7fcc915de000 r-xp 00000000 00:02 7475 /lib/libuClibc-1.0.32.so
7fcc915de000-7fcc915df000 ---p 00000000 00:00 0
7fcc915df000-7fcc915e0000 r-xp 00077000 00:02 7475 /lib/libuClibc-1.0.32.so
7fcc915e0000-7fcc915e1000 rwxp 00078000 00:02 7475 /lib/libuClibc-1.0.32.so
7fcc915e1000-7fcc915fc000 rwxp 00000000 00:00 0
7fcc915fc000-7fcc91603000 r-xp 00000000 00:02 7480 /lib/ld64-uClibc-1.0.32.so
7fcc91603000-7fcc91604000 r-xp 00006000 00:02 7480 /lib/ld64-uClibc-1.0.32.so
7fcc91604000-7fcc91605000 rwxp 00007000 00:02 7480 /lib/ld64-uClibc-1.0.32.so
7ffd92001000-7ffd92022000 rwxp 00000000 00:00 0 [stack]
7ffd920c3000-7ffd920c6000 r--p 00000000 00:00 0 [vvar]
7ffd920c6000-7ffd920c7000 r-xp 00000000 00:00 0 [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall]
# QEMU 4.2.50 monitor - type 'help' for more information
(qemu) quit
14:38:52 [alex.bennee@hackbox2:~/l/q/b/all] review/fix-user-brk|… + ./x86_64-linux-user/qemu-x86_64 /bin/cat /proc/self/maps
4000000000-4000008000 r-xp 00000000 00:19 8131 /bin/cat
4000008000-4000207000 ---p 00000000 00:00 0
4000207000-4000208000 r--p 00007000 00:19 8131 /bin/cat
4000208000-4000209000 rw-p 00008000 00:19 8131 /bin/cat
4000209000-400022a000 rw-p 00000000 00:00 0
4001209000-400120a000 ---p 00000000 00:00 0
400120a000-4001a0a000 rw-p 00000000 00:00 0 [stack]
4001a0a000-4001a31000 r-xp 00000000 00:19 3644 /lib/x86_64-linux-gnu/ld-2.27.so
4001a31000-4001c31000 ---p 00000000 00:00 0
4001c31000-4001c32000 r--p 00027000 00:19 3644 /lib/x86_64-linux-gnu/ld-2.27.so
4001c32000-4001c33000 rw-p 00028000 00:19 3644 /lib/x86_64-linux-gnu/ld-2.27.so
4001c33000-4001c36000 rw-p 00000000 00:00 0
4001c6d000-4001e54000 r-xp 00000000 00:19 3648 /lib/x86_64-linux-gnu/libc-2.27.so
4001e54000-4002054000 ---p 001e7000 00:19 3648 /lib/x86_64-linux-gnu/libc-2.27.so
4002054000-4002058000 r--p 001e7000 00:19 3648 /lib/x86_64-linux-gnu/libc-2.27.so
4002058000-400205a000 rw-p 001eb000 00:19 3648 /lib/x86_64-linux-gnu/libc-2.27.so
400205a000-4002060000 rw-p 00000000 00:00 0
4002060000-40023d2000 r--p 00000000 00:19 195276 /usr/lib/locale/locale-archive
ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0 [vsyscall]
> free(line);
> fclose(fp);
--
Alex Bennée
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 5/5] linux-user: Flush out implementation of gettimeofday
2020-01-16 19:43 [PATCH v2 0/5] linux-user: Implement x86_64 vsyscalls Richard Henderson
` (3 preceding siblings ...)
2020-01-16 19:43 ` [PATCH v2 4/5] linux-user: Add x86_64 vsyscall page to /proc/self/maps Richard Henderson
@ 2020-01-16 19:43 ` Richard Henderson
2020-01-17 6:37 ` Philippe Mathieu-Daudé
2020-01-20 13:27 ` Alex Bennée
2020-01-16 19:46 ` [PATCH v2 0/5] linux-user: Implement x86_64 vsyscalls Richard Henderson
2020-01-28 17:26 ` Laurent Vivier
6 siblings, 2 replies; 18+ messages in thread
From: Richard Henderson @ 2020-01-16 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee, laurent
The first argument, timeval, is allowed to be NULL.
The second argument, timezone, was missing. While its use is
deprecated, it is still present in the syscall.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/syscall.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index eb867a5296..628b4de9a1 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1219,6 +1219,23 @@ static inline abi_long host_to_target_timespec64(abi_ulong target_addr,
return 0;
}
+static inline abi_long copy_to_user_timezone(abi_ulong target_tz_addr,
+ struct timezone *tz)
+{
+ struct target_timezone *target_tz;
+
+ if (!lock_user_struct(VERIFY_WRITE, target_tz, target_tz_addr, 1)) {
+ return -TARGET_EFAULT;
+ }
+
+ __put_user(tz->tz_minuteswest, &target_tz->tz_minuteswest);
+ __put_user(tz->tz_dsttime, &target_tz->tz_dsttime);
+
+ unlock_user_struct(target_tz, target_tz_addr, 1);
+
+ return 0;
+}
+
static inline abi_long copy_from_user_timezone(struct timezone *tz,
abi_ulong target_tz_addr)
{
@@ -8567,10 +8584,16 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
case TARGET_NR_gettimeofday:
{
struct timeval tv;
- ret = get_errno(gettimeofday(&tv, NULL));
+ struct timezone tz;
+
+ ret = get_errno(gettimeofday(&tv, &tz));
if (!is_error(ret)) {
- if (copy_to_user_timeval(arg1, &tv))
+ if (arg1 && copy_to_user_timeval(arg1, &tv)) {
return -TARGET_EFAULT;
+ }
+ if (arg2 && copy_to_user_timezone(arg2, &tz)) {
+ return -TARGET_EFAULT;
+ }
}
}
return ret;
--
2.20.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 5/5] linux-user: Flush out implementation of gettimeofday
2020-01-16 19:43 ` [PATCH v2 5/5] linux-user: Flush out implementation of gettimeofday Richard Henderson
@ 2020-01-17 6:37 ` Philippe Mathieu-Daudé
2020-01-20 13:27 ` Alex Bennée
1 sibling, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-17 6:37 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: alex.bennee, laurent
On 1/16/20 8:43 PM, Richard Henderson wrote:
> The first argument, timeval, is allowed to be NULL.
>
> The second argument, timezone, was missing. While its use is
> deprecated, it is still present in the syscall.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/syscall.c | 27 +++++++++++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index eb867a5296..628b4de9a1 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1219,6 +1219,23 @@ static inline abi_long host_to_target_timespec64(abi_ulong target_addr,
> return 0;
> }
>
> +static inline abi_long copy_to_user_timezone(abi_ulong target_tz_addr,
> + struct timezone *tz)
> +{
> + struct target_timezone *target_tz;
> +
> + if (!lock_user_struct(VERIFY_WRITE, target_tz, target_tz_addr, 1)) {
> + return -TARGET_EFAULT;
> + }
> +
> + __put_user(tz->tz_minuteswest, &target_tz->tz_minuteswest);
> + __put_user(tz->tz_dsttime, &target_tz->tz_dsttime);
> +
> + unlock_user_struct(target_tz, target_tz_addr, 1);
> +
> + return 0;
> +}
> +
> static inline abi_long copy_from_user_timezone(struct timezone *tz,
> abi_ulong target_tz_addr)
> {
> @@ -8567,10 +8584,16 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
> case TARGET_NR_gettimeofday:
> {
> struct timeval tv;
> - ret = get_errno(gettimeofday(&tv, NULL));
> + struct timezone tz;
> +
> + ret = get_errno(gettimeofday(&tv, &tz));
> if (!is_error(ret)) {
> - if (copy_to_user_timeval(arg1, &tv))
> + if (arg1 && copy_to_user_timeval(arg1, &tv)) {
> return -TARGET_EFAULT;
> + }
> + if (arg2 && copy_to_user_timezone(arg2, &tz)) {
> + return -TARGET_EFAULT;
> + }
> }
> }
> return ret;
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 5/5] linux-user: Flush out implementation of gettimeofday
2020-01-16 19:43 ` [PATCH v2 5/5] linux-user: Flush out implementation of gettimeofday Richard Henderson
2020-01-17 6:37 ` Philippe Mathieu-Daudé
@ 2020-01-20 13:27 ` Alex Bennée
1 sibling, 0 replies; 18+ messages in thread
From: Alex Bennée @ 2020-01-20 13:27 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, laurent
Richard Henderson <richard.henderson@linaro.org> writes:
> The first argument, timeval, is allowed to be NULL.
>
> The second argument, timezone, was missing. While its use is
> deprecated, it is still present in the syscall.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> linux-user/syscall.c | 27 +++++++++++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index eb867a5296..628b4de9a1 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1219,6 +1219,23 @@ static inline abi_long host_to_target_timespec64(abi_ulong target_addr,
> return 0;
> }
>
> +static inline abi_long copy_to_user_timezone(abi_ulong target_tz_addr,
> + struct timezone *tz)
> +{
> + struct target_timezone *target_tz;
> +
> + if (!lock_user_struct(VERIFY_WRITE, target_tz, target_tz_addr, 1)) {
> + return -TARGET_EFAULT;
> + }
> +
> + __put_user(tz->tz_minuteswest, &target_tz->tz_minuteswest);
> + __put_user(tz->tz_dsttime, &target_tz->tz_dsttime);
> +
> + unlock_user_struct(target_tz, target_tz_addr, 1);
> +
> + return 0;
> +}
> +
> static inline abi_long copy_from_user_timezone(struct timezone *tz,
> abi_ulong target_tz_addr)
> {
> @@ -8567,10 +8584,16 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
> case TARGET_NR_gettimeofday:
> {
> struct timeval tv;
> - ret = get_errno(gettimeofday(&tv, NULL));
> + struct timezone tz;
> +
> + ret = get_errno(gettimeofday(&tv, &tz));
> if (!is_error(ret)) {
> - if (copy_to_user_timeval(arg1, &tv))
> + if (arg1 && copy_to_user_timeval(arg1, &tv)) {
> return -TARGET_EFAULT;
> + }
> + if (arg2 && copy_to_user_timezone(arg2, &tz)) {
> + return -TARGET_EFAULT;
> + }
> }
> }
> return ret;
--
Alex Bennée
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/5] linux-user: Implement x86_64 vsyscalls
2020-01-16 19:43 [PATCH v2 0/5] linux-user: Implement x86_64 vsyscalls Richard Henderson
` (4 preceding siblings ...)
2020-01-16 19:43 ` [PATCH v2 5/5] linux-user: Flush out implementation of gettimeofday Richard Henderson
@ 2020-01-16 19:46 ` Richard Henderson
2020-01-28 17:26 ` Laurent Vivier
6 siblings, 0 replies; 18+ messages in thread
From: Richard Henderson @ 2020-01-16 19:46 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee, laurent
On 1/16/20 9:43 AM, Richard Henderson wrote:
> Changes since v2:
>
> * Add /proc/self/maps line
>
> I'm not sure this is really necessary. The linux kernel
> self-test checks for it, and modifies the set of tests that
> it runs based on it. But otherwise I think it's unused.
>
> * Fix errors in base gettimeofday syscall
>
> This is also checked by test_vsyscall, as noticed by AJB.
Oh, and
* Set error_code in write_ok_or_segv (patch 2)
r~
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/5] linux-user: Implement x86_64 vsyscalls
2020-01-16 19:43 [PATCH v2 0/5] linux-user: Implement x86_64 vsyscalls Richard Henderson
` (5 preceding siblings ...)
2020-01-16 19:46 ` [PATCH v2 0/5] linux-user: Implement x86_64 vsyscalls Richard Henderson
@ 2020-01-28 17:26 ` Laurent Vivier
6 siblings, 0 replies; 18+ messages in thread
From: Laurent Vivier @ 2020-01-28 17:26 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: alex.bennee
Le 16/01/2020 à 20:43, Richard Henderson a écrit :
> Changes since v2:
>
> * Add /proc/self/maps line
>
> I'm not sure this is really necessary. The linux kernel
> self-test checks for it, and modifies the set of tests that
> it runs based on it. But otherwise I think it's unused.
>
> * Fix errors in base gettimeofday syscall
>
> This is also checked by test_vsyscall, as noticed by AJB.
>
Hi Richard,
do you plan to send a v3?
Thanks,
Laurent
> r~
>
>
> Original blurb:
>
> The x86_64 abi has a legacy vsyscall page. The kernel folk
> have been trying to deprecate this since at least v3.1, but
>
> (1) We don't implement the vdso that replaces vsyscalls,
> (2) As of v5.5, the vsyscall page is still enabled by default.
>
> This lack is affecting Peter's linux-user testing.
>
> The dependency is not obvious because Peter is running the tests
> on x86_64, so the host is providing a vsyscall page to qemu.
>
> Because of how user-only memory operations are handled, with no
> validation of guest vs host pages, so long as qemu chooses to
> run with guest_base == 0, the guest may Just So Happen to read
> the host's vsyscall page.
>
> Complicating this, new OS releases may use a kernel configured
> with CONFIG_LEGACY_VSYSCALL_XONLY=y, which means the the vsyscall
> page cannot be read, only executed. Which means that the guest
> then cannot read the host vsyscall page during translation and
> will SIGSEGV.
>
> Exactly which of these many variables is affecting Peter's testing
> with Ubuntu 18.04 of my TCG merge, I'm not exactly sure. I suspect
> that it is the change to drop the textseg_addr adjustment to user-only
> static binaries. IIRC bionic does not support -static-pie, which is
> the preferred replacement. This could mean that the host and guest
> binaries overlap, which leads to guest_base != 0.
>
> I vaguely remember someone (Paolo?) implementing something like
> this many years ago, but clearly it never got merged.
>
> In any case, this emulation has been missing for too long.
>
>
> Richard Henderson (5):
> target/i386: Renumber EXCP_SYSCALL
> linux-user/i386: Split out gen_signal
> linux-user/i386: Emulate x86_64 vsyscalls
> linux-user: Add x86_64 vsyscall page to /proc/self/maps
> linux-user: Flush out implementation of gettimeofday
>
> target/i386/cpu.h | 6 +-
> linux-user/i386/cpu_loop.c | 198 ++++++++++++++++++++++++++-----------
> linux-user/syscall.c | 36 ++++++-
> target/i386/translate.c | 16 ++-
> 4 files changed, 190 insertions(+), 66 deletions(-)
>
^ permalink raw reply [flat|nested] 18+ messages in thread