linux-kernel - Re: arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference of noderef expression

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YpeZ0M5we/0yzxVC@arm.com>
Date:   Wed, 1 Jun 2022 17:54:40 +0100
From:   Catalin Marinas <catalin.marinas@....com>
To:     Mark Brown <broonie@...nel.org>
Cc:     kernel test robot <lkp@...el.com>, kbuild-all@...ts.01.org,
        linux-kernel@...r.kernel.org
Subject: Re: arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference
 of noderef expression

On Wed, Jun 01, 2022 at 09:50:39AM +0200, Mark Brown wrote:
> On Tue, May 31, 2022 at 05:57:42PM +0100, Catalin Marinas wrote:
> > This looks wrong indeed. IIUC, something like below but waiting for Mark
> > to confirm:
> > 
> > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> > index 819979398127..ac521b3403d6 100644
> > --- a/arch/arm64/kernel/fpsimd.c
> > +++ b/arch/arm64/kernel/fpsimd.c
> > @@ -1965,7 +1965,7 @@ void __efi_fpsimd_end(void)
> >  							   0,
> >  							   SVCR_SM_MASK);
> >  					if (!system_supports_fa64())
> > -						ffr = efi_sm_state;
> > +						ffr = false;
> 
> Yes, that looks right - feel free to add my Reviewed-by or I'll
> do that when I see the patch sent properly.

I'm no longer convinced that's the right fix. In __efi_fpsimd_begin(),
ffr is true if either !system_supports_fa64() or SVCR.SM is set. The FFR
state would be saved in this case.

In __efi_fpsimd_end(), shouldn't we just restore the FFR state if
efi_sm_state, irrespective of system_supports_fa64() (as it was checked
in the beginning). Something like:

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 819979398127..8bc9080adab2 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -1960,13 +1960,12 @@ void __efi_fpsimd_end(void)
 			 * streaming mode.
 			 */
 			if (system_supports_sme()) {
-				if (__this_cpu_read(efi_sm_state)) {
+				if (__this_cpu_read(efi_sm_state))
 					sysreg_clear_set_s(SYS_SVCR,
 							   0,
 							   SVCR_SM_MASK);
-					if (!system_supports_fa64())
-						ffr = efi_sm_state;
-				}
+				else
+					ffr = false;
 			}
 
 			sve_load_state(sve_state + sve_ffr_offset(sve_max_vl()),

-- 
Catalin