[SRU][Xenial][Bionic][Cosmic][Unstable][PATCH 0/1] Fix for CVE-2018-19407

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][Bionic][Cosmic][Unstable][PATCH 0/1] Fix for CVE-2018-19407

Khaled Elmously
Clean cherry-pick for all targets.

This CVE also affects Trusty but backporting the fix to Trusty is not straightforward so will be sent separately.

Wanpeng Li (1):
  KVM: X86: Fix scan ioapic use-before-initialization

 arch/x86/kvm/x86.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

Khaled Elmously
From: Wanpeng Li <[hidden email]>

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 <[hidden email]>
Cc: Paolo Bonzini <[hidden email]>
Cc: Radim Krčmář <[hidden email]>
Cc: Wei Wu <[hidden email]>
Signed-off-by: Wanpeng Li <[hidden email]>
Cc: [hidden email]
Signed-off-by: Paolo Bonzini <[hidden email]>
(cherry-picked from e97f852fd4561e77721bb9a4e0ea9d98305b1e93)
Signed-off-by: Khalid Elmously <[hidden email]>
---
 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))
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

Juerg Haefliger
On Mon,  3 Dec 2018 21:22:38 -0500
Khalid Elmously <[hidden email]> wrote:

> From: Wanpeng Li <[hidden email]>
>
> 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 <[hidden email]>
> Cc: Paolo Bonzini <[hidden email]>
> Cc: Radim Krčmář <[hidden email]>
> Cc: Wei Wu <[hidden email]>
> Signed-off-by: Wanpeng Li <[hidden email]>
> Cc: [hidden email]
> Signed-off-by: Paolo Bonzini <[hidden email]>
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 <[hidden email]>
> ---
>  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))

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

attachment0 (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [SRU][Xenial][Bionic][Cosmic][Unstable][PATCH 0/1] Fix for CVE-2018-19407

Seth Forshee
In reply to this post by Khaled Elmously
On Mon, Dec 03, 2018 at 09:22:37PM -0500, Khalid Elmously wrote:
> Clean cherry-pick for all targets.
>
> This CVE also affects Trusty but backporting the fix to Trusty is not straightforward so will be sent separately.

Applied to disco/master-next with the "cherry picked from ..." line
fixed. Thanks!

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

Seth Forshee
In reply to this post by Juerg Haefliger
On Tue, Dec 04, 2018 at 12:14:08PM +0100, Juerg Haefliger wrote:

> On Mon,  3 Dec 2018 21:22:38 -0500
> Khalid Elmously <[hidden email]> wrote:
>
> > From: Wanpeng Li <[hidden email]>
> >
> > 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 <[hidden email]>
> > Cc: Paolo Bonzini <[hidden email]>
> > Cc: Radim Krčmář <[hidden email]>
> > Cc: Wei Wu <[hidden email]>
> > Signed-off-by: Wanpeng Li <[hidden email]>
> > Cc: [hidden email]
> > Signed-off-by: Paolo Bonzini <[hidden email]>
>
> 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.


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

Colin Ian King-2
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 <[hidden email]> wrote:
>>
>>> From: Wanpeng Li <[hidden email]>
>>>
>>> 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 <[hidden email]>
>>> Cc: Paolo Bonzini <[hidden email]>
>>> Cc: Radim Krčmář <[hidden email]>
>>> Cc: Wei Wu <[hidden email]>
>>> Signed-off-by: Wanpeng Li <[hidden email]>
>>> Cc: [hidden email]
>>> Signed-off-by: Paolo Bonzini <[hidden email]>
>>
>> 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

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

Stefan Bader-2
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 <[hidden email]> wrote:
>>>
>>>> From: Wanpeng Li <[hidden email]>
>>>>
>>>> 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 <[hidden email]>
>>>> Cc: Paolo Bonzini <[hidden email]>
>>>> Cc: Radim Krčmář <[hidden email]>
>>>> Cc: Wei Wu <[hidden email]>
>>>> Signed-off-by: Wanpeng Li <[hidden email]>
>>>> Cc: [hidden email]
>>>> Signed-off-by: Paolo Bonzini <[hidden email]>
>>>
>>> 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


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Kleber Souza
In reply to this post by Seth Forshee
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 <[hidden email]> wrote:
>>
>>> From: Wanpeng Li <[hidden email]>
>>>
>>> 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 <[hidden email]>
>>> Cc: Paolo Bonzini <[hidden email]>
>>> Cc: Radim Krčmář <[hidden email]>
>>> Cc: Wei Wu <[hidden email]>
>>> Signed-off-by: Wanpeng Li <[hidden email]>
>>> Cc: [hidden email]
>>> Signed-off-by: Paolo Bonzini <[hidden email]>
>> 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


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

Seth Forshee
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 <[hidden email]> wrote:
> >>
> >>> From: Wanpeng Li <[hidden email]>
> >>>
> >>> 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 <[hidden email]>
> >>> Cc: Paolo Bonzini <[hidden email]>
> >>> Cc: Radim Krčmář <[hidden email]>
> >>> Cc: Wei Wu <[hidden email]>
> >>> Signed-off-by: Wanpeng Li <[hidden email]>
> >>> Cc: [hidden email]
> >>> Signed-off-by: Paolo Bonzini <[hidden email]>
> >> 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.

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team