Re: [PATCH v7 1/2] arm64/sve: Split TIF_SVE into separate execute and register state flags

From: Mark Brown <broonie@kernel.org>
To: Dave Martin <Dave.Martin@arm.com>
Cc: Julien Grall <julien@xen.com>, Julien Grall <julien@xen.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Zhang Lei <zhang.lei@jp.fujitsu.com>,
	Will Deacon <will@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	Daniel Kiss <Daniel.Kiss@arm.com>
Subject: Re: [PATCH v7 1/2] arm64/sve: Split TIF_SVE into separate execute and register state flags
Date: Wed, 10 Feb 2021 19:52:41 +0000	[thread overview]
Message-ID: <20210210195241.GF4748@sirena.org.uk> (raw)
In-Reply-To: <20210209221627.GB4916@sirena.org.uk>

[-- Attachment #1.1: Type: text/plain, Size: 1344 bytes --]

On Tue, Feb 09, 2021 at 10:16:27PM +0000, Mark Brown wrote:
> On Tue, Feb 09, 2021 at 05:59:46PM +0000, Dave Martin wrote:

> > > @@ -952,8 +1022,9 @@ void do_sve_acc(unsigned int esr, struct pt_regs *regs)
> > >  	fpsimd_flush_task_state(current);
> > >  
> > >  	fpsimd_to_sve(current);

> > Hmmm, there's a latent bug upstream here: if the WARN() fires then
> > sve_state is not safe to dereference.  But we already did.

> > So perhaps this should have been something like:

> > 	if (!WARN_ON(test_and_set_thread_flag(TIF_SVE)))
> > 		fpsimd_to_sve();

> > This might make sense as a separate Fixes patch to precede the series.

> Yes, that's definitely a separate fix I think.

Actually now I look at this properly I think that we're sufficiently
confused if this happens that it's not worth worrying about avoiding the
conversion.  We already did a sve_alloc() which will have either
allocated SVE state or dereferenced a SVE state pointer that was there
and pointing to freed memory so we *might* be OK and if we're not then
it's too late to prevent anything and we'll exit with TIF_SVE which will
have a high likelyhood of leading to future dereferences even if we stop
this one.  It's not clear to me that we're helping by potentially
leaving stale data around so I'm inclined to leave things as they are.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel