* [PATCH] linux-user: fix bug about incorrect base addresss of gdt on i386 and x86_64
@ 2023-02-08 15:49 fanwj
2023-03-07 14:30 ` Laurent Vivier
2023-03-07 18:45 ` Laurent Vivier
0 siblings, 2 replies; 6+ messages in thread
From: fanwj @ 2023-02-08 15:49 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, peter.maydell
On linux user mode, CPUX86State::gdt::base from Different CPUX86State Objects have same value, It is incorrect! Every CPUX86State::gdt::base Must points to independent memory space.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1405
Signed-off-by: fanwenjie <fanwj@mail.ustc.edu.cn>
---
linux-user/i386/cpu_loop.c | 9 +++++++++
linux-user/main.c | 7 +++++++
2 files changed, 16 insertions(+)
diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
index 865413c..48511cd 100644
--- a/linux-user/i386/cpu_loop.c
+++ b/linux-user/i386/cpu_loop.c
@@ -314,8 +314,17 @@ void cpu_loop(CPUX86State *env)
}
}
+static void target_cpu_free(void *obj)
+{
+ CPUArchState* env = ((CPUState*)obj)->env_ptr;
+ target_munmap(env->gdt.base, sizeof(uint64_t) * TARGET_GDT_ENTRIES);
+ g_free(obj);
+}
+
void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
{
+ CPUState* cpu = env_cpu(env);
+ OBJECT(cpu)->free = target_cpu_free;
env->cr[0] = CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK;
env->hflags |= HF_PE_MASK | HF_CPL_MASK;
if (env->features[FEAT_1_EDX] & CPUID_SSE) {
diff --git a/linux-user/main.c b/linux-user/main.c
index a17fed0..3acd9b4 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -234,6 +234,13 @@ CPUArchState *cpu_copy(CPUArchState *env)
new_cpu->tcg_cflags = cpu->tcg_cflags;
memcpy(new_env, env, sizeof(CPUArchState));
+#if defined(TARGET_I386) || defined(TARGET_X86_64)
+ new_env->gdt.base = target_mmap(0, sizeof(uint64_t) * TARGET_GDT_ENTRIES,
+ PROT_READ|PROT_WRITE,
+ MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+ memcpy((void*)g2h_untagged(new_env->gdt.base), (void*)g2h_untagged(env->gdt.base), sizeof(uint64_t) * TARGET_GDT_ENTRIES);
+ OBJECT(new_cpu)->free = OBJECT(cpu)->free;
+#endif
/* Clone all break/watchpoints.
Note: Once we support ptrace with hw-debug register access, make sure
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] linux-user: fix bug about incorrect base addresss of gdt on i386 and x86_64
2023-02-08 15:49 [PATCH] linux-user: fix bug about incorrect base addresss of gdt on i386 and x86_64 fanwj
@ 2023-03-07 14:30 ` Laurent Vivier
2023-03-07 18:20 ` Richard Henderson
2023-03-07 18:45 ` Laurent Vivier
1 sibling, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2023-03-07 14:30 UTC (permalink / raw)
To: Richard Henderson; +Cc: peter.maydell, fanwj, qemu-devel
Richard,
do you think it's correct?
Thanks,
Laurent
Le 08/02/2023 à 16:49, fanwj@mail.ustc.edu.cn a écrit :
> On linux user mode, CPUX86State::gdt::base from Different CPUX86State Objects have same value, It is incorrect! Every CPUX86State::gdt::base Must points to independent memory space.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1405
> Signed-off-by: fanwenjie <fanwj@mail.ustc.edu.cn>
> ---
> linux-user/i386/cpu_loop.c | 9 +++++++++
> linux-user/main.c | 7 +++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
> index 865413c..48511cd 100644
> --- a/linux-user/i386/cpu_loop.c
> +++ b/linux-user/i386/cpu_loop.c
> @@ -314,8 +314,17 @@ void cpu_loop(CPUX86State *env)
> }
> }
>
> +static void target_cpu_free(void *obj)
> +{
> + CPUArchState* env = ((CPUState*)obj)->env_ptr;
> + target_munmap(env->gdt.base, sizeof(uint64_t) * TARGET_GDT_ENTRIES);
> + g_free(obj);
> +}
> +
> void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
> {
> + CPUState* cpu = env_cpu(env);
> + OBJECT(cpu)->free = target_cpu_free;
> env->cr[0] = CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK;
> env->hflags |= HF_PE_MASK | HF_CPL_MASK;
> if (env->features[FEAT_1_EDX] & CPUID_SSE) {
> diff --git a/linux-user/main.c b/linux-user/main.c
> index a17fed0..3acd9b4 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -234,6 +234,13 @@ CPUArchState *cpu_copy(CPUArchState *env)
>
> new_cpu->tcg_cflags = cpu->tcg_cflags;
> memcpy(new_env, env, sizeof(CPUArchState));
> +#if defined(TARGET_I386) || defined(TARGET_X86_64)
> + new_env->gdt.base = target_mmap(0, sizeof(uint64_t) * TARGET_GDT_ENTRIES,
> + PROT_READ|PROT_WRITE,
> + MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> + memcpy((void*)g2h_untagged(new_env->gdt.base), (void*)g2h_untagged(env->gdt.base), sizeof(uint64_t) * TARGET_GDT_ENTRIES);
> + OBJECT(new_cpu)->free = OBJECT(cpu)->free;
> +#endif
>
> /* Clone all break/watchpoints.
> Note: Once we support ptrace with hw-debug register access, make sure
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] linux-user: fix bug about incorrect base addresss of gdt on i386 and x86_64
2023-03-07 14:30 ` Laurent Vivier
@ 2023-03-07 18:20 ` Richard Henderson
0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2023-03-07 18:20 UTC (permalink / raw)
To: Laurent Vivier; +Cc: peter.maydell, fanwj, qemu-devel
On 3/7/23 06:30, Laurent Vivier wrote:
> Richard,
>
> do you think it's correct?
It's correct enough, until target/i386 is fixed to not require the GDT/LDT to be
incorrectly mapped in the (ring 3) user address space.
You may wish to fix a few nits when applying:
>> }
>> +static void target_cpu_free(void *obj)
Missing line before function.
>> +#if defined(TARGET_I386) || defined(TARGET_X86_64)
>> + new_env->gdt.base = target_mmap(0, sizeof(uint64_t) * TARGET_GDT_ENTRIES,
>> + PROT_READ|PROT_WRITE,
>> + MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
>> + memcpy((void*)g2h_untagged(new_env->gdt.base), (void*)g2h_untagged(env->gdt.base),
>> sizeof(uint64_t) * TARGET_GDT_ENTRIES);
Unnecessary casts, overlong line.
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] linux-user: fix bug about incorrect base addresss of gdt on i386 and x86_64
2023-02-08 15:49 [PATCH] linux-user: fix bug about incorrect base addresss of gdt on i386 and x86_64 fanwj
2023-03-07 14:30 ` Laurent Vivier
@ 2023-03-07 18:45 ` Laurent Vivier
1 sibling, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2023-03-07 18:45 UTC (permalink / raw)
To: fanwj, qemu-devel; +Cc: peter.maydell, Richard Henderson
Le 08/02/2023 à 16:49, fanwj@mail.ustc.edu.cn a écrit :
> On linux user mode, CPUX86State::gdt::base from Different CPUX86State Objects have same value, It is incorrect! Every CPUX86State::gdt::base Must points to independent memory space.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1405
> Signed-off-by: fanwenjie <fanwj@mail.ustc.edu.cn>
> ---
> linux-user/i386/cpu_loop.c | 9 +++++++++
> linux-user/main.c | 7 +++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
> index 865413c..48511cd 100644
> --- a/linux-user/i386/cpu_loop.c
> +++ b/linux-user/i386/cpu_loop.c
> @@ -314,8 +314,17 @@ void cpu_loop(CPUX86State *env)
> }
> }
>
> +static void target_cpu_free(void *obj)
> +{
> + CPUArchState* env = ((CPUState*)obj)->env_ptr;
> + target_munmap(env->gdt.base, sizeof(uint64_t) * TARGET_GDT_ENTRIES);
> + g_free(obj);
> +}
> +
> void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
> {
> + CPUState* cpu = env_cpu(env);
> + OBJECT(cpu)->free = target_cpu_free;
> env->cr[0] = CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK;
> env->hflags |= HF_PE_MASK | HF_CPL_MASK;
> if (env->features[FEAT_1_EDX] & CPUID_SSE) {
> diff --git a/linux-user/main.c b/linux-user/main.c
> index a17fed0..3acd9b4 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -234,6 +234,13 @@ CPUArchState *cpu_copy(CPUArchState *env)
>
> new_cpu->tcg_cflags = cpu->tcg_cflags;
> memcpy(new_env, env, sizeof(CPUArchState));
> +#if defined(TARGET_I386) || defined(TARGET_X86_64)
> + new_env->gdt.base = target_mmap(0, sizeof(uint64_t) * TARGET_GDT_ENTRIES,
> + PROT_READ|PROT_WRITE,
> + MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> + memcpy((void*)g2h_untagged(new_env->gdt.base), (void*)g2h_untagged(env->gdt.base), sizeof(uint64_t) * TARGET_GDT_ENTRIES);
> + OBJECT(new_cpu)->free = OBJECT(cpu)->free;
> +#endif
>
> /* Clone all break/watchpoints.
> Note: Once we support ptrace with hw-debug register access, make sure
Applied to my linux-user-for-8.0 branch.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] linux-user: fix bug about incorrect base addresss of gdt on i386 and x86_64
2023-01-03 11:38 fanwenjie
@ 2023-01-15 5:32 ` Richard Henderson
0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2023-01-15 5:32 UTC (permalink / raw)
To: fanwenjie, qemu-devel; +Cc: laurent
On 1/3/23 01:38, fanwenjie wrote:
> On linux user mode, CPUX86State::gdt::base from Different CPUX86State Objects have same value, It is incorrect! Every CPUX86State::gdt::base Must points to independent memory space.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1405
> Signed-off-by: fanwenjie <fanwj@mail.ustc.edu.cn>
> ---
> linux-user/i386/cpu_loop.c | 9 +++++++++
> linux-user/main.c | 7 +++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
> index 865413c..48511cd 100644
> --- a/linux-user/i386/cpu_loop.c
> +++ b/linux-user/i386/cpu_loop.c
> @@ -314,8 +314,17 @@ void cpu_loop(CPUX86State *env)
> }
> }
>
> +static void target_cpu_free(void *obj)
> +{
> + CPUArchState* env = ((CPUState*)obj)->env_ptr;
> + target_munmap(env->gdt.base, sizeof(uint64_t) * TARGET_GDT_ENTRIES);
> + g_free(obj);
> +}
> +
> void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
> {
> + CPUState* cpu = env_cpu(env);
> + OBJECT(cpu)->free = target_cpu_free;
> env->cr[0] = CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK;
> env->hflags |= HF_PE_MASK | HF_CPL_MASK;
> if (env->features[FEAT_1_EDX] & CPUID_SSE) {
> diff --git a/linux-user/main.c b/linux-user/main.c
> index a17fed0..3acd9b4 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -234,6 +234,13 @@ CPUArchState *cpu_copy(CPUArchState *env)
>
> new_cpu->tcg_cflags = cpu->tcg_cflags;
> memcpy(new_env, env, sizeof(CPUArchState));
> +#if defined(TARGET_I386) || defined(TARGET_X86_64)
> + new_env->gdt.base = target_mmap(0, sizeof(uint64_t) * TARGET_GDT_ENTRIES,
> + PROT_READ|PROT_WRITE,
> + MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> + memcpy((void*)new_env->gdt.base, (void*)env->gdt.base, sizeof(uint64_t) * TARGET_GDT_ENTRIES);
> + OBJECT(new_cpu)->free = OBJECT(cpu)->free;
> +#endif
This isn't a fantastic solution, because neither the ldt nor the gdt should be mapped into
the user address space -- these are kernel private data structures. But cpu.h uses a
target_ulong, and seg_helper.c is set up to load data from the guest, and it would be a
medium sized job to address that.
The memcpy is definitely wrong, because you're casting a guest address into a host
address, which is incorrect. You have to use g2h().
I'm actually surprised that you need this for TARGET_X86_64 at all -- the two TLS segments
don't really use the GDT at all, since fs_base and gs_base may be set directly.
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] linux-user: fix bug about incorrect base addresss of gdt on i386 and x86_64
@ 2023-01-03 11:38 fanwenjie
2023-01-15 5:32 ` Richard Henderson
0 siblings, 1 reply; 6+ messages in thread
From: fanwenjie @ 2023-01-03 11:38 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, fanwenjie
On linux user mode, CPUX86State::gdt::base from Different CPUX86State Objects have same value, It is incorrect! Every CPUX86State::gdt::base Must points to independent memory space.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1405
Signed-off-by: fanwenjie <fanwj@mail.ustc.edu.cn>
---
linux-user/i386/cpu_loop.c | 9 +++++++++
linux-user/main.c | 7 +++++++
2 files changed, 16 insertions(+)
diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
index 865413c..48511cd 100644
--- a/linux-user/i386/cpu_loop.c
+++ b/linux-user/i386/cpu_loop.c
@@ -314,8 +314,17 @@ void cpu_loop(CPUX86State *env)
}
}
+static void target_cpu_free(void *obj)
+{
+ CPUArchState* env = ((CPUState*)obj)->env_ptr;
+ target_munmap(env->gdt.base, sizeof(uint64_t) * TARGET_GDT_ENTRIES);
+ g_free(obj);
+}
+
void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
{
+ CPUState* cpu = env_cpu(env);
+ OBJECT(cpu)->free = target_cpu_free;
env->cr[0] = CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK;
env->hflags |= HF_PE_MASK | HF_CPL_MASK;
if (env->features[FEAT_1_EDX] & CPUID_SSE) {
diff --git a/linux-user/main.c b/linux-user/main.c
index a17fed0..3acd9b4 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -234,6 +234,13 @@ CPUArchState *cpu_copy(CPUArchState *env)
new_cpu->tcg_cflags = cpu->tcg_cflags;
memcpy(new_env, env, sizeof(CPUArchState));
+#if defined(TARGET_I386) || defined(TARGET_X86_64)
+ new_env->gdt.base = target_mmap(0, sizeof(uint64_t) * TARGET_GDT_ENTRIES,
+ PROT_READ|PROT_WRITE,
+ MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+ memcpy((void*)new_env->gdt.base, (void*)env->gdt.base, sizeof(uint64_t) * TARGET_GDT_ENTRIES);
+ OBJECT(new_cpu)->free = OBJECT(cpu)->free;
+#endif
/* Clone all break/watchpoints.
Note: Once we support ptrace with hw-debug register access, make sure
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-07 18:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-08 15:49 [PATCH] linux-user: fix bug about incorrect base addresss of gdt on i386 and x86_64 fanwj
2023-03-07 14:30 ` Laurent Vivier
2023-03-07 18:20 ` Richard Henderson
2023-03-07 18:45 ` Laurent Vivier
-- strict thread matches above, loose matches on Subject: below --
2023-01-03 11:38 fanwenjie
2023-01-15 5:32 ` Richard Henderson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.