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**:
- **Confidential data leakage**: Hypervisor can read protected firmware secrets
Wrong
- **Data corruption**: Hypervisor can modify what should be protected memory
Absolutely NO
- **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.