[SRU][PATCH 1/1] KVM: X86: Fix scan ioapic use-before-initialization

[Kleber Souza]

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 at canonical.com> wrote:
>>
>>> From: Wanpeng Li <wanpengli at 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 at gmail.com>
>>> Cc: Paolo Bonzini <pbonzini at redhat.com>
>>> Cc: Radim Krčmář <rkrcmar at redhat.com>
>>> Cc: Wei Wu <ww9210 at gmail.com>
>>> Signed-off-by: Wanpeng Li <wanpengli at tencent.com>
>>> Cc: stable at vger.kernel.org
>>> Signed-off-by: Paolo Bonzini <pbonzini at 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