[v5,07/16] target/i386: Allow setting of R_LDTR and R_TR with cpu_x86_load_seg_cache()

Commit Message

Roy Hopkins Aug. 13, 2024, 3:01 p.m. UTC

The x86 segment registers are identified by the X86Seg enumeration which
includes LDTR and TR as well as the normal segment registers. The
function 'cpu_x86_load_seg_cache()' uses the enum to determine which
segment to set. However, specifying R_LDTR or R_TR results in an
out-of-bounds access of the segment array.

Possibly by coincidence, the function does correctly set LDTR or TR in
this case as the structures for these registers immediately follow the
array which is accessed out of bounds.

This patch adds correct handling for R_LDTR and R_TR in the function.

Signed-off-by: Roy Hopkins <[e-mail]>
Reviewed-by: Michael S. Tsirkin <[e-mail]>
---
 target/i386/cpu.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Stefano Garzarella Sept. 2, 2024, 2:19 p.m. UTC | #1

On Tue, Aug 13, 2024 at 04:01:09PM GMT, Roy Hopkins wrote:
>The x86 segment registers are identified by the X86Seg enumeration which
>includes LDTR and TR as well as the normal segment registers. The
>function 'cpu_x86_load_seg_cache()' uses the enum to determine which
>segment to set. However, specifying R_LDTR or R_TR results in an
>out-of-bounds access of the segment array.
>
>Possibly by coincidence, the function does correctly set LDTR or TR in
>this case as the structures for these registers immediately follow the
>array which is accessed out of bounds.
>
>This patch adds correct handling for R_LDTR and R_TR in the function.
>
>Signed-off-by: Roy Hopkins <[e-mail]>
>Reviewed-by: Michael S. Tsirkin <[e-mail]>
>---
> target/i386/cpu.h | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)

Reviewed-by: Stefano Garzarella <[e-mail]>

>
>diff --git a/target/i386/cpu.h b/target/i386/cpu.h
>index c6cc035df3..227bf2600a 100644
>--- a/target/i386/cpu.h
>+++ b/target/i386/cpu.h
>@@ -2256,7 +2256,14 @@ static inline void cpu_x86_load_seg_cache(CPUX86State *env,
>     SegmentCache *sc;
>     unsigned int new_hflags;
>
>-    sc = &env->segs[seg_reg];
>+    if (seg_reg == R_LDTR) {
>+        sc = &env->ldt;
>+    } else if (seg_reg == R_TR) {
>+        sc = &env->tr;
>+    } else {
>+        sc = &env->segs[seg_reg];
>+    }
>+
>     sc->selector = selector;
>     sc->base = base;
>     sc->limit = limit;
>-- 
>2.43.0
>

Patch

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index c6cc035df3..227bf2600a 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2256,7 +2256,14 @@  static inline void cpu_x86_load_seg_cache(CPUX86State *env,
     SegmentCache *sc;
     unsigned int new_hflags;
 
-    sc = &env->segs[seg_reg];
+    if (seg_reg == R_LDTR) {
+        sc = &env->ldt;
+    } else if (seg_reg == R_TR) {
+        sc = &env->tr;
+    } else {
+        sc = &env->segs[seg_reg];
+    }
+
     sc->selector = selector;
     sc->base = base;
     sc->limit = limit;