Re: [BUG] ARM64 regression: NULL pointer dereference in arm_smccc_version_init+0x90/0x1ac

[Will Deacon <will@kernel.org>]

On Thu, Jan 30, 2025 at 03:56:10PM +0100, Emanuele Rocca wrote:
> On 2025-01-30 12:19, Mark Rutland wrote:
> > For the benefit others, when we looked into this a few days ago it
> > appeared that a GPR was being clobbered across an SMCCC call, resulting
> > in a later crash (as that GPR should hold the ADRP'd base address of
> > 'smccc_version'). I didn't have the time to dig more into that (e.g.  to
> > figure out whether kernel/compiler/firmware was to blame).
> > 
> > Emanuele, could you please dump the result of:
> > 
> >   objdump --disassemble=arm_smccc_version_init vmlinux
> > 
> > ... for this kernel? That'd make it possible for others to
> > perform/verify the analysis I mentioned above.
> 
> Sure, here it is:
> https://people.debian.org/~ema/Z5OpGluX6oX5NLxh@NH27D9T0LF.objdump.txt
> 
> > If you can share any more details from the crash, that'd be helpful. The
> > GPR dump would be *enormously* helpful in this case, and even a photo of
> > the crash log might be useful.
> 
> pc : arm_smccc_version_init+0x90/0x1ac
> lr : psci_probe+0x1fc/0x2c0
> sp : ffffa3d8eea33ca0
> x29: ffffa3d8eea33ce0 x28: 0000000081000200 x27: ffffa3d8edd72a80
> x26: ffffa3d8edd728b0 x25: ffffa3d8eec66248 x24: ffffa3d8edc528d8
> x23: ffffa3d8eed7f4f8 x22: 0000000000000001 x21: ffffa3d8eea58410
> x20: ffffa3d8eed7f000 x19: 0000000000010001 x18: 0000000000000006
> x17: 0000000009f69730 x16: 00000008760968d0 x15: 0000000000000000
> x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
> x11: 00000000ffffefff x10: ffffa3d8eeac09c8 x9 : ffffa3d8eea687d8
> x8 : ffffa3d8eea33cb8 x7 : 0000000000000000 x6 : 0000000000000000 
> x5 : ffffa3d8eed7f000 x4 : ffffa3d8edeb0000 x3 : 0000000000000000
> x2 : 0000000000000000 x1 : 0000000000000001 x0 : 0000000000000000
> 
> Call trace:
>  arm_smccc_version_init+0x90/0x1ac
>  psci_0_2_init+0x20/0x2c
>  psci_1_0_init+0x1c/0x58
>  psci_dt_init+0x6c/0x98
>  setup_arch+0x400/0x5ec
>  start_kernel+0x90/0x788
>  __primary_switched+0xbc/0xc4
> Code: d29fffe1 eb01001f 1a9f97e0 f0ffd384 (b94264c2)
> 
> The above was transcribed by hand. I double-checked, but there may be
> mistakes.
> 
> Photo here: https://people.debian.org/~ema/Z5OpGluX6oX5NLxh@NH27D9T0LF.jpg

Crikey, you transcribed all that junk by _hand_?! Thanks for the effort,
but next time please don't waste your time! The screenshot is fine.

Anyway, it looks like x6 gets trashed across the SMC. I think that's fine
per the SMCCC spec:

  | Unused result and scratch registers can leak information after an
  | SMC or HVC call. An implementation can mitigate this risk by either
  | preserving the register state over the call, or returning a constant
  | value,such as zero, in each register.

so the question is whether or not the compiler should be relying on the
register being preserved. It looks to me like the arm_smccc_1_1_smc()
macro is actually broken in this regard: it constructs the input list
based on the number of arguments passed and doesn't take the above into
account at all.

So I think we should change that so unused arguments are clobbered
instead. Mark -- what do you think?

Will