[Bug 1879955] [NEW] target/i386/seg_helper.c: 16-bit TSS struct format wrong?

* [Bug 1879955] [NEW] target/i386/seg_helper.c: 16-bit TSS struct format wrong?
@ 2020-05-21 13:41 Peter Maydell
  2021-05-15  9:38 ` [Bug 1879955] " Thomas Huth
  2021-06-01 10:33 ` Thomas Huth
  0 siblings, 2 replies; 3+ messages in thread
From: Peter Maydell @ 2020-05-21 13:41 UTC (permalink / raw)
  To: qemu-devel

Public bug reported:

In target/i386/seg_helper.c:switch_tss_ra() we have the following code
to load registers from a 16-bit TSS struct:

        /* 16 bit */
        new_cr3 = 0;
        new_eip = cpu_lduw_kernel_ra(env, tss_base + 0x0e, retaddr);
        new_eflags = cpu_lduw_kernel_ra(env, tss_base + 0x10, retaddr);
        for (i = 0; i < 8; i++) {
            new_regs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x12 + i * 2),
                                             retaddr) | 0xffff0000;
        }
        for (i = 0; i < 4; i++) {
            new_segs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x22 + i * 4),
                                             retaddr);
        }
        new_ldt = cpu_lduw_kernel_ra(env, tss_base + 0x2a, retaddr);

This doesn't match up with the structure described here:
https://www.sandpile.org/x86/tss.htm -- which has only 2-byte slots for
the segment registers. It also makes the 3rd segreg use the same offset
as the LDTR, which is very suspicious. I suspect that this should use
"(0x22 + i * 2)".

The code later in the same function that stores the segment registers to
the struct has the same bug.

Found by code inspection; I don't have a test case to check this. As a
non-x86-expert I'm just going to file a bug report in case somebody else
feels like confirming the issue and sending a patch.

** Affects: qemu
     Importance: Undecided
         Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1879955

Title:
  target/i386/seg_helper.c: 16-bit TSS struct format wrong?

Status in QEMU:
  New

Bug description:
  In target/i386/seg_helper.c:switch_tss_ra() we have the following code
  to load registers from a 16-bit TSS struct:

          /* 16 bit */
          new_cr3 = 0;
          new_eip = cpu_lduw_kernel_ra(env, tss_base + 0x0e, retaddr);
          new_eflags = cpu_lduw_kernel_ra(env, tss_base + 0x10, retaddr);
          for (i = 0; i < 8; i++) {
              new_regs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x12 + i * 2),
                                               retaddr) | 0xffff0000;
          }
          for (i = 0; i < 4; i++) {
              new_segs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x22 + i * 4),
                                               retaddr);
          }
          new_ldt = cpu_lduw_kernel_ra(env, tss_base + 0x2a, retaddr);

  This doesn't match up with the structure described here:
  https://www.sandpile.org/x86/tss.htm -- which has only 2-byte slots
  for the segment registers. It also makes the 3rd segreg use the same
  offset as the LDTR, which is very suspicious. I suspect that this
  should use "(0x22 + i * 2)".

  The code later in the same function that stores the segment registers
  to the struct has the same bug.

  Found by code inspection; I don't have a test case to check this. As a
  non-x86-expert I'm just going to file a bug report in case somebody
  else feels like confirming the issue and sending a patch.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1879955/+subscriptions

^ permalink raw reply	[flat|nested] 3+ messages in thread