1. Patchew
  2. QEMU
  3. View series

[PATCH] linux-user: fix bug about incorrect base addresss of gdt on i386 and x86_64

fanwj@mail.ustc.edu.cn posted 1 patch 1 year, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/4172b90.58b08.18631b77860.Coremail.fanwj@mail.ustc.edu.cn
Maintainers: Laurent Vivier <laurent@vivier.eu>
linux-user/i386/cpu_loop.c | 9 +++++++++
linux-user/main.c          | 7 +++++++
2 files changed, 16 insertions(+)
[PATCH] linux-user: fix bug about incorrect base addresss of gdt on i386 and x86_64
Posted by fanwj@mail.ustc.edu.cn 1 year, 2 months ago 
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
Re: [PATCH] linux-user: fix bug about incorrect base addresss of gdt on i386 and x86_64
Posted by Laurent Vivier 1 year, 1 month ago 
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
Re: [PATCH] linux-user: fix bug about incorrect base addresss of gdt on i386 and x86_64
Posted by Laurent Vivier 1 year, 1 month ago 
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
Re: [PATCH] linux-user: fix bug about incorrect base addresss of gdt on i386 and x86_64
Posted by Richard Henderson 1 year, 1 month ago 
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~

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.