From: Wanpeng Li <[email protected]>
Reported by syzkaller:
#PF: supervisor write access in kernel mode
#PF: error_code(0x0002) - not-present page
PGD 403c01067 P4D 403c01067 PUD 0
Oops: 0002 [#1] SMP PTI
CPU: 1 PID: 12564 Comm: a.out Tainted: G OE 5.3.0-rc4+ #4
RIP: 0010:coalesced_mmio_write+0xcc/0x130 [kvm]
Call Trace:
__kvm_io_bus_write+0x91/0xe0 [kvm]
kvm_io_bus_write+0x79/0xf0 [kvm]
write_mmio+0xae/0x170 [kvm]
emulator_read_write_onepage+0x252/0x430 [kvm]
emulator_read_write+0xcd/0x180 [kvm]
emulator_write_emulated+0x15/0x20 [kvm]
segmented_write+0x59/0x80 [kvm]
writeback+0x113/0x250 [kvm]
x86_emulate_insn+0x78c/0xd80 [kvm]
x86_emulate_instruction+0x386/0x7c0 [kvm]
kvm_mmu_page_fault+0xf9/0x9e0 [kvm]
handle_ept_violation+0x10a/0x220 [kvm_intel]
vmx_handle_exit+0xbe/0x6b0 [kvm_intel]
vcpu_enter_guest+0x4dc/0x18d0 [kvm]
kvm_arch_vcpu_ioctl_run+0x407/0x660 [kvm]
kvm_vcpu_ioctl+0x3ad/0x690 [kvm]
do_vfs_ioctl+0xa2/0x690
ksys_ioctl+0x6d/0x80
__x64_sys_ioctl+0x1a/0x20
do_syscall_64+0x74/0x720
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0010:coalesced_mmio_write+0xcc/0x130 [kvm]
Both the coalesced_mmio ring buffer indexs ring->first and ring->last are
bigger than KVM_COALESCED_MMIO_MAX from the testcase, array out-of-bounds
access triggers by ring->coalesced_mmio[ring->last].phys_addr = addr;
assignment. This patch fixes it by mod indexs by KVM_COALESCED_MMIO_MAX.
syzkaller source: https://syzkaller.appspot.com/x/repro.c?x=134b2826a00000
Reported-by: [email protected]
Cc: [email protected]
Signed-off-by: Wanpeng Li <[email protected]>
---
virt/kvm/coalesced_mmio.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c
index 5294abb..cff1ec9 100644
--- a/virt/kvm/coalesced_mmio.c
+++ b/virt/kvm/coalesced_mmio.c
@@ -73,6 +73,8 @@ static int coalesced_mmio_write(struct kvm_vcpu *vcpu,
spin_lock(&dev->kvm->ring_lock);
+ ring->first = ring->first % KVM_COALESCED_MMIO_MAX;
+ ring->last = ring->last % KVM_COALESCED_MMIO_MAX;
if (!coalesced_mmio_has_room(dev)) {
spin_unlock(&dev->kvm->ring_lock);
return -EOPNOTSUPP;
--
2.7.4
On Tue, Sep 17, 2019 at 7:59 AM Jim Mattson <[email protected]> wrote:
> On Tue, Sep 17, 2019 at 1:16 AM Wanpeng Li <[email protected]> wrote:
> > From: Wanpeng Li <[email protected]>
> >
> > Reported by syzkaller:
> >
> > #PF: supervisor write access in kernel mode
> > #PF: error_code(0x0002) - not-present page
> > PGD 403c01067 P4D 403c01067 PUD 0
> > Oops: 0002 [#1] SMP PTI
> > CPU: 1 PID: 12564 Comm: a.out Tainted: G OE 5.3.0-rc4+ #4
> > RIP: 0010:coalesced_mmio_write+0xcc/0x130 [kvm]
> > Call Trace:
> > __kvm_io_bus_write+0x91/0xe0 [kvm]
> > kvm_io_bus_write+0x79/0xf0 [kvm]
> > write_mmio+0xae/0x170 [kvm]
> > emulator_read_write_onepage+0x252/0x430 [kvm]
> > emulator_read_write+0xcd/0x180 [kvm]
> > emulator_write_emulated+0x15/0x20 [kvm]
> > segmented_write+0x59/0x80 [kvm]
> > writeback+0x113/0x250 [kvm]
> > x86_emulate_insn+0x78c/0xd80 [kvm]
> > x86_emulate_instruction+0x386/0x7c0 [kvm]
> > kvm_mmu_page_fault+0xf9/0x9e0 [kvm]
> > handle_ept_violation+0x10a/0x220 [kvm_intel]
> > vmx_handle_exit+0xbe/0x6b0 [kvm_intel]
> > vcpu_enter_guest+0x4dc/0x18d0 [kvm]
> > kvm_arch_vcpu_ioctl_run+0x407/0x660 [kvm]
> > kvm_vcpu_ioctl+0x3ad/0x690 [kvm]
> > do_vfs_ioctl+0xa2/0x690
> > ksys_ioctl+0x6d/0x80
> > __x64_sys_ioctl+0x1a/0x20
> > do_syscall_64+0x74/0x720
> > entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > RIP: 0010:coalesced_mmio_write+0xcc/0x130 [kvm]
> >
> > Both the coalesced_mmio ring buffer indexs ring->first and ring->last are
> > bigger than KVM_COALESCED_MMIO_MAX from the testcase, array out-of-bounds
> > access triggers by ring->coalesced_mmio[ring->last].phys_addr = addr;
> > assignment. This patch fixes it by mod indexs by KVM_COALESCED_MMIO_MAX.
> >
> > syzkaller source: https://syzkaller.appspot.com/x/repro.c?x=134b2826a00000
> >
> > Reported-by: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Wanpeng Li <[email protected]>
> > ---
> > virt/kvm/coalesced_mmio.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c
> > index 5294abb..cff1ec9 100644
> > --- a/virt/kvm/coalesced_mmio.c
> > +++ b/virt/kvm/coalesced_mmio.c
> > @@ -73,6 +73,8 @@ static int coalesced_mmio_write(struct kvm_vcpu *vcpu,
> >
> > spin_lock(&dev->kvm->ring_lock);
> >
> > + ring->first = ring->first % KVM_COALESCED_MMIO_MAX;
This update to first doesn't provide any worthwhile benefit (it's not
used to compute the address of a write) and likely adds the overhead
cost of a 2nd divide operation (via the non-power-of-2 modulus). If
first is invalid then the app and/or kernel will be confused about
whether the ring is empty or full, but no serious harm will occur (and
since the only write to first is by an app the app is only causing
harm to itself).
> > + ring->last = ring->last % KVM_COALESCED_MMIO_MAX;
>
> I don't think this is sufficient, since the memory that ring points to
> is shared with userspace. Userspace can overwrite your corrected
> values with illegal ones before they are used. Not exactly a TOCTTOU
> issue, since there isn't technically a 'check' here, but the same
> idea.
>
> > if (!coalesced_mmio_has_room(dev)) {
> > spin_unlock(&dev->kvm->ring_lock);
> > return -EOPNOTSUPP;
> > --
> > 2.7.4
> >
On Tue, Sep 17, 2019 at 1:16 AM Wanpeng Li <[email protected]> wrote:
>
> From: Wanpeng Li <[email protected]>
>
> Reported by syzkaller:
>
> #PF: supervisor write access in kernel mode
> #PF: error_code(0x0002) - not-present page
> PGD 403c01067 P4D 403c01067 PUD 0
> Oops: 0002 [#1] SMP PTI
> CPU: 1 PID: 12564 Comm: a.out Tainted: G OE 5.3.0-rc4+ #4
> RIP: 0010:coalesced_mmio_write+0xcc/0x130 [kvm]
> Call Trace:
> __kvm_io_bus_write+0x91/0xe0 [kvm]
> kvm_io_bus_write+0x79/0xf0 [kvm]
> write_mmio+0xae/0x170 [kvm]
> emulator_read_write_onepage+0x252/0x430 [kvm]
> emulator_read_write+0xcd/0x180 [kvm]
> emulator_write_emulated+0x15/0x20 [kvm]
> segmented_write+0x59/0x80 [kvm]
> writeback+0x113/0x250 [kvm]
> x86_emulate_insn+0x78c/0xd80 [kvm]
> x86_emulate_instruction+0x386/0x7c0 [kvm]
> kvm_mmu_page_fault+0xf9/0x9e0 [kvm]
> handle_ept_violation+0x10a/0x220 [kvm_intel]
> vmx_handle_exit+0xbe/0x6b0 [kvm_intel]
> vcpu_enter_guest+0x4dc/0x18d0 [kvm]
> kvm_arch_vcpu_ioctl_run+0x407/0x660 [kvm]
> kvm_vcpu_ioctl+0x3ad/0x690 [kvm]
> do_vfs_ioctl+0xa2/0x690
> ksys_ioctl+0x6d/0x80
> __x64_sys_ioctl+0x1a/0x20
> do_syscall_64+0x74/0x720
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0010:coalesced_mmio_write+0xcc/0x130 [kvm]
>
> Both the coalesced_mmio ring buffer indexs ring->first and ring->last are
> bigger than KVM_COALESCED_MMIO_MAX from the testcase, array out-of-bounds
> access triggers by ring->coalesced_mmio[ring->last].phys_addr = addr;
> assignment. This patch fixes it by mod indexs by KVM_COALESCED_MMIO_MAX.
>
> syzkaller source: https://syzkaller.appspot.com/x/repro.c?x=134b2826a00000
>
> Reported-by: [email protected]
> Cc: [email protected]
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> virt/kvm/coalesced_mmio.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c
> index 5294abb..cff1ec9 100644
> --- a/virt/kvm/coalesced_mmio.c
> +++ b/virt/kvm/coalesced_mmio.c
> @@ -73,6 +73,8 @@ static int coalesced_mmio_write(struct kvm_vcpu *vcpu,
>
> spin_lock(&dev->kvm->ring_lock);
>
> + ring->first = ring->first % KVM_COALESCED_MMIO_MAX;
> + ring->last = ring->last % KVM_COALESCED_MMIO_MAX;
I don't think this is sufficient, since the memory that ring points to
is shared with userspace. Userspace can overwrite your corrected
values with illegal ones before they are used. Not exactly a TOCTTOU
issue, since there isn't technically a 'check' here, but the same
idea.
> if (!coalesced_mmio_has_room(dev)) {
> spin_unlock(&dev->kvm->ring_lock);
> return -EOPNOTSUPP;
> --
> 2.7.4
>