| Message ID | 20181204022238.13757-2-khalid.elmously@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix for CVE-2018-19407 | expand |
On Mon, 3 Dec 2018 21:22:38 -0500 Khalid Elmously <khalid.elmously@canonical.com> wrote: > From: Wanpeng Li <wanpengli@tencent.com> > > CVE-2018-19407 > > Reported by syzkaller: > > BUG: unable to handle kernel NULL pointer dereference at 00000000000001c8 > PGD 80000003ec4da067 P4D 80000003ec4da067 PUD 3f7bfa067 PMD 0 > Oops: 0000 [#1] PREEMPT SMP PTI > CPU: 7 PID: 5059 Comm: debug Tainted: G OE 4.19.0-rc5 #16 > RIP: 0010:__lock_acquire+0x1a6/0x1990 > Call Trace: > lock_acquire+0xdb/0x210 > _raw_spin_lock+0x38/0x70 > kvm_ioapic_scan_entry+0x3e/0x110 [kvm] > vcpu_enter_guest+0x167e/0x1910 [kvm] > kvm_arch_vcpu_ioctl_run+0x35c/0x610 [kvm] > kvm_vcpu_ioctl+0x3e9/0x6d0 [kvm] > do_vfs_ioctl+0xa5/0x690 > ksys_ioctl+0x6d/0x80 > __x64_sys_ioctl+0x1a/0x20 > do_syscall_64+0x83/0x6e0 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > The reason is that the testcase writes hyperv synic HV_X64_MSR_SINT6 msr > and triggers scan ioapic logic to load synic vectors into EOI exit bitmap. > However, irqchip is not initialized by this simple testcase, ioapic/apic > objects should not be accessed. > This can be triggered by the following program: > > #define _GNU_SOURCE > > #include <endian.h> > #include <stdint.h> > #include <stdio.h> > #include <stdlib.h> > #include <string.h> > #include <sys/syscall.h> > #include <sys/types.h> > #include <unistd.h> > > uint64_t r[3] = {0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff}; > > int main(void) > { > syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0); > long res = 0; > memcpy((void*)0x20000040, "/dev/kvm", 9); > res = syscall(__NR_openat, 0xffffffffffffff9c, 0x20000040, 0, 0); > if (res != -1) > r[0] = res; > res = syscall(__NR_ioctl, r[0], 0xae01, 0); > if (res != -1) > r[1] = res; > res = syscall(__NR_ioctl, r[1], 0xae41, 0); > if (res != -1) > r[2] = res; > memcpy( > (void*)0x20000080, > "\x01\x00\x00\x00\x00\x5b\x61\xbb\x96\x00\x00\x40\x00\x00\x00\x00\x01\x00" > "\x08\x00\x00\x00\x00\x00\x0b\x77\xd1\x78\x4d\xd8\x3a\xed\xb1\x5c\x2e\x43" > "\xaa\x43\x39\xd6\xff\xf5\xf0\xa8\x98\xf2\x3e\x37\x29\x89\xde\x88\xc6\x33" > "\xfc\x2a\xdb\xb7\xe1\x4c\xac\x28\x61\x7b\x9c\xa9\xbc\x0d\xa0\x63\xfe\xfe" > "\xe8\x75\xde\xdd\x19\x38\xdc\x34\xf5\xec\x05\xfd\xeb\x5d\xed\x2e\xaf\x22" > "\xfa\xab\xb7\xe4\x42\x67\xd0\xaf\x06\x1c\x6a\x35\x67\x10\x55\xcb", > 106); > syscall(__NR_ioctl, r[2], 0x4008ae89, 0x20000080); > syscall(__NR_ioctl, r[2], 0xae80, 0); > return 0; > } > > This patch fixes it by bailing out scan ioapic if ioapic is not initialized in > kernel. > > Reported-by: Wei Wu <ww9210@gmail.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Radim Krčmář <rkrcmar@redhat.com> > Cc: Wei Wu <ww9210@gmail.com> > Signed-off-by: Wanpeng Li <wanpengli@tencent.com> > Cc: stable@vger.kernel.org > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Empty line here please. > (cherry-picked from e97f852fd4561e77721bb9a4e0ea9d98305b1e93) (cherry picked from commit ... Just like git cherry-pick -x produces. ...Juerg > Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com> > --- > arch/x86/kvm/x86.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index c98ffb0e6b47..a3b6f780ad99 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -7225,7 +7225,8 @@ static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu) > else { > if (vcpu->arch.apicv_active) > kvm_x86_ops->sync_pir_to_irr(vcpu); > - kvm_ioapic_scan_entry(vcpu, vcpu->arch.ioapic_handled_vectors); > + if (ioapic_in_kernel(vcpu->kvm)) > + kvm_ioapic_scan_entry(vcpu, vcpu->arch.ioapic_handled_vectors); > } > > if (is_guest_mode(vcpu))
On Tue, Dec 04, 2018 at 12:14:08PM +0100, Juerg Haefliger wrote: > On Mon, 3 Dec 2018 21:22:38 -0500 > Khalid Elmously <khalid.elmously@canonical.com> wrote: > > > From: Wanpeng Li <wanpengli@tencent.com> > > > > CVE-2018-19407 > > > > Reported by syzkaller: > > > > BUG: unable to handle kernel NULL pointer dereference at 00000000000001c8 > > PGD 80000003ec4da067 P4D 80000003ec4da067 PUD 3f7bfa067 PMD 0 > > Oops: 0000 [#1] PREEMPT SMP PTI > > CPU: 7 PID: 5059 Comm: debug Tainted: G OE 4.19.0-rc5 #16 > > RIP: 0010:__lock_acquire+0x1a6/0x1990 > > Call Trace: > > lock_acquire+0xdb/0x210 > > _raw_spin_lock+0x38/0x70 > > kvm_ioapic_scan_entry+0x3e/0x110 [kvm] > > vcpu_enter_guest+0x167e/0x1910 [kvm] > > kvm_arch_vcpu_ioctl_run+0x35c/0x610 [kvm] > > kvm_vcpu_ioctl+0x3e9/0x6d0 [kvm] > > do_vfs_ioctl+0xa5/0x690 > > ksys_ioctl+0x6d/0x80 > > __x64_sys_ioctl+0x1a/0x20 > > do_syscall_64+0x83/0x6e0 > > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > > > The reason is that the testcase writes hyperv synic HV_X64_MSR_SINT6 msr > > and triggers scan ioapic logic to load synic vectors into EOI exit bitmap. > > However, irqchip is not initialized by this simple testcase, ioapic/apic > > objects should not be accessed. > > This can be triggered by the following program: > > > > #define _GNU_SOURCE > > > > #include <endian.h> > > #include <stdint.h> > > #include <stdio.h> > > #include <stdlib.h> > > #include <string.h> > > #include <sys/syscall.h> > > #include <sys/types.h> > > #include <unistd.h> > > > > uint64_t r[3] = {0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff}; > > > > int main(void) > > { > > syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0); > > long res = 0; > > memcpy((void*)0x20000040, "/dev/kvm", 9); > > res = syscall(__NR_openat, 0xffffffffffffff9c, 0x20000040, 0, 0); > > if (res != -1) > > r[0] = res; > > res = syscall(__NR_ioctl, r[0], 0xae01, 0); > > if (res != -1) > > r[1] = res; > > res = syscall(__NR_ioctl, r[1], 0xae41, 0); > > if (res != -1) > > r[2] = res; > > memcpy( > > (void*)0x20000080, > > "\x01\x00\x00\x00\x00\x5b\x61\xbb\x96\x00\x00\x40\x00\x00\x00\x00\x01\x00" > > "\x08\x00\x00\x00\x00\x00\x0b\x77\xd1\x78\x4d\xd8\x3a\xed\xb1\x5c\x2e\x43" > > "\xaa\x43\x39\xd6\xff\xf5\xf0\xa8\x98\xf2\x3e\x37\x29\x89\xde\x88\xc6\x33" > > "\xfc\x2a\xdb\xb7\xe1\x4c\xac\x28\x61\x7b\x9c\xa9\xbc\x0d\xa0\x63\xfe\xfe" > > "\xe8\x75\xde\xdd\x19\x38\xdc\x34\xf5\xec\x05\xfd\xeb\x5d\xed\x2e\xaf\x22" > > "\xfa\xab\xb7\xe4\x42\x67\xd0\xaf\x06\x1c\x6a\x35\x67\x10\x55\xcb", > > 106); > > syscall(__NR_ioctl, r[2], 0x4008ae89, 0x20000080); > > syscall(__NR_ioctl, r[2], 0xae80, 0); > > return 0; > > } > > > > This patch fixes it by bailing out scan ioapic if ioapic is not initialized in > > kernel. > > > > Reported-by: Wei Wu <ww9210@gmail.com> > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > Cc: Radim Krčmář <rkrcmar@redhat.com> > > Cc: Wei Wu <ww9210@gmail.com> > > Signed-off-by: Wanpeng Li <wanpengli@tencent.com> > > Cc: stable@vger.kernel.org > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > Empty line here please. Um, why? This has never been a requirement, and it's not what is produced by 'git cherry-pick -x'. I personally don't want to have to go in and hand-edit the commit message to add a blank line here every time I cherry pick a patch.
On 05/12/2018 14:06, Seth Forshee wrote: > On Tue, Dec 04, 2018 at 12:14:08PM +0100, Juerg Haefliger wrote: >> On Mon, 3 Dec 2018 21:22:38 -0500 >> Khalid Elmously <khalid.elmously@canonical.com> wrote: >> >>> From: Wanpeng Li <wanpengli@tencent.com> >>> >>> CVE-2018-19407 >>> >>> Reported by syzkaller: >>> >>> BUG: unable to handle kernel NULL pointer dereference at 00000000000001c8 >>> PGD 80000003ec4da067 P4D 80000003ec4da067 PUD 3f7bfa067 PMD 0 >>> Oops: 0000 [#1] PREEMPT SMP PTI >>> CPU: 7 PID: 5059 Comm: debug Tainted: G OE 4.19.0-rc5 #16 >>> RIP: 0010:__lock_acquire+0x1a6/0x1990 >>> Call Trace: >>> lock_acquire+0xdb/0x210 >>> _raw_spin_lock+0x38/0x70 >>> kvm_ioapic_scan_entry+0x3e/0x110 [kvm] >>> vcpu_enter_guest+0x167e/0x1910 [kvm] >>> kvm_arch_vcpu_ioctl_run+0x35c/0x610 [kvm] >>> kvm_vcpu_ioctl+0x3e9/0x6d0 [kvm] >>> do_vfs_ioctl+0xa5/0x690 >>> ksys_ioctl+0x6d/0x80 >>> __x64_sys_ioctl+0x1a/0x20 >>> do_syscall_64+0x83/0x6e0 >>> entry_SYSCALL_64_after_hwframe+0x49/0xbe >>> >>> The reason is that the testcase writes hyperv synic HV_X64_MSR_SINT6 msr >>> and triggers scan ioapic logic to load synic vectors into EOI exit bitmap. >>> However, irqchip is not initialized by this simple testcase, ioapic/apic >>> objects should not be accessed. >>> This can be triggered by the following program: >>> >>> #define _GNU_SOURCE >>> >>> #include <endian.h> >>> #include <stdint.h> >>> #include <stdio.h> >>> #include <stdlib.h> >>> #include <string.h> >>> #include <sys/syscall.h> >>> #include <sys/types.h> >>> #include <unistd.h> >>> >>> uint64_t r[3] = {0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff}; >>> >>> int main(void) >>> { >>> syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0); >>> long res = 0; >>> memcpy((void*)0x20000040, "/dev/kvm", 9); >>> res = syscall(__NR_openat, 0xffffffffffffff9c, 0x20000040, 0, 0); >>> if (res != -1) >>> r[0] = res; >>> res = syscall(__NR_ioctl, r[0], 0xae01, 0); >>> if (res != -1) >>> r[1] = res; >>> res = syscall(__NR_ioctl, r[1], 0xae41, 0); >>> if (res != -1) >>> r[2] = res; >>> memcpy( >>> (void*)0x20000080, >>> "\x01\x00\x00\x00\x00\x5b\x61\xbb\x96\x00\x00\x40\x00\x00\x00\x00\x01\x00" >>> "\x08\x00\x00\x00\x00\x00\x0b\x77\xd1\x78\x4d\xd8\x3a\xed\xb1\x5c\x2e\x43" >>> "\xaa\x43\x39\xd6\xff\xf5\xf0\xa8\x98\xf2\x3e\x37\x29\x89\xde\x88\xc6\x33" >>> "\xfc\x2a\xdb\xb7\xe1\x4c\xac\x28\x61\x7b\x9c\xa9\xbc\x0d\xa0\x63\xfe\xfe" >>> "\xe8\x75\xde\xdd\x19\x38\xdc\x34\xf5\xec\x05\xfd\xeb\x5d\xed\x2e\xaf\x22" >>> "\xfa\xab\xb7\xe4\x42\x67\xd0\xaf\x06\x1c\x6a\x35\x67\x10\x55\xcb", >>> 106); >>> syscall(__NR_ioctl, r[2], 0x4008ae89, 0x20000080); >>> syscall(__NR_ioctl, r[2], 0xae80, 0); >>> return 0; >>> } >>> >>> This patch fixes it by bailing out scan ioapic if ioapic is not initialized in >>> kernel. >>> >>> Reported-by: Wei Wu <ww9210@gmail.com> >>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>> Cc: Radim Krčmář <rkrcmar@redhat.com> >>> Cc: Wei Wu <ww9210@gmail.com> >>> Signed-off-by: Wanpeng Li <wanpengli@tencent.com> >>> Cc: stable@vger.kernel.org >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> >> Empty line here please. > > Um, why? This has never been a requirement, and it's not what is > produced by 'git cherry-pick -x'. I personally don't want to have to go > in and hand-edit the commit message to add a blank line here every time > I cherry pick a patch. +1 to that too, we've never added blank lines in the past, why the change? Colin
On 05.12.18 15:10, Colin Ian King wrote: > On 05/12/2018 14:06, Seth Forshee wrote: >> On Tue, Dec 04, 2018 at 12:14:08PM +0100, Juerg Haefliger wrote: >>> On Mon, 3 Dec 2018 21:22:38 -0500 >>> Khalid Elmously <khalid.elmously@canonical.com> wrote: >>> >>>> From: Wanpeng Li <wanpengli@tencent.com> >>>> >>>> CVE-2018-19407 >>>> >>>> Reported by syzkaller: >>>> >>>> BUG: unable to handle kernel NULL pointer dereference at 00000000000001c8 >>>> PGD 80000003ec4da067 P4D 80000003ec4da067 PUD 3f7bfa067 PMD 0 >>>> Oops: 0000 [#1] PREEMPT SMP PTI >>>> CPU: 7 PID: 5059 Comm: debug Tainted: G OE 4.19.0-rc5 #16 >>>> RIP: 0010:__lock_acquire+0x1a6/0x1990 >>>> Call Trace: >>>> lock_acquire+0xdb/0x210 >>>> _raw_spin_lock+0x38/0x70 >>>> kvm_ioapic_scan_entry+0x3e/0x110 [kvm] >>>> vcpu_enter_guest+0x167e/0x1910 [kvm] >>>> kvm_arch_vcpu_ioctl_run+0x35c/0x610 [kvm] >>>> kvm_vcpu_ioctl+0x3e9/0x6d0 [kvm] >>>> do_vfs_ioctl+0xa5/0x690 >>>> ksys_ioctl+0x6d/0x80 >>>> __x64_sys_ioctl+0x1a/0x20 >>>> do_syscall_64+0x83/0x6e0 >>>> entry_SYSCALL_64_after_hwframe+0x49/0xbe >>>> >>>> The reason is that the testcase writes hyperv synic HV_X64_MSR_SINT6 msr >>>> and triggers scan ioapic logic to load synic vectors into EOI exit bitmap. >>>> However, irqchip is not initialized by this simple testcase, ioapic/apic >>>> objects should not be accessed. >>>> This can be triggered by the following program: >>>> >>>> #define _GNU_SOURCE >>>> >>>> #include <endian.h> >>>> #include <stdint.h> >>>> #include <stdio.h> >>>> #include <stdlib.h> >>>> #include <string.h> >>>> #include <sys/syscall.h> >>>> #include <sys/types.h> >>>> #include <unistd.h> >>>> >>>> uint64_t r[3] = {0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff}; >>>> >>>> int main(void) >>>> { >>>> syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0); >>>> long res = 0; >>>> memcpy((void*)0x20000040, "/dev/kvm", 9); >>>> res = syscall(__NR_openat, 0xffffffffffffff9c, 0x20000040, 0, 0); >>>> if (res != -1) >>>> r[0] = res; >>>> res = syscall(__NR_ioctl, r[0], 0xae01, 0); >>>> if (res != -1) >>>> r[1] = res; >>>> res = syscall(__NR_ioctl, r[1], 0xae41, 0); >>>> if (res != -1) >>>> r[2] = res; >>>> memcpy( >>>> (void*)0x20000080, >>>> "\x01\x00\x00\x00\x00\x5b\x61\xbb\x96\x00\x00\x40\x00\x00\x00\x00\x01\x00" >>>> "\x08\x00\x00\x00\x00\x00\x0b\x77\xd1\x78\x4d\xd8\x3a\xed\xb1\x5c\x2e\x43" >>>> "\xaa\x43\x39\xd6\xff\xf5\xf0\xa8\x98\xf2\x3e\x37\x29\x89\xde\x88\xc6\x33" >>>> "\xfc\x2a\xdb\xb7\xe1\x4c\xac\x28\x61\x7b\x9c\xa9\xbc\x0d\xa0\x63\xfe\xfe" >>>> "\xe8\x75\xde\xdd\x19\x38\xdc\x34\xf5\xec\x05\xfd\xeb\x5d\xed\x2e\xaf\x22" >>>> "\xfa\xab\xb7\xe4\x42\x67\xd0\xaf\x06\x1c\x6a\x35\x67\x10\x55\xcb", >>>> 106); >>>> syscall(__NR_ioctl, r[2], 0x4008ae89, 0x20000080); >>>> syscall(__NR_ioctl, r[2], 0xae80, 0); >>>> return 0; >>>> } >>>> >>>> This patch fixes it by bailing out scan ioapic if ioapic is not initialized in >>>> kernel. >>>> >>>> Reported-by: Wei Wu <ww9210@gmail.com> >>>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>>> Cc: Radim Krčmář <rkrcmar@redhat.com> >>>> Cc: Wei Wu <ww9210@gmail.com> >>>> Signed-off-by: Wanpeng Li <wanpengli@tencent.com> >>>> Cc: stable@vger.kernel.org >>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>> >>> Empty line here please. >> >> Um, why? This has never been a requirement, and it's not what is >> produced by 'git cherry-pick -x'. I personally don't want to have to go >> in and hand-edit the commit message to add a blank line here every time >> I cherry pick a patch. > > +1 to that too, we've never added blank lines in the past, why the change? > > Colin > I did in some cases but would not make that a strict requirement in general. It helps to improve separating the signed off chain from upstream from those added by us. -Stefan
On 12/5/18 3:06 PM, Seth Forshee wrote: > On Tue, Dec 04, 2018 at 12:14:08PM +0100, Juerg Haefliger wrote: >> On Mon, 3 Dec 2018 21:22:38 -0500 >> Khalid Elmously <khalid.elmously@canonical.com> wrote: >> >>> From: Wanpeng Li <wanpengli@tencent.com> >>> >>> CVE-2018-19407 >>> >>> Reported by syzkaller: >>> >>> BUG: unable to handle kernel NULL pointer dereference at 00000000000001c8 >>> PGD 80000003ec4da067 P4D 80000003ec4da067 PUD 3f7bfa067 PMD 0 >>> Oops: 0000 [#1] PREEMPT SMP PTI >>> CPU: 7 PID: 5059 Comm: debug Tainted: G OE 4.19.0-rc5 #16 >>> RIP: 0010:__lock_acquire+0x1a6/0x1990 >>> Call Trace: >>> lock_acquire+0xdb/0x210 >>> _raw_spin_lock+0x38/0x70 >>> kvm_ioapic_scan_entry+0x3e/0x110 [kvm] >>> vcpu_enter_guest+0x167e/0x1910 [kvm] >>> kvm_arch_vcpu_ioctl_run+0x35c/0x610 [kvm] >>> kvm_vcpu_ioctl+0x3e9/0x6d0 [kvm] >>> do_vfs_ioctl+0xa5/0x690 >>> ksys_ioctl+0x6d/0x80 >>> __x64_sys_ioctl+0x1a/0x20 >>> do_syscall_64+0x83/0x6e0 >>> entry_SYSCALL_64_after_hwframe+0x49/0xbe >>> >>> The reason is that the testcase writes hyperv synic HV_X64_MSR_SINT6 msr >>> and triggers scan ioapic logic to load synic vectors into EOI exit bitmap. >>> However, irqchip is not initialized by this simple testcase, ioapic/apic >>> objects should not be accessed. >>> This can be triggered by the following program: >>> >>> #define _GNU_SOURCE >>> >>> #include <endian.h> >>> #include <stdint.h> >>> #include <stdio.h> >>> #include <stdlib.h> >>> #include <string.h> >>> #include <sys/syscall.h> >>> #include <sys/types.h> >>> #include <unistd.h> >>> >>> uint64_t r[3] = {0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff}; >>> >>> int main(void) >>> { >>> syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0); >>> long res = 0; >>> memcpy((void*)0x20000040, "/dev/kvm", 9); >>> res = syscall(__NR_openat, 0xffffffffffffff9c, 0x20000040, 0, 0); >>> if (res != -1) >>> r[0] = res; >>> res = syscall(__NR_ioctl, r[0], 0xae01, 0); >>> if (res != -1) >>> r[1] = res; >>> res = syscall(__NR_ioctl, r[1], 0xae41, 0); >>> if (res != -1) >>> r[2] = res; >>> memcpy( >>> (void*)0x20000080, >>> "\x01\x00\x00\x00\x00\x5b\x61\xbb\x96\x00\x00\x40\x00\x00\x00\x00\x01\x00" >>> "\x08\x00\x00\x00\x00\x00\x0b\x77\xd1\x78\x4d\xd8\x3a\xed\xb1\x5c\x2e\x43" >>> "\xaa\x43\x39\xd6\xff\xf5\xf0\xa8\x98\xf2\x3e\x37\x29\x89\xde\x88\xc6\x33" >>> "\xfc\x2a\xdb\xb7\xe1\x4c\xac\x28\x61\x7b\x9c\xa9\xbc\x0d\xa0\x63\xfe\xfe" >>> "\xe8\x75\xde\xdd\x19\x38\xdc\x34\xf5\xec\x05\xfd\xeb\x5d\xed\x2e\xaf\x22" >>> "\xfa\xab\xb7\xe4\x42\x67\xd0\xaf\x06\x1c\x6a\x35\x67\x10\x55\xcb", >>> 106); >>> syscall(__NR_ioctl, r[2], 0x4008ae89, 0x20000080); >>> syscall(__NR_ioctl, r[2], 0xae80, 0); >>> return 0; >>> } >>> >>> This patch fixes it by bailing out scan ioapic if ioapic is not initialized in >>> kernel. >>> >>> Reported-by: Wei Wu <ww9210@gmail.com> >>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>> Cc: Radim Krčmář <rkrcmar@redhat.com> >>> Cc: Wei Wu <ww9210@gmail.com> >>> Signed-off-by: Wanpeng Li <wanpengli@tencent.com> >>> Cc: stable@vger.kernel.org >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> Empty line here please. > Um, why? This has never been a requirement, and it's not what is > produced by 'git cherry-pick -x'. I personally don't want to have to go > in and hand-edit the commit message to add a blank line here every time > I cherry pick a patch. > > I guess what Juerg meant with the output of 'git cherry-pick -x' is that it adds the line in the format: (cherry picked from commit ...) where the "commit" word was missing on the original patch. And this is strictly needed since it is the pattern matched by our tools. Thanks, Kleber
On Thu, Dec 06, 2018 at 08:47:33AM +0100, Kleber Souza wrote: > On 12/5/18 3:06 PM, Seth Forshee wrote: > > On Tue, Dec 04, 2018 at 12:14:08PM +0100, Juerg Haefliger wrote: > >> On Mon, 3 Dec 2018 21:22:38 -0500 > >> Khalid Elmously <khalid.elmously@canonical.com> wrote: > >> > >>> From: Wanpeng Li <wanpengli@tencent.com> > >>> > >>> CVE-2018-19407 > >>> > >>> Reported by syzkaller: > >>> > >>> BUG: unable to handle kernel NULL pointer dereference at 00000000000001c8 > >>> PGD 80000003ec4da067 P4D 80000003ec4da067 PUD 3f7bfa067 PMD 0 > >>> Oops: 0000 [#1] PREEMPT SMP PTI > >>> CPU: 7 PID: 5059 Comm: debug Tainted: G OE 4.19.0-rc5 #16 > >>> RIP: 0010:__lock_acquire+0x1a6/0x1990 > >>> Call Trace: > >>> lock_acquire+0xdb/0x210 > >>> _raw_spin_lock+0x38/0x70 > >>> kvm_ioapic_scan_entry+0x3e/0x110 [kvm] > >>> vcpu_enter_guest+0x167e/0x1910 [kvm] > >>> kvm_arch_vcpu_ioctl_run+0x35c/0x610 [kvm] > >>> kvm_vcpu_ioctl+0x3e9/0x6d0 [kvm] > >>> do_vfs_ioctl+0xa5/0x690 > >>> ksys_ioctl+0x6d/0x80 > >>> __x64_sys_ioctl+0x1a/0x20 > >>> do_syscall_64+0x83/0x6e0 > >>> entry_SYSCALL_64_after_hwframe+0x49/0xbe > >>> > >>> The reason is that the testcase writes hyperv synic HV_X64_MSR_SINT6 msr > >>> and triggers scan ioapic logic to load synic vectors into EOI exit bitmap. > >>> However, irqchip is not initialized by this simple testcase, ioapic/apic > >>> objects should not be accessed. > >>> This can be triggered by the following program: > >>> > >>> #define _GNU_SOURCE > >>> > >>> #include <endian.h> > >>> #include <stdint.h> > >>> #include <stdio.h> > >>> #include <stdlib.h> > >>> #include <string.h> > >>> #include <sys/syscall.h> > >>> #include <sys/types.h> > >>> #include <unistd.h> > >>> > >>> uint64_t r[3] = {0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff}; > >>> > >>> int main(void) > >>> { > >>> syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0); > >>> long res = 0; > >>> memcpy((void*)0x20000040, "/dev/kvm", 9); > >>> res = syscall(__NR_openat, 0xffffffffffffff9c, 0x20000040, 0, 0); > >>> if (res != -1) > >>> r[0] = res; > >>> res = syscall(__NR_ioctl, r[0], 0xae01, 0); > >>> if (res != -1) > >>> r[1] = res; > >>> res = syscall(__NR_ioctl, r[1], 0xae41, 0); > >>> if (res != -1) > >>> r[2] = res; > >>> memcpy( > >>> (void*)0x20000080, > >>> "\x01\x00\x00\x00\x00\x5b\x61\xbb\x96\x00\x00\x40\x00\x00\x00\x00\x01\x00" > >>> "\x08\x00\x00\x00\x00\x00\x0b\x77\xd1\x78\x4d\xd8\x3a\xed\xb1\x5c\x2e\x43" > >>> "\xaa\x43\x39\xd6\xff\xf5\xf0\xa8\x98\xf2\x3e\x37\x29\x89\xde\x88\xc6\x33" > >>> "\xfc\x2a\xdb\xb7\xe1\x4c\xac\x28\x61\x7b\x9c\xa9\xbc\x0d\xa0\x63\xfe\xfe" > >>> "\xe8\x75\xde\xdd\x19\x38\xdc\x34\xf5\xec\x05\xfd\xeb\x5d\xed\x2e\xaf\x22" > >>> "\xfa\xab\xb7\xe4\x42\x67\xd0\xaf\x06\x1c\x6a\x35\x67\x10\x55\xcb", > >>> 106); > >>> syscall(__NR_ioctl, r[2], 0x4008ae89, 0x20000080); > >>> syscall(__NR_ioctl, r[2], 0xae80, 0); > >>> return 0; > >>> } > >>> > >>> This patch fixes it by bailing out scan ioapic if ioapic is not initialized in > >>> kernel. > >>> > >>> Reported-by: Wei Wu <ww9210@gmail.com> > >>> Cc: Paolo Bonzini <pbonzini@redhat.com> > >>> Cc: Radim Krčmář <rkrcmar@redhat.com> > >>> Cc: Wei Wu <ww9210@gmail.com> > >>> Signed-off-by: Wanpeng Li <wanpengli@tencent.com> > >>> Cc: stable@vger.kernel.org > >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > >> Empty line here please. > > Um, why? This has never been a requirement, and it's not what is > > produced by 'git cherry-pick -x'. I personally don't want to have to go > > in and hand-edit the commit message to add a blank line here every time > > I cherry pick a patch. > > > > > I guess what Juerg meant with the output of 'git cherry-pick -x' is that > it adds the line in the format: > > (cherry picked from commit ...) > > where the "commit" word was missing on the original patch. And this is > strictly needed since it is the pattern matched by our tools. Yes, I understand that. My question was about requiring an empty line before that line, since git cherry-pick does not add one and we have never required it in the past.
On 12/4/18 3:22 AM, Khalid Elmously wrote: > From: Wanpeng Li <wanpengli@tencent.com> > > CVE-2018-19407 > > Reported by syzkaller: > > BUG: unable to handle kernel NULL pointer dereference at 00000000000001c8 > PGD 80000003ec4da067 P4D 80000003ec4da067 PUD 3f7bfa067 PMD 0 > Oops: 0000 [#1] PREEMPT SMP PTI > CPU: 7 PID: 5059 Comm: debug Tainted: G OE 4.19.0-rc5 #16 > RIP: 0010:__lock_acquire+0x1a6/0x1990 > Call Trace: > lock_acquire+0xdb/0x210 > _raw_spin_lock+0x38/0x70 > kvm_ioapic_scan_entry+0x3e/0x110 [kvm] > vcpu_enter_guest+0x167e/0x1910 [kvm] > kvm_arch_vcpu_ioctl_run+0x35c/0x610 [kvm] > kvm_vcpu_ioctl+0x3e9/0x6d0 [kvm] > do_vfs_ioctl+0xa5/0x690 > ksys_ioctl+0x6d/0x80 > __x64_sys_ioctl+0x1a/0x20 > do_syscall_64+0x83/0x6e0 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > The reason is that the testcase writes hyperv synic HV_X64_MSR_SINT6 msr > and triggers scan ioapic logic to load synic vectors into EOI exit bitmap. > However, irqchip is not initialized by this simple testcase, ioapic/apic > objects should not be accessed. > This can be triggered by the following program: > > #define _GNU_SOURCE > > #include <endian.h> > #include <stdint.h> > #include <stdio.h> > #include <stdlib.h> > #include <string.h> > #include <sys/syscall.h> > #include <sys/types.h> > #include <unistd.h> > > uint64_t r[3] = {0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff}; > > int main(void) > { > syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0); > long res = 0; > memcpy((void*)0x20000040, "/dev/kvm", 9); > res = syscall(__NR_openat, 0xffffffffffffff9c, 0x20000040, 0, 0); > if (res != -1) > r[0] = res; > res = syscall(__NR_ioctl, r[0], 0xae01, 0); > if (res != -1) > r[1] = res; > res = syscall(__NR_ioctl, r[1], 0xae41, 0); > if (res != -1) > r[2] = res; > memcpy( > (void*)0x20000080, > "\x01\x00\x00\x00\x00\x5b\x61\xbb\x96\x00\x00\x40\x00\x00\x00\x00\x01\x00" > "\x08\x00\x00\x00\x00\x00\x0b\x77\xd1\x78\x4d\xd8\x3a\xed\xb1\x5c\x2e\x43" > "\xaa\x43\x39\xd6\xff\xf5\xf0\xa8\x98\xf2\x3e\x37\x29\x89\xde\x88\xc6\x33" > "\xfc\x2a\xdb\xb7\xe1\x4c\xac\x28\x61\x7b\x9c\xa9\xbc\x0d\xa0\x63\xfe\xfe" > "\xe8\x75\xde\xdd\x19\x38\xdc\x34\xf5\xec\x05\xfd\xeb\x5d\xed\x2e\xaf\x22" > "\xfa\xab\xb7\xe4\x42\x67\xd0\xaf\x06\x1c\x6a\x35\x67\x10\x55\xcb", > 106); > syscall(__NR_ioctl, r[2], 0x4008ae89, 0x20000080); > syscall(__NR_ioctl, r[2], 0xae80, 0); > return 0; > } > > This patch fixes it by bailing out scan ioapic if ioapic is not initialized in > kernel. > > Reported-by: Wei Wu <ww9210@gmail.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Radim Krčmář <rkrcmar@redhat.com> > Cc: Wei Wu <ww9210@gmail.com> > Signed-off-by: Wanpeng Li <wanpengli@tencent.com> > Cc: stable@vger.kernel.org > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > (cherry-picked from e97f852fd4561e77721bb9a4e0ea9d98305b1e93) (cherry picked from commit e97f852fd4561e77721bb9a4e0ea9d98305b1e93) > Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com> With the cherry-pick line above fixed: Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > arch/x86/kvm/x86.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index c98ffb0e6b47..a3b6f780ad99 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -7225,7 +7225,8 @@ static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu) > else { > if (vcpu->arch.apicv_active) > kvm_x86_ops->sync_pir_to_irr(vcpu); > - kvm_ioapic_scan_entry(vcpu, vcpu->arch.ioapic_handled_vectors); > + if (ioapic_in_kernel(vcpu->kvm)) > + kvm_ioapic_scan_entry(vcpu, vcpu->arch.ioapic_handled_vectors); > } > > if (is_guest_mode(vcpu))
On 18.12.18 13:37, Kleber Souza wrote: > On 12/4/18 3:22 AM, Khalid Elmously wrote: >> From: Wanpeng Li <wanpengli@tencent.com> >> >> CVE-2018-19407 >> >> Reported by syzkaller: >> >> BUG: unable to handle kernel NULL pointer dereference at 00000000000001c8 >> PGD 80000003ec4da067 P4D 80000003ec4da067 PUD 3f7bfa067 PMD 0 >> Oops: 0000 [#1] PREEMPT SMP PTI >> CPU: 7 PID: 5059 Comm: debug Tainted: G OE 4.19.0-rc5 #16 >> RIP: 0010:__lock_acquire+0x1a6/0x1990 >> Call Trace: >> lock_acquire+0xdb/0x210 >> _raw_spin_lock+0x38/0x70 >> kvm_ioapic_scan_entry+0x3e/0x110 [kvm] >> vcpu_enter_guest+0x167e/0x1910 [kvm] >> kvm_arch_vcpu_ioctl_run+0x35c/0x610 [kvm] >> kvm_vcpu_ioctl+0x3e9/0x6d0 [kvm] >> do_vfs_ioctl+0xa5/0x690 >> ksys_ioctl+0x6d/0x80 >> __x64_sys_ioctl+0x1a/0x20 >> do_syscall_64+0x83/0x6e0 >> entry_SYSCALL_64_after_hwframe+0x49/0xbe >> >> The reason is that the testcase writes hyperv synic HV_X64_MSR_SINT6 msr >> and triggers scan ioapic logic to load synic vectors into EOI exit bitmap. >> However, irqchip is not initialized by this simple testcase, ioapic/apic >> objects should not be accessed. >> This can be triggered by the following program: >> >> #define _GNU_SOURCE >> >> #include <endian.h> >> #include <stdint.h> >> #include <stdio.h> >> #include <stdlib.h> >> #include <string.h> >> #include <sys/syscall.h> >> #include <sys/types.h> >> #include <unistd.h> >> >> uint64_t r[3] = {0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff}; >> >> int main(void) >> { >> syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0); >> long res = 0; >> memcpy((void*)0x20000040, "/dev/kvm", 9); >> res = syscall(__NR_openat, 0xffffffffffffff9c, 0x20000040, 0, 0); >> if (res != -1) >> r[0] = res; >> res = syscall(__NR_ioctl, r[0], 0xae01, 0); >> if (res != -1) >> r[1] = res; >> res = syscall(__NR_ioctl, r[1], 0xae41, 0); >> if (res != -1) >> r[2] = res; >> memcpy( >> (void*)0x20000080, >> "\x01\x00\x00\x00\x00\x5b\x61\xbb\x96\x00\x00\x40\x00\x00\x00\x00\x01\x00" >> "\x08\x00\x00\x00\x00\x00\x0b\x77\xd1\x78\x4d\xd8\x3a\xed\xb1\x5c\x2e\x43" >> "\xaa\x43\x39\xd6\xff\xf5\xf0\xa8\x98\xf2\x3e\x37\x29\x89\xde\x88\xc6\x33" >> "\xfc\x2a\xdb\xb7\xe1\x4c\xac\x28\x61\x7b\x9c\xa9\xbc\x0d\xa0\x63\xfe\xfe" >> "\xe8\x75\xde\xdd\x19\x38\xdc\x34\xf5\xec\x05\xfd\xeb\x5d\xed\x2e\xaf\x22" >> "\xfa\xab\xb7\xe4\x42\x67\xd0\xaf\x06\x1c\x6a\x35\x67\x10\x55\xcb", >> 106); >> syscall(__NR_ioctl, r[2], 0x4008ae89, 0x20000080); >> syscall(__NR_ioctl, r[2], 0xae80, 0); >> return 0; >> } >> >> This patch fixes it by bailing out scan ioapic if ioapic is not initialized in >> kernel. >> >> Reported-by: Wei Wu <ww9210@gmail.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Radim Krčmář <rkrcmar@redhat.com> >> Cc: Wei Wu <ww9210@gmail.com> >> Signed-off-by: Wanpeng Li <wanpengli@tencent.com> >> Cc: stable@vger.kernel.org >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> (cherry-picked from e97f852fd4561e77721bb9a4e0ea9d98305b1e93) > (cherry picked from commit e97f852fd4561e77721bb9a4e0ea9d98305b1e93) >> Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com> > > With the cherry-pick line above fixed: > > Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > >> --- +1 to fix cherry pick line. >> arch/x86/kvm/x86.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index c98ffb0e6b47..a3b6f780ad99 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -7225,7 +7225,8 @@ static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu) >> else { >> if (vcpu->arch.apicv_active) >> kvm_x86_ops->sync_pir_to_irr(vcpu); >> - kvm_ioapic_scan_entry(vcpu, vcpu->arch.ioapic_handled_vectors); >> + if (ioapic_in_kernel(vcpu->kvm)) >> + kvm_ioapic_scan_entry(vcpu, vcpu->arch.ioapic_handled_vectors); >> } >> >> if (is_guest_mode(vcpu)) > > >
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c98ffb0e6b47..a3b6f780ad99 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -7225,7 +7225,8 @@ static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu) else { if (vcpu->arch.apicv_active) kvm_x86_ops->sync_pir_to_irr(vcpu); - kvm_ioapic_scan_entry(vcpu, vcpu->arch.ioapic_handled_vectors); + if (ioapic_in_kernel(vcpu->kvm)) + kvm_ioapic_scan_entry(vcpu, vcpu->arch.ioapic_handled_vectors); } if (is_guest_mode(vcpu))