Re: [PATCH AUTOSEL 6.17-6.16] arm64: realm: ioremap: Allow mapping memory as encrypted

[Sasha Levin <sashal@kernel.org>]

On Thu, Oct 02, 2025 at 05:43:18PM +0100, Suzuki K Poulose wrote:
>Hello !
>
>On 02/10/2025 16:29, Sasha Levin wrote:
>>From: Suzuki K Poulose <suzuki.poulose@arm.com>
>>
>>[ Upstream commit fa84e534c3ec2904d8718a83180294f7b5afecc7 ]
>>
>>For ioremap(), so far we only checked if it was a device (RIPAS_DEV) to choose
>>an encrypted vs decrypted mapping. However, we may have firmware reserved memory
>>regions exposed to the OS (e.g., EFI Coco Secret Securityfs, ACPI CCEL).
>>We need to make sure that anything that is RIPAS_RAM (i.e., Guest
>>protected memory with RMM guarantees) are also mapped as encrypted.
>>
>>Rephrasing the above, anything that is not RIPAS_EMPTY is guaranteed to be
>>protected by the RMM. Thus we choose encrypted mapping for anything that is not
>>RIPAS_EMPTY. While at it, rename the helper function
>>
>>   __arm64_is_protected_mmio => arm64_rsi_is_protected
>>
>>to clearly indicate that this not an arm64 generic helper, but something to do
>>with Realms.
>>
>>Cc: Sami Mujawar <sami.mujawar@arm.com>
>>Cc: Will Deacon <will@kernel.org>
>>Cc: Catalin Marinas <catalin.marinas@arm.com>
>>Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
>>Cc: Steven Price <steven.price@arm.com>
>>Reviewed-by: Gavin Shan <gshan@redhat.com>
>>Reviewed-by: Steven Price <steven.price@arm.com>
>>Tested-by: Sami Mujawar <sami.mujawar@arm.com>
>>Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>Signed-off-by: Will Deacon <will@kernel.org>
>>Signed-off-by: Sasha Levin <sashal@kernel.org>
>>---
>>
>>LLM Generated explanations, may be completely bogus:
>
>Indeed, some are clearly incorrect.
>
>>
>>Based on my comprehensive analysis of this commit, I can now provide my
>>determination.
>>
>>## Analysis Summary
>>
>>### Code Change Analysis
>>
>>The commit makes a **critical logic change** in
>>`arch/arm64/kernel/rsi.c:104`:
>>
>>**Before**: `if (ripas != RSI_RIPAS_DEV) break;`
>>- Only returns true if **all** regions are RIPAS_DEV (device memory)
>>- Other states (RIPAS_RAM, RIPAS_DESTROYED) cause early exit → mapped as
>>   **decrypted**
>>
>>**After**: `if (ripas == RSI_RIPAS_EMPTY) break;`
>>- Returns true for RIPAS_RAM, RIPAS_DESTROYED, and RIPAS_DEV
>>- Only RIPAS_EMPTY (unprotected/shared) regions are mapped as
>>   **decrypted**
>>
>>### Problem Being Fixed
>>
>>The original implementation from commit 371589437616f (Oct 2024) only
>>encrypted RIPAS_DEV regions. However, **firmware-reserved memory
>>regions** use RIPAS_RAM state:
>>
>>- **EFI Coco Secret Securityfs** areas
>>- **ACPI CCEL** (Confidential Computing Event Log) tables
>>
>>Without this fix, these RIPAS_RAM regions are incorrectly mapped with
>>`pgprot_decrypted()`, which sets `PROT_NS_SHARED`, making them
>
>The Realm would have mapped them as decrypted and might have consumed
>untrusted information from (a malicious) hypervisor
>
>>**accessible to the untrusted hypervisor**.
>
>No, hypervisor doesn't get access to the protected data.
>
>>
>>### Security Impact
>>
>>This is a **security and data integrity bug**:
>>1. **Confidential data leakage**: Hypervisor can read protected firmware
>>    secrets
>
>Wrong
>
>>2. **Data corruption**: Hypervisor can modify what should be protected
>>    memory
>
>Absolutely NO
>
>>3. **Violation of ARM CCA guarantees**: Breaks confidential computing
>>    promises
>
>Not really. The Guest could consume "untrusted" data, thats the only
>violation here.

Thanks for the review! I'll drop it.

-- 
Thanks,
Sasha