Re: [PATCH v3 04/11] arm64/mm: Add set_memory_device() and set_memory_normal()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Misuse of aggregate PROT_* macros as masks corrupts memory types, permissions, and page table block structures.
- [High] Modifying live memory attributes (type/cacheability) without a Break-Before-Make (BBM) sequence violates ARM64 architecture invariants.
- [High] Bypasses `can_set_direct_map()`, alignment checks, and boundary validation, leading to crashes on non-splittable linear maps.
--

commit 4153df151684415d2f2a476b7fbb71cc99c64650
Author: Chun Ng <chunn@nvidia.com>

arm64/mm: Add set_memory_device() and set_memory_normal()

Add helpers to swap PROT_NORMAL and PROT_DEVICE_nGnRnE protection bits
on a kernel-linear-map range.

> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
> index bbe98ac9ad8c6..871b59a6c9eaf 100644
> --- a/arch/arm64/mm/pageattr.c
> +++ b/arch/arm64/mm/pageattr.c
> @@ -251,6 +251,22 @@ int set_memory_valid(unsigned long addr, int numpages, int enable)
>  					__pgprot(PTE_PRESENT_VALID_KERNEL));
>  }
>  
> +int set_memory_device(unsigned long addr, int numpages)
> +{

[Severity: High]
Does this bypass necessary linear map validation checks?

Unlike other helpers such as set_direct_map_invalid_noflush(), these functions
do not call can_set_direct_map(). If the kernel is booted with configurations
preventing page-level splitting (like rodata=off), the modification could fail
or corrupt perfectly aligned PMD block mappings.

Additionally, should this validate address alignment and check __is_lm_address()
before modifying the memory, similar to change_memory_common()?

> +	return __change_memory_common(addr, PAGE_SIZE * numpages,
> +				__pgprot(PROT_DEVICE_nGnRnE),
> +				__pgprot(PROT_NORMAL));

[Severity: Critical]
Will passing aggregate PROT_* macros as clear_mask and set_mask corrupt memory
types and page table structures?

When __change_memory_common() uses the clear_mask, it clears the bits present in
the mask. PROT_NORMAL contains memory type index 0 (MT_NORMAL). Clearing 0
leaves existing memory type bits intact, meaning the subsequent OR of
PROT_DEVICE_nGnRnE (index 3) will combine with the old bits, potentially
resulting in an invalid memory type index.

Also, both PROT_NORMAL and PROT_DEVICE_nGnRnE include PROT_DEFAULT, which has
PTE_TYPE_PAGE (binary 11). If this is applied to a PMD or PUD block mapping
(which has descriptor type PMD_TYPE_SECT, binary 01), clearing and setting
these bits will mutate a valid block descriptor into a table descriptor, causing
a translation fault. Should this use explicit PTE_ATTRINDX masks instead?

[Severity: High]
Does changing the memory type from Normal to Device require a Break-Before-Make
(BBM) sequence?

The ARM Architecture Reference Manual requires a BBM sequence when updating
a live page table entry to change its memory attributes. __change_memory_common()
modifies PTEs in-place via read-modify-write (update_range_prot) and issues
a TLB invalidation only after the writes complete.

Skipping the intermediate invalidation step (writing an invalid entry, issuing
DSB/TLBI, and a second DSB before writing the new attribute) can cause hardware
coherency issues and TLB Conflict Aborts.

> +}
> +EXPORT_SYMBOL_GPL(set_memory_device);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-tegra-vpr-v3-0-d80f7b871bb4@nvidia.com?part=4