Launching VM w/ AVIC disabled together with pass-through device
results in NULL pointer dereference bug with the following call trace.
RIP: 0010:svm_refresh_apicv_exec_ctrl+0x17e/0x1a0 [kvm_amd]
Call Trace:
kvm_vcpu_update_apicv+0x44/0x60 [kvm]
kvm_arch_vcpu_ioctl_run+0x3f4/0x1c80 [kvm]
kvm_vcpu_ioctl+0x3d8/0x650 [kvm]
do_vfs_ioctl+0xaa/0x660
? tomoyo_file_ioctl+0x19/0x20
ksys_ioctl+0x67/0x90
__x64_sys_ioctl+0x1a/0x20
do_syscall_64+0x57/0x190
entry_SYSCALL_64_after_hwframe+0x44/0xa9
Investigation shows that this is due to the uninitialized usage of
struct vapu_svm.ir_list in the svm_set_pi_irte_mode(), which is
called from svm_refresh_apicv_exec_ctrl().
The ir_list is initialized only if AVIC is enabled. So, fixes by
adding a check if AVIC is enabled in the svm_refresh_apicv_exec_ctrl().
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=206579
Fixes: 8937d762396d ("kvm: x86: svm: Add support to (de)activate posted interrupts.")
Signed-off-by: Suravee Suthikulpanit <[email protected]>
Cc: Alex Williamson <[email protected]>
---
arch/x86/kvm/svm.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 19035fb..1858455 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -5222,6 +5222,9 @@ static void svm_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu)
struct vmcb *vmcb = svm->vmcb;
bool activated = kvm_vcpu_apicv_active(vcpu);
+ if (!avic)
+ return;
+
if (activated) {
/**
* During AVIC temporary deactivation, guest could update
--
1.8.3.1
On Fri, 21 Feb 2020 08:52:17 -0600
Suravee Suthikulpanit <[email protected]> wrote:
> Launching VM w/ AVIC disabled together with pass-through device
> results in NULL pointer dereference bug with the following call trace.
>
> RIP: 0010:svm_refresh_apicv_exec_ctrl+0x17e/0x1a0 [kvm_amd]
>
> Call Trace:
> kvm_vcpu_update_apicv+0x44/0x60 [kvm]
> kvm_arch_vcpu_ioctl_run+0x3f4/0x1c80 [kvm]
> kvm_vcpu_ioctl+0x3d8/0x650 [kvm]
> do_vfs_ioctl+0xaa/0x660
> ? tomoyo_file_ioctl+0x19/0x20
> ksys_ioctl+0x67/0x90
> __x64_sys_ioctl+0x1a/0x20
> do_syscall_64+0x57/0x190
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> Investigation shows that this is due to the uninitialized usage of
> struct vapu_svm.ir_list in the svm_set_pi_irte_mode(), which is
> called from svm_refresh_apicv_exec_ctrl().
>
> The ir_list is initialized only if AVIC is enabled. So, fixes by
> adding a check if AVIC is enabled in the svm_refresh_apicv_exec_ctrl().
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=206579
> Fixes: 8937d762396d ("kvm: x86: svm: Add support to (de)activate posted interrupts.")
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> Cc: Alex Williamson <[email protected]>
> ---
> arch/x86/kvm/svm.c | 3 +++
> 1 file changed, 3 insertions(+)
Works for me, thanks Suravee!
Tested-by: Alex Williamson <[email protected]>
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 19035fb..1858455 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -5222,6 +5222,9 @@ static void svm_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu)
> struct vmcb *vmcb = svm->vmcb;
> bool activated = kvm_vcpu_apicv_active(vcpu);
>
> + if (!avic)
> + return;
> +
> if (activated) {
> /**
> * During AVIC temporary deactivation, guest could update
On 21/02/20 16:39, Alex Williamson wrote:
> On Fri, 21 Feb 2020 08:52:17 -0600
> Suravee Suthikulpanit <[email protected]> wrote:
>
>> Launching VM w/ AVIC disabled together with pass-through device
>> results in NULL pointer dereference bug with the following call trace.
>>
>> RIP: 0010:svm_refresh_apicv_exec_ctrl+0x17e/0x1a0 [kvm_amd]
>>
>> Call Trace:
>> kvm_vcpu_update_apicv+0x44/0x60 [kvm]
>> kvm_arch_vcpu_ioctl_run+0x3f4/0x1c80 [kvm]
>> kvm_vcpu_ioctl+0x3d8/0x650 [kvm]
>> do_vfs_ioctl+0xaa/0x660
>> ? tomoyo_file_ioctl+0x19/0x20
>> ksys_ioctl+0x67/0x90
>> __x64_sys_ioctl+0x1a/0x20
>> do_syscall_64+0x57/0x190
>> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>
>> Investigation shows that this is due to the uninitialized usage of
>> struct vapu_svm.ir_list in the svm_set_pi_irte_mode(), which is
>> called from svm_refresh_apicv_exec_ctrl().
>>
>> The ir_list is initialized only if AVIC is enabled. So, fixes by
>> adding a check if AVIC is enabled in the svm_refresh_apicv_exec_ctrl().
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=206579
>> Fixes: 8937d762396d ("kvm: x86: svm: Add support to (de)activate posted interrupts.")
>> Signed-off-by: Suravee Suthikulpanit <[email protected]>
>> Cc: Alex Williamson <[email protected]>
>> ---
>> arch/x86/kvm/svm.c | 3 +++
>> 1 file changed, 3 insertions(+)
>
> Works for me, thanks Suravee!
>
> Tested-by: Alex Williamson <[email protected]>
>
>>
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> index 19035fb..1858455 100644
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -5222,6 +5222,9 @@ static void svm_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu)
>> struct vmcb *vmcb = svm->vmcb;
>> bool activated = kvm_vcpu_apicv_active(vcpu);
>>
>> + if (!avic)
>> + return;
>> +
>> if (activated) {
>> /**
>> * During AVIC temporary deactivation, guest could update
>
Thanks to both of you. I'll get it to Linus next Monday.
Paolo