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

[Will Deacon]

On Fri, Jan 31, 2025 at 01:54:57PM +0000, Mark Rutland wrote:
> On Fri, Jan 31, 2025 at 12:41:04PM +0000, Will Deacon wrote:
> > 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.
> 
> I think that's misleading/stale and x6 specifically is not supposed to
> be clobbered. Shortly before that there's wording about preserving the
> registers, summary/history below:
> 
> * SMCCC v1.0 allowed x0 to x17 to be clobbered over a call; all other
>   GPRs had to be preserved.
> 
>   That was defined in ARM DEN 0028B, AKA "SMC CALLING CONVENTION":
>   https://developer.arm.com/documentation/den0028/b/?lang=en
> 
> * SMCCC v1.1 mandated that x4 to x17 were preserved. This was critical
>   for some of the ARCH_WORKAROUND_* calls made from kernel/kvm entry
>   asm.
> 
>   That was defined in ARM DEN 0070, AKA "Firmware interfaces for
>   mitigating cache speculation vulnerabilities":
> 
>   https://developer.arm.com/cache-speculation-vulnerability-firmware-specification

I can't find "DEN 0070" anywhere there...

> * SMCCC v1.2 relaxed things such that x4 to x17 must be preserved
>   *unless* they contained results of the function.
> 
>   That was defined in ARM DEN 0028C, AKA "SMC CALLING CONVENTION":
>   https://developer.arm.com/documentation/den0028/c/?lang=en
> 
>   .. which folded inthe requirements from ARM DEN 0070, with the
>   relaxation above, but also kept the stale not from ARM DEN 0028B.
> 
> In Linux we depend on x4 to x17 not being clobbered *unless* they're
> being used to return a result.
> 
> The current wording in the spec is messed up, and suggests that SMC32
> calls could clobber the upper 32 bits of x4 to x17, which is inccorect
> and would break the ARCH_WORKAROUND_* calls mentioend above (given those
> are SMC32). I'll go chase the SMCCC architects about that; it's clearly
> an unintentional error when reformatting the document.
> 
> The *intent* is that x4 to x17 are preserved here. AFAICT, nothing in
> arm_smccc_version_init() returns a value in x4 to x17. If any of those
> are clobbered for an SMCCCv1.1 call, it's definitely a firmware bug.

Perhaps, but I must admit that I have sympathy for the f/w developers
here given that the latest spec on the Arm website says that they can do
this and the spec that apparently requires the registers to be preserved
is either missing or hard to find. How are they supposed to know what
the intention was when the documentation definitively says something
else?

Emanuele -- could you hack the code to poison the other unused result
registers () and see if they are also cleared? ARM_SMCCC_TRNG_VERSION
looks like a 32-bit call, so that would be W1-W7 afaict.

Will