Re: [PATCH RFC 4/6] arm64/efi: Honor EFI_MEMORY_ISA_MASK for Device-nGnRnE vs -nGnRE

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Silent truncation of the 16-bit EFI_MEMORY_ISA_MASK field into a u8 variable, bypassing validation and printing misleading warnings.
--

--- Patch [4]: [PATCH RFC 4/6] arm64/efi: Honor EFI_MEMORY_ISA_MASK for Device-nGnRnE vs -nGnRE ---
commit 6b01988498665d7f9d353c79435cf09a3768a624
Author: Sven Peter <sven@kernel.org>

arm64/efi: Honor EFI_MEMORY_ISA_MASK for Device-nGnRnE vs -nGnRE

This patch updates the mapping protection for EFI MMIO regions to honor the
MAIR attributes provided in EFI_MEMORY_ISA_MASK if EFI_MEMORY_ISA_VALID is
set. It defaults to Device-nGnRE for unsupported values or when the valid bit
is absent.

> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index e40885567f9e9..50ba8e9112fc8 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c

[ ... ]

> @@ -38,7 +40,27 @@ static __init ptval_t create_mapping_protection(efi_memory_desc_t *md)
>  	u32 type = md->type;
>  
>  	if (type == EFI_MEMORY_MAPPED_IO) {
> -		pgprot_t prot = __pgprot(PROT_DEVICE_nGnRE);
> +		pgprot_t prot;
> +
> +		if (attr & EFI_MEMORY_ISA_VALID) {
> +			u8 mair = FIELD_GET(EFI_MEMORY_ISA_MASK, attr);

[Severity: Medium]
Will this silently truncate the extracted field?

EFI_MEMORY_ISA_MASK covers 16 bits (bits 44-59). If the firmware provides an
attribute with non-zero bits in the upper 8 bits of this mask (like 0x0100),
assigning the result of FIELD_GET to a u8 will truncate it to 0x00.

This would cause the switch statement below to incorrectly match
MAIR_ATTR_DEVICE_nGnRnE (0x00) instead of falling through to the default
case. Should mair be a u16 or u32 instead?

> +
> +			switch (mair) {
> +			case MAIR_ATTR_DEVICE_nGnRnE:
> +				prot = __pgprot(PROT_DEVICE_nGnRnE);
> +				break;
> +			case MAIR_ATTR_DEVICE_nGnRE:
> +				prot = __pgprot(PROT_DEVICE_nGnRE);
> +				break;
> +			default:
> +				prot = __pgprot(PROT_DEVICE_nGnRE);
> +				pr_warn("unsupported MAIR attribute %#x for EFI MMIO region at 0x%llx, using Device-nGnRE\n",
> +					mair, md->phys_addr);

[Severity: Medium]
If truncation occurs above, will this warning print a misleading value?

Because mair is a u8, any unsupported attributes in the upper 8 bits of the
ISA mask will be stripped out before being printed here, obscuring the actual
unsupported bits provided by the firmware.

> +				break;
> +			}
> +		} else {
> +			prot = __pgprot(PROT_DEVICE_nGnRE);
> +		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-efi-psci-v1-0-9efb3abf0e4c@kernel.org?part=4