Skip to content

Commit

Permalink
KVM: x86 emulator: fix bit string operations operand size
Browse files Browse the repository at this point in the history
On x86, bit operations operate on a string of bits that can reside in
multiple words.  For example, 'btsl %eax, (blah)' will touch the word
at blah+4 if %eax is between 32 and 63.

The x86 emulator compensates for that by advancing the operand address
by (bit offset / BITS_PER_LONG) and truncating the bit offset to the
range (0..BITS_PER_LONG-1).  This has a side effect of forcing the operand
size to 8 bytes on 64-bit hosts.

Now, a 32-bit guest goes and fork()s a process.  It write protects a stack
page at 0xbffff000 using the 'btr' instruction, at offset 0xffc in the page
table, with bit offset 1 (for the write permission bit).

The emulator now forces the operand size to 8 bytes as previously described,
and an innocent page table update turns into a cross-page-boundary write,
which is assumed by the mmu code not to be a page table, so it doesn't
actually clear the corresponding shadow page table entry.  The guest and
host permissions are out of sync and guest memory is corrupted soon
afterwards, leading to guest failure.

Fix by not using BITS_PER_LONG as the word size; instead use the actual
operand size, so we get a 32-bit write in that case.

Note we still have to teach the mmu to handle cross-page-boundary writes
to guest page table; but for now this allows Damn Small Linux 0.4 (2.4.20)
to boot.

Signed-off-by: Avi Kivity <avi@qumranet.com>
  • Loading branch information
Avi Kivity committed May 3, 2007
1 parent afeb1f1 commit df513e2
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions drivers/kvm/x86_emulate.c
Original file line number Diff line number Diff line change
Expand Up @@ -833,8 +833,9 @@ x86_emulate_memop(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops)
dst.ptr = (unsigned long *)cr2;
dst.bytes = (d & ByteOp) ? 1 : op_bytes;
if (d & BitOp) {
dst.ptr += src.val / BITS_PER_LONG;
dst.bytes = sizeof(long);
unsigned long mask = ~(dst.bytes * 8 - 1);

dst.ptr = (void *)dst.ptr + (src.val & mask) / 8;
}
if (!(d & Mov) && /* optimisation - avoid slow emulated read */
((rc = ops->read_emulated((unsigned long)dst.ptr,
Expand Down

0 comments on commit df513e2

Please sign in to comment.