Skip to content

Commit 87da7e6

Browse files
Xiao Guangrongmatosatti
authored andcommitted
KVM: x86: fix vcpu->mmio_fragments overflow
After commit b3356bf (KVM: emulator: optimize "rep ins" handling), the pieces of io data can be collected and write them to the guest memory or MMIO together Unfortunately, kvm splits the mmio access into 8 bytes and store them to vcpu->mmio_fragments. If the guest uses "rep ins" to move large data, it will cause vcpu->mmio_fragments overflow The bug can be exposed by isapc (-M isapc): [23154.818733] general protection fault: 0000 [Digilent#1] SMP DEBUG_PAGEALLOC [ ......] [23154.858083] Call Trace: [23154.859874] [<ffffffffa04f0e17>] kvm_get_cr8+0x1d/0x28 [kvm] [23154.861677] [<ffffffffa04fa6d4>] kvm_arch_vcpu_ioctl_run+0xcda/0xe45 [kvm] [23154.863604] [<ffffffffa04f5a1a>] ? kvm_arch_vcpu_load+0x17b/0x180 [kvm] Actually, we can use one mmio_fragment to store a large mmio access then split it when we pass the mmio-exit-info to userspace. After that, we only need two entries to store mmio info for the cross-mmio pages access Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
1 parent 35fd3dc commit 87da7e6

2 files changed

Lines changed: 36 additions & 39 deletions

File tree

arch/x86/kvm/x86.c

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3779,7 +3779,7 @@ static int write_exit_mmio(struct kvm_vcpu *vcpu, gpa_t gpa,
37793779
{
37803780
struct kvm_mmio_fragment *frag = &vcpu->mmio_fragments[0];
37813781

3782-
memcpy(vcpu->run->mmio.data, frag->data, frag->len);
3782+
memcpy(vcpu->run->mmio.data, frag->data, min(8u, frag->len));
37833783
return X86EMUL_CONTINUE;
37843784
}
37853785

@@ -3832,18 +3832,11 @@ static int emulator_read_write_onepage(unsigned long addr, void *val,
38323832
bytes -= handled;
38333833
val += handled;
38343834

3835-
while (bytes) {
3836-
unsigned now = min(bytes, 8U);
3837-
3838-
frag = &vcpu->mmio_fragments[vcpu->mmio_nr_fragments++];
3839-
frag->gpa = gpa;
3840-
frag->data = val;
3841-
frag->len = now;
3842-
3843-
gpa += now;
3844-
val += now;
3845-
bytes -= now;
3846-
}
3835+
WARN_ON(vcpu->mmio_nr_fragments >= KVM_MAX_MMIO_FRAGMENTS);
3836+
frag = &vcpu->mmio_fragments[vcpu->mmio_nr_fragments++];
3837+
frag->gpa = gpa;
3838+
frag->data = val;
3839+
frag->len = bytes;
38473840
return X86EMUL_CONTINUE;
38483841
}
38493842

@@ -3890,7 +3883,7 @@ int emulator_read_write(struct x86_emulate_ctxt *ctxt, unsigned long addr,
38903883
vcpu->mmio_needed = 1;
38913884
vcpu->mmio_cur_fragment = 0;
38923885

3893-
vcpu->run->mmio.len = vcpu->mmio_fragments[0].len;
3886+
vcpu->run->mmio.len = min(8u, vcpu->mmio_fragments[0].len);
38943887
vcpu->run->mmio.is_write = vcpu->mmio_is_write = ops->write;
38953888
vcpu->run->exit_reason = KVM_EXIT_MMIO;
38963889
vcpu->run->mmio.phys_addr = gpa;
@@ -5522,42 +5515,57 @@ static int complete_emulated_pio(struct kvm_vcpu *vcpu)
55225515
*
55235516
* read:
55245517
* for each fragment
5525-
* write gpa, len
5526-
* exit
5527-
* copy data
5518+
* for each mmio piece in the fragment
5519+
* write gpa, len
5520+
* exit
5521+
* copy data
55285522
* execute insn
55295523
*
55305524
* write:
55315525
* for each fragment
5532-
* write gpa, len
5533-
* copy data
5534-
* exit
5526+
* for each mmio piece in the fragment
5527+
* write gpa, len
5528+
* copy data
5529+
* exit
55355530
*/
55365531
static int complete_emulated_mmio(struct kvm_vcpu *vcpu)
55375532
{
55385533
struct kvm_run *run = vcpu->run;
55395534
struct kvm_mmio_fragment *frag;
5535+
unsigned len;
55405536

55415537
BUG_ON(!vcpu->mmio_needed);
55425538

55435539
/* Complete previous fragment */
5544-
frag = &vcpu->mmio_fragments[vcpu->mmio_cur_fragment++];
5540+
frag = &vcpu->mmio_fragments[vcpu->mmio_cur_fragment];
5541+
len = min(8u, frag->len);
55455542
if (!vcpu->mmio_is_write)
5546-
memcpy(frag->data, run->mmio.data, frag->len);
5543+
memcpy(frag->data, run->mmio.data, len);
5544+
5545+
if (frag->len <= 8) {
5546+
/* Switch to the next fragment. */
5547+
frag++;
5548+
vcpu->mmio_cur_fragment++;
5549+
} else {
5550+
/* Go forward to the next mmio piece. */
5551+
frag->data += len;
5552+
frag->gpa += len;
5553+
frag->len -= len;
5554+
}
5555+
55475556
if (vcpu->mmio_cur_fragment == vcpu->mmio_nr_fragments) {
55485557
vcpu->mmio_needed = 0;
55495558
if (vcpu->mmio_is_write)
55505559
return 1;
55515560
vcpu->mmio_read_completed = 1;
55525561
return complete_emulated_io(vcpu);
55535562
}
5554-
/* Initiate next fragment */
5555-
++frag;
5563+
55565564
run->exit_reason = KVM_EXIT_MMIO;
55575565
run->mmio.phys_addr = frag->gpa;
55585566
if (vcpu->mmio_is_write)
5559-
memcpy(run->mmio.data, frag->data, frag->len);
5560-
run->mmio.len = frag->len;
5567+
memcpy(run->mmio.data, frag->data, min(8u, frag->len));
5568+
run->mmio.len = min(8u, frag->len);
55615569
run->mmio.is_write = vcpu->mmio_is_write;
55625570
vcpu->arch.complete_userspace_io = complete_emulated_mmio;
55635571
return 0;

include/linux/kvm_host.h

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,19 +42,8 @@
4242
*/
4343
#define KVM_MEMSLOT_INVALID (1UL << 16)
4444

45-
/*
46-
* If we support unaligned MMIO, at most one fragment will be split into two:
47-
*/
48-
#ifdef KVM_UNALIGNED_MMIO
49-
# define KVM_EXTRA_MMIO_FRAGMENTS 1
50-
#else
51-
# define KVM_EXTRA_MMIO_FRAGMENTS 0
52-
#endif
53-
54-
#define KVM_USER_MMIO_SIZE 8
55-
56-
#define KVM_MAX_MMIO_FRAGMENTS \
57-
(KVM_MMIO_SIZE / KVM_USER_MMIO_SIZE + KVM_EXTRA_MMIO_FRAGMENTS)
45+
/* Two fragments for cross MMIO pages. */
46+
#define KVM_MAX_MMIO_FRAGMENTS 2
5847

5948
/*
6049
* For the normal pfn, the highest 12 bits should be zero,

0 commit comments

Comments
 (0)