lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aBT+Hlp3h/9UFe0N@e129823.arm.com>
Date: Fri, 2 May 2025 18:17:18 +0100
From: Yeoreum Yun <yeoreum.yun@....com>
To: Nathan Chancellor <nathan@...nel.org>
Cc: catalin.marinas@....com, will@...nel.org,
	nick.desaulniers+lkml@...il.com, morbo@...gle.com,
	justinstitt@...gle.com, broonie@...nel.org, maz@...nel.org,
	oliver.upton@...ux.dev, frederic@...nel.org, joey.gouly@....com,
	james.morse@....com, hardevsinh.palaniya@...iconsignals.io,
	shameerali.kolothum.thodi@...wei.com, ardb@...nel.org,
	ryan.roberts@....com, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, llvm@...ts.linux.dev,
	stable@...r.kernel.org
Subject: Re: [PATCH] arm64/cpufeature: annotate arm64_use_ng_mappings with
 ro_after_init to prevent wrong idmap generation

Hi Nathan,

> Hi Yeoreum,
>
> On Fri, May 02, 2025 at 03:57:55PM +0100, Yeoreum Yun wrote:
> > create_init_idmap() could be called before .bss section initialization
> > which is done in early_map_kernel() since data/test_prot could be set
> > wrong for PTE_MAYBE_NG macro.
> >
> > PTE_MAYBE_NG macro is set according to value of "arm64_use_ng_mappings".
> > and this variable is located in .bss section.
> >
> >    # llvm-objdump-21 --syms vmlinux-gcc | grep arm64_use_ng_mappings
> >      ffff800082f242a8 g O .bss    0000000000000001 arm64_use_ng_mappings
> >
> > If .bss section doesn't initialized, "arm64_use_ng_mappings" would be set
> > with garbage value ant then the text_prot or data_prot could be set
> > with garbgae value.
> >
> > Here is what i saw with kernel compiled via llvm-21
> >
> >   // create_init_idmap()
> >   ffff80008255c058: d10103ff     	sub	sp, sp, #0x40
> >   ffff80008255c05c: a9017bfd     	stp	x29, x30, [sp, #0x10]
> >   ffff80008255c060: a90257f6     	stp	x22, x21, [sp, #0x20]
> >   ffff80008255c064: a9034ff4     	stp	x20, x19, [sp, #0x30]
> >   ffff80008255c068: 910043fd     	add	x29, sp, #0x10
> >   ffff80008255c06c: 90003fc8     	adrp	x8, 0xffff800082d54000
> >   ffff80008255c070: d280e06a     	mov	x10, #0x703     // =1795
> >   ffff80008255c074: 91400409     	add	x9, x0, #0x1, lsl #12 // =0x1000
> >   ffff80008255c078: 394a4108     	ldrb	w8, [x8, #0x290] ------------- (1)
> >   ffff80008255c07c: f2e00d0a     	movk	x10, #0x68, lsl #48
> >   ffff80008255c080: f90007e9     	str	x9, [sp, #0x8]
> >   ffff80008255c084: aa0103f3     	mov	x19, x1
> >   ffff80008255c088: aa0003f4     	mov	x20, x0
> >   ffff80008255c08c: 14000000     	b	0xffff80008255c08c <__pi_create_init_idmap+0x34>
> >   ffff80008255c090: aa082d56     	orr	x22, x10, x8, lsl #11 -------- (2)
> >
> > Note (1) is load the arm64_use_ng_mappings value in w8.
> > and (2) is set the text or data prot with the w8 value to set PTE_NG bit.
> > If .bss section doesn't initialized, x8 can include garbage value
> > -- In case of some platform, x8 loaded with 0xcf -- it could generate
> > wrong mapping. (i.e) text_prot is expected with
> > PAGE_KERNEL_ROX(0x0040000000000F83) but
> > with garbage x8 -- 0xcf, it sets with (0x0040000000067F83)
> > and This makes boot failure with translation fault.
> >
> > This error cannot happen according to code generated by compiler.
> > here is the case of gcc:
> >
> >    ffff80008260a940 <__pi_create_init_idmap>:
> >    ffff80008260a940: d100c3ff      sub     sp, sp, #0x30
> >    ffff80008260a944: aa0003ed      mov     x13, x0
> >    ffff80008260a948: 91400400      add     x0, x0, #0x1, lsl #12 // =0x1000
> >    ffff80008260a94c: a9017bfd      stp     x29, x30, [sp, #0x10]
> >    ffff80008260a950: 910043fd      add     x29, sp, #0x10
> >    ffff80008260a954: f90017e0      str     x0, [sp, #0x28]
> >    ffff80008260a958: d00048c0      adrp    x0, 0xffff800082f24000 <reset_devices>
> >    ffff80008260a95c: 394aa000      ldrb    w0, [x0, #0x2a8]
> >    ffff80008260a960: 37000640      tbnz    w0, #0x0, 0xffff80008260aa28 <__pi_create_init_idmap+0xe8> ---(3)
> >    ffff80008260a964: d280f060      mov     x0, #0x783      // =1923
> >    ffff80008260a968: d280e062      mov     x2, #0x703      // =1795
> >    ffff80008260a96c: f2e00800      movk    x0, #0x40, lsl #48
> >    ffff80008260a970: f2e00d02      movk    x2, #0x68, lsl #48
> >    ffff80008260a974: aa2103e4      mvn     x4, x1
> >    ffff80008260a978: 8a210049      bic     x9, x2, x1
> >    ...
> >    ffff80008260aa28: d281f060      mov     x0, #0xf83      // =3971
> >    ffff80008260aa2c: d281e062      mov     x2, #0xf03      // =3843
> >    ffff80008260aa30: f2e00800      movk    x0, #0x40, lsl #48
> >
> > In case of gcc, according to value of arm64_use_ng_mappings (annoated as(3)),
> > it branches to each prot settup code.
>
> > However this is also problem since it branches according to garbage
> > value too -- idmapping with wrong pgprot.
> >
> > To resolve this, annotate arm64_use_ng_mappings as ro_after_init.
> >
> > Fixes: 84b04d3e6bdb ("arm64: kernel: Create initial ID map from C code")
> > Cc: <stable@...r.kernel.org> # 6.9.x
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@....com>
> > ---
>
> This appears to resolve the issue that I reported to LLVM upstream:
>
> https://github.com/llvm/llvm-project/issues/138019
>
> Tested-by: Nathan Chancellor <nathan@...nel.org>
>
> It does not look like there is anything for the compiler to fix in this
> case, correct?

No. There's no need any change in compiler.

Thanks!
>
> > ---
> >  arch/arm64/kernel/cpufeature.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > index d2104a1e7843..967ffb1cbd52 100644
> > --- a/arch/arm64/kernel/cpufeature.c
> > +++ b/arch/arm64/kernel/cpufeature.c
> > @@ -114,7 +114,7 @@ static struct arm64_cpu_capabilities const __ro_after_init *cpucap_ptrs[ARM64_NC
> >
> >  DECLARE_BITMAP(boot_cpucaps, ARM64_NCAPS);
> >
> > -bool arm64_use_ng_mappings = false;
> > +bool arm64_use_ng_mappings __ro_after_init = false;
> >  EXPORT_SYMBOL(arm64_use_ng_mappings);
> >
> >  DEFINE_PER_CPU_READ_MOSTLY(const char *, this_cpu_vector) = vectors;
> > --
> > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
> >
> >

--
Sincerely,
Yeoreum Yun

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ