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 <[email protected]> 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/[email protected]?part=4
