Received-SPF: pass (google.com: domain of linux-kernel+bounces-44731-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Tue, 30 Jan 2024 14:09:34 +0000
From: Mark Brown <broonie@kernel.org>
To: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Dave Martin <dave.martin@arm.com>,
	linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH] arm64/signal: Don't assume that TIF_SVE means we saved
 SVE state
Message-ID: <b0ed6698-bc65-48fb-96b8-0cd077448196@sirena.org.uk>
References: <20240119-arm64-sve-signal-regs-v1-1-b9fd61b0289a@kernel.org>
 <20240130115107.GB13551@willie-the-truck>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="uWPAAXNST8xADQkB"
Content-Disposition: inline
In-Reply-To: <20240130115107.GB13551@willie-the-truck>


--uWPAAXNST8xADQkB
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Jan 30, 2024 at 11:51:07AM +0000, Will Deacon wrote:
> On Fri, Jan 19, 2024 at 12:29:13PM +0000, Mark Brown wrote:

> > -	if (test_thread_flag(TIF_SVE))
> > +	if (current->thread.fp_type == FP_STATE_SVE)
> >  		sve_to_fpsimd(current);
> >  }

> I don't think this hunk applies on -rc2 ^^.

Hrm, git seemed to figure out a rebase with no intervention - I've
thrown it at my CI and will resend assuming no changes from the rest of
the discussion.

> > -		if (add_all || test_thread_flag(TIF_SVE) ||
> > +		if (add_all || current->thread.fp_type == FP_STATE_SVE ||
> >  		    thread_sm_enabled(&current->thread)) {
> >  			int vl = max(sve_max_vl(), sme_max_vl());

> I think this code is preemptible, so I'm struggling to understand what
> happens if the fp_type changes under our feet as a result of a context
> switch.

We are relying here on having forced a flush of the floating point
register state prior to this code running, simple preemption won't
change the state from what was already saved.  The same consideration
also applies to the check for streaming mode here.

That said if this is preempted ptrace *could* come in and rewrite the
data or at worst change the vector length (which could leave us with
sve_state deallocated or a different size, possibly while we're in the
middle of accessing it).  This could also happen with the existing check
for TIF_SVE so I don't think there's anything new here - AFAICT this has
always been an issue with the vector code, unless I'm missing some
bigger thing which excludes ptrace.  I think any change that's needed
there won't overlap with this one, I'm looking.

--uWPAAXNST8xADQkB
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmW5Ax0ACgkQJNaLcl1U
h9CV/Qf+M+B80WOMZNgHU9dCZZG69Aykjj55IozuDVFkd5exazmMfTEW34Ojr6Ab
Oqj5DCHsru08tlJPGg1fgUKbx/g2vRP/rwLathQOZVwHKomcbc9P56EzHduVn43a
fXecMSex+3LDm85+En5p2pCDrDyHEthdHdLALVM1QhEVbKGtsV/UpAH8OmQaPL7m
L6fMUh8qdP8gRZSFyXfCRhJDbmAy1Y2wVxiX1wAuee2UcFF0e8h6W8zamC6h9Lf7
PlwmVqT3NKan89/f61Fhckh//vw6/B9y/MFTthhCUz9W8m07AZ27xqd+ZNsF1Vsm
i/eugtrfijoSaHKnYWMVE8ZuILjNzA==
=7Na8
-----END PGP SIGNATURE-----

--uWPAAXNST8xADQkB--