linux-kernel - Re: [PATCH v6 30/43] arm64: rme: Prevent Device mappings for Realms

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: <fb817590-19c4-4242-8942-14c9fa275f0c@arm.com>
Date: Fri, 7 Feb 2025 17:08:23 +0000
From: Steven Price <steven.price@....com>
To: Gavin Shan <gshan@...hat.com>, kvm@...r.kernel.org, kvmarm@...ts.linux.dev
Cc: Catalin Marinas <catalin.marinas@....com>, Marc Zyngier <maz@...nel.org>,
 Will Deacon <will@...nel.org>, James Morse <james.morse@....com>,
 Oliver Upton <oliver.upton@...ux.dev>,
 Suzuki K Poulose <suzuki.poulose@....com>, Zenghui Yu
 <yuzenghui@...wei.com>, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, Joey Gouly <joey.gouly@....com>,
 Alexandru Elisei <alexandru.elisei@....com>,
 Christoffer Dall <christoffer.dall@....com>, Fuad Tabba <tabba@...gle.com>,
 linux-coco@...ts.linux.dev,
 Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com>,
 Shanker Donthineni <sdonthineni@...dia.com>, Alper Gun
 <alpergun@...gle.com>, "Aneesh Kumar K . V" <aneesh.kumar@...nel.org>
Subject: Re: [PATCH v6 30/43] arm64: rme: Prevent Device mappings for Realms

On 02/02/2025 07:12, Gavin Shan wrote:
> On 12/13/24 1:55 AM, Steven Price wrote:
>> Physical device assignment is not yet supported by the RMM, so it
>> doesn't make much sense to allow device mappings within the realm.
>> Prevent them when the guest is a realm.
>>
>> Signed-off-by: Steven Price <steven.price@....com>
>> ---
>> Changes from v5:
>>   * Also prevent accesses in user_mem_abort()
>> ---
>>   arch/arm64/kvm/mmu.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
>> index 9ede143ccef1..cef7c3dcbf99 100644
>> --- a/arch/arm64/kvm/mmu.c
>> +++ b/arch/arm64/kvm/mmu.c
>> @@ -1149,6 +1149,10 @@ int kvm_phys_addr_ioremap(struct kvm *kvm,
>> phys_addr_t guest_ipa,
>>       if (is_protected_kvm_enabled())
>>           return -EPERM;
>>   +    /* We don't support mapping special pages into a Realm */
>> +    if (kvm_is_realm(kvm))
>> +        return -EINVAL;
>> +
> 
>         return -EPERM;
> 
>>       size += offset_in_page(guest_ipa);
>>       guest_ipa &= PAGE_MASK;
>>   @@ -1725,6 +1729,14 @@ static int user_mem_abort(struct kvm_vcpu
>> *vcpu, phys_addr_t fault_ipa,
>>       if (exec_fault && device)
>>           return -ENOEXEC;
>>   +    /*
>> +     * Don't allow device accesses to protected memory as we don't (yet)
>> +     * support protected devices.
>> +     */
>> +    if (device && kvm_is_realm(kvm) &&
>> +        kvm_gpa_from_fault(kvm, fault_ipa) == fault_ipa)
>> +        return -EINVAL;
>> +
> 
> s/kvm_is_realm/vcpu_is_rec
> 
> I don't understand the check very well. What I understood is mem_abort()
> is called
> only when kvm_gpa_from_fault(kvm, fault_ipa) != fault_ipa, meaning only
> the page
> faults in the shared address space is handled by mem_abort(). So I guess
> we perhaps
> need something like below.

private_memslot_fault() is only when the memslot is a private
guest_memfd(). So there's also the situation of a 'normal' memslot which
can still end up in user_mem_abort().

As things currently stand we can't really deal with this (the bottom
part of the IPA is protected, and therefore must come from
guest_memfd()). But in the future we are expecting to be able to support
protected devices.

So I think really the correct solution for now is to drop the "device"
check and update the comment to reflect the fact that
private_memslot_fault() should be handling all protected address until
we get support for assigning devices.

Thanks,
Steve

>     if (vcpu_is_rec(vcpu) && device)
>         return -EPERM;
> 
> kvm_handle_guest_abort
>   kvm_slot_can_be_private
>     private_memslot_fault    // page fault in the private space is
> handled here
>   io_mem_abort            // MMIO emulation is handled here
>   user_mem_abort                // page fault in the shared space is
> handled here
> 
>>       /*
>>        * Potentially reduce shadow S2 permissions to match the guest's
>> own
>>        * S2. For exec faults, we'd only reach this point if the guest
> 
> Thanks,
> Gavin
>