Re: [PATCH v4 3/5] KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] kvm_deliver_exception_payload() incorrectly clears previously accumulated active-low DR6 events (such as DR6_BUS_LOCK and DR6_RTM) upon subsequent #DB exceptions.
--

--- Patch [3]: [PATCH v4 3/5] KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed() ---
commit eaefa84a8068404a2e75f11446b9fa345fef2bae
Author: Shivansh Dhiman <shivansh.dhiman@amd.com>

KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed()

When preparing vmcb02 for nested VMRUN, KVM ORs DR6_ACTIVE_LOW into the
guest DR6 to force the fixed bits to 1. DR6_ACTIVE_LOW forces bit 11
(DR6_BUS_LOCK) to 1 unconditionally.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 8d1967f5c099f..9b1afa47c54c9 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -810,7 +810,7 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
>  
>  	if (unlikely(new_vmcb12 || vmcb12_is_dirty(control, VMCB_DR))) {
>  		vmcb02->save.dr7 = svm->nested.save.dr7 | DR7_FIXED_1;
> -		svm->vcpu.arch.dr6  = svm->nested.save.dr6 | DR6_ACTIVE_LOW;
> +		svm->vcpu.arch.dr6  = svm->nested.save.dr6 | kvm_dr6_fixed(vcpu);

[Severity: High]
This isn't a bug introduced by this patch, but kvm_deliver_exception_payload()
in arch/x86/kvm/x86.c has the exact same flaw with DR6_ACTIVE_LOW.

Could it be fixed using the newly exported kvm_dr6_fixed() helper?

Currently, kvm_deliver_exception_payload() executes:

    case DB_VECTOR:
        ...
        vcpu->arch.dr6 |= DR6_ACTIVE_LOW;
        vcpu->arch.dr6 |= ex->payload;
        vcpu->arch.dr6 ^= ex->payload & DR6_ACTIVE_LOW;

This unconditionally forces all active-low bits to 1 before applying the
current exception payload, destroying any previously accumulated 0 bits.

If a guest OS relies on the Bus Lock Detect #DB to mitigate DoS attacks,
and another #DB event (like a Single Step) occurs before the guest clears
DR6, kvm_deliver_exception_payload() will force DR6 bit 11 back to 1.

Because the new #DB payload does not have the bus lock bit set, the
subsequent XOR leaves bit 11 as 1.

Does this destroy the accumulated event and allow a malicious task to evade
detection?

Could this be updated to use kvm_dr6_fixed(vcpu) instead of DR6_ACTIVE_LOW
to avoid clearing previously accumulated active-low DR6 events?

>  		vmcb_mark_dirty(vmcb02, VMCB_DR);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721050600.87268-1-shivansh.dhiman@amd.com?part=3