Re: [PATCH 17/24] KVM: x86/mmu: Take memory protection attributes into account during faults

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The new `kvm_faultin_memory_protections` trace event directly calls `kvm_rip_read(vcpu)` inside its `TP_fast_assign()` block. This introduces a dangerous side effect that mutates vCPU state and triggers a host-side DoS (via `WARN_ON_ONCE` and potential kernel panic) for confidential VMs like Intel TDX.
--

--- Patch [17]: [PATCH 17/24] KVM: x86/mmu: Take memory protection attributes into account during faults ---
commit df2f2adf06ac42cf5912c212a4b41b02bf9ceb82
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: x86/mmu: Take memory protection attributes into account during faults

This commit updates the x86 MMU fault handling to check memory protection
attributes. If a guest accesses memory with prohibited attributes, it
exits to userspace with -EFAULT. It also adds a trace event for these
memory protection faults.

> diff --git a/arch/x86/kvm/mmu/mmutrace.h b/arch/x86/kvm/mmu/mmutrace.h
> --- a/arch/x86/kvm/mmu/mmutrace.h
> +++ b/arch/x86/kvm/mmu/mmutrace.h
> @@ -447,6 +447,35 @@ TRACE_EVENT(
[ ... ]
> +TRACE_EVENT(kvm_faultin_memory_protections,
> +	TP_PROTO(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault,
> +		 unsigned access),
> +	TP_ARGS(vcpu, fault, access),
> +
> +	TP_STRUCT__entry(
> +		__field(unsigned int, vcpu_id)
> +		__field(unsigned long, guest_rip)
> +		__field(u64, fault_address)
> +		__field(bool, write)
> +		__field(bool, exec)
> +		__field(unsigned, access)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->vcpu_id = vcpu->vcpu_id;
> +		__entry->guest_rip = kvm_rip_read(vcpu);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
Does this code trigger a side-effect that crashes the host for confidential
VMs like Intel TDX?

Reading the instruction pointer dynamically via kvm_rip_read() inside a
tracepoint evaluates kvm_register_is_available(). If the register is not
marked available, it calls the architecture-specific cache_reg hook to
fetch it. 

For TDX guests, VCPU_REG_RIP is explicitly omitted from the available
register set. Consequently, kvm_rip_read() will fall back to calling
vt_cache_reg(), which executes a WARN_ON_ONCE(is_td_vcpu(vcpu)).

Since an untrusted TDX guest can intentionally trigger this fault by
accessing restricted memory, this allows a malicious guest to trigger a
host-side denial of service if tracing is enabled and panic_on_warn is set.

Should this use tracing_kvm_rip_read() instead to safely fetch the RIP
without triggering caching side-effects?

> +		__entry->fault_address = fault->gfn;
> +		__entry->write = fault->write;
> +		__entry->exec = fault->exec;
> +		__entry->access = access;
> +	),

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=17