Hi Ryan,
On 06/05/2025 11:17, Catalin Marinas wrote:
On Tue, May 06, 2025 at 11:41:05AM +0200, Ard Biesheuvel wrote:
On Tue, 6 May 2025 at 10:16, Ryan Roberts ryan.roberts@arm.com wrote:
On 06/05/2025 09:09, Yeoreum Yun wrote:
On Sat, May 03, 2025 at 09:23:27PM +0100, Yeoreum Yun wrote: >> On Sat, May 03, 2025 at 11:16:12AM +0100, Catalin Marinas wrote: >>> On Fri, 02 May 2025 19:04:12 +0100, Yeoreum Yun wrote: >>>> create_init_idmap() could be called before .bss section initialization >>>> which is done in early_map_kernel(). >>>> Therefore, data/test_prot could be set incorrectly by PTE_MAYBE_NG macro. >>>> >>>> PTE_MAYBE_NG macro set NG bit according to value of "arm64_use_ng_mappings". >>>> and this variable places in .bss section. >>>> >>>> [...] >>> >>> Applied to arm64 (for-next/fixes), with some slight tweaking of the >>> comment, thanks! >>> >>> [1/1] arm64/cpufeature: annotate arm64_use_ng_mappings with ro_after_init to prevent wrong idmap generation >>> https://git.kernel.org/arm64/c/12657bcd1835 >> >> I'm going to drop this for now. The kernel compiled with a clang 19.1.5 >> version I have around (Debian sid) fails to boot, gets stuck early on: >> >> $ clang --version >> Debian clang version 19.1.5 (1) >> Target: aarch64-unknown-linux-gnu >> Thread model: posix >> InstalledDir: /usr/lib/llvm-19/bin >> >> I didn't have time to investigate, disassemble etc. I'll have a look >> next week. > > Just for your information. > When I see the debian package, clang 19.1.5-1 doesn't supply anymore: > - https://ftp.debian.org/debian/pool/main/l/llvm-toolchain-19/ > > and the default version for sid is below: > > $ clang-19 --version > Debian clang version 19.1.7 (3) > Target: aarch64-unknown-linux-gnu > Thread model: posix > InstalledDir: /usr/lib/llvm-19/bin > > When I tested with above version with arm64-linux's for-next/fixes > including this patch. it works well.
It doesn't seem to be toolchain related. It fails with gcc as well from Debian stable but you'd need some older CPU (even if emulated, e.g. qemu). It fails with Cortex-A72 (guest on Raspberry Pi 4) but not Neoverse-N2. Also changing the annotation from __ro_after_init to __read_mostly also works.
I think this is likely because __ro_after_init is also "ro before init" - i.e. if you try to write to it in the PI code an exception is generated due to it being mapped RO. Looks like early_map_kernel() is writiing to it.
Indeed.
I've noticed a similar problem in the past and it would be nice to fix it so that PI code maps __ro_after_init RW.
The issue is that the store occurs via the ID map, which only consists of one R-X and one RW- section. I'm not convinced that it's worth the hassle to relax this.
If moving the variable to .data works, then let's just do that.
Good to know there's no other more serious issue. I'll move this variable to __read_mostly.
It seems to fail in early_map_kernel() if RANDOMIZE_BASE is enabled.
Ahh that explains why Yeoreum Yun can't see the issue:
if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) { u64 kaslr_seed = kaslr_early_init(fdt, chosen);
if (kaslr_seed && kaslr_requires_kpti()) arm64_use_ng_mappings = true; kaslr_offset |= kaslr_seed & ~(MIN_KIMG_ALIGN - 1);
}
Thanks to clarify me to know. I've misread the address of arm64_use_ng_mapping :( so my brain works wrong.
Thanks again!
-- Sincerely, Yeoreum Yun