linux-kernel - Re: [PATCH v3 1/3] KVM: arm64: Add pre_fault

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5cffcb56-26f7-4dac-a0ae-44329084d27d@gmail.com>
Date: Wed, 26 Nov 2025 17:14:05 +0000
From: "Thomson, Jack" <jackabt.amazon@...il.com>
To: Marc Zyngier <maz@...nel.org>
Cc: oliver.upton@...ux.dev, pbonzini@...hat.com, joey.gouly@....com,
 suzuki.poulose@....com, yuzenghui@...wei.com, catalin.marinas@....com,
 will@...nel.org, shuah@...nel.org, linux-arm-kernel@...ts.infradead.org,
 kvmarm@...ts.linux.dev, linux-kernel@...r.kernel.org,
 linux-kselftest@...r.kernel.org, isaku.yamahata@...el.com,
 xmarcalx@...zon.co.uk, kalyazin@...zon.co.uk, jackabt@...zon.com
Subject: Re: [PATCH v3 1/3] KVM: arm64: Add pre_fault_memory implementation



On 24/11/2025 12:54 pm, Marc Zyngier wrote:
> On Mon, 24 Nov 2025 11:34:38 +0000,
> Marc Zyngier <maz@...nel.org> wrote:
>>
>> On Wed, 19 Nov 2025 15:49:08 +0000,
>> Jack Thomson <jackabt.amazon@...il.com> wrote:
>>>
> 
> [...]
> 
>>> +	fault_info->hpfar_el2 = HPFAR_EL2_NS |
>>> +		FIELD_PREP(HPFAR_EL2_FIPA, ipa >> 12);
>>> +
>>> +	if (kvm_slot_has_gmem(memslot)) {
>>> +		ret = gmem_abort(vcpu, ipa, NULL, memslot, false);
>>> +	} else {
>>> +		hva = gfn_to_hva_memslot_prot(memslot, gfn, NULL);
>>> +		if (kvm_is_error_hva(hva)) {
>>> +			ret = -EFAULT;
>>> +			goto out;
>>> +		}
>>> +		ret = user_mem_abort(vcpu, ipa, NULL, memslot, &page_size, hva,
>>> +				     false);
>>> +	}
> 
> And thinking of it a bit more, this is completely broken. What happens
> if the vcpu is in a nested context? You just populate random pages in
> an IPA space that is not relevant at all, corrupting the guest state.
> 
> You must correctly handle the context the vcpu is in, instead of
> assuming that this is the canonical context. This means going via the
> *guest's* S2 translation, just like handle_mem_abort() does.
> 
> 	M.
> 

Thanks Marc for taking a look, I'll update to fix these issues and
address the other comments.

-- 
Thanks,
Jack