Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] `execmem_fill_trapping_insns()` writes trapping instructions using `memset32()` with a host-endian constant, producing incorrect/unallocated instructions on big-endian ARM64 kernels. - [High] `execmem_fill_trapping_insns()` fails to synchronize D/I caches to the Point of Unification (PoU), potentially causing execution of random uninitialized garbage. - [Low] The function `system_supports_bbml2_noabort()` is called but is completely undefined in the kernel tree. --
--- Patch [6]: [RFC PATCH 6/9] arm64: Enable the execmem ROX cache for module text --- commit 9f2855ae0653215694678b01c1ed088b2dce033c Author: Ard Biesheuvel <[email protected]> arm64: Enable the execmem ROX cache for module text This commit wires up execmem ROX cache support on arm64 for module text and init.text regions. This allows these regions to be allocated from a single chunk of memory in subsequent changes. > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c > index 3b9e1b8de6cbd..40088d362f6c7 100644 > --- a/arch/arm64/mm/init.c > +++ b/arch/arm64/mm/init.c [ ... ] > @@ -557,9 +558,25 @@ struct execmem_info __init *execmem_arch_setup(void) [ ... ] > }; > > + if (!system_supports_bbml2_noabort()) [Severity: Low] Does system_supports_bbml2_noabort() exist in the kernel tree? It appears this might cause a compilation error due to an undefined reference. > + execmem_info.ranges[EXECMEM_DEFAULT].flags |= > EXECMEM_NO_HUGE_VMAP; > + > return &execmem_info; > } > + > +void execmem_fill_trapping_insns(void *ptr, size_t size) > +{ > + memset32(ptr, AARCH64_BREAK_FAULT, size / sizeof(__le32)); [Severity: High] Does this correctly handle big-endian ARM64 kernels? Since AARCH64_BREAK_FAULT is a host-endian constant and instruction fetches are strictly little-endian, it looks like this might write an incorrect byte pattern and execute unallocated instructions instead of the intended trap if cpu_to_le32() isn't used. [Severity: High] Does this missing cache synchronization allow the CPU to fetch stale instructions? Writing instructions as data requires cleaning the D-cache and invalidating the I-cache to the Point of Unification (PoU) (e.g., using flush_icache_range()). Otherwise, the CPU might bypass the newly written trap instructions and fetch garbage from physical memory or the I-cache. > +} -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
