[PATCH v2 2/4] arm64/signal: Include TPIDR2 in the signal context

[Will Deacon]

On Tue, Nov 15, 2022 at 04:42:14PM +0000, Mark Brown wrote:
> On Tue, Nov 15, 2022 at 12:22:26PM +0000, Will Deacon wrote:
> > On Tue, Nov 15, 2022 at 12:08:44PM +0000, Mark Brown wrote:
> > > On Mon, Nov 14, 2022 at 04:10:06PM +0000, Will Deacon wrote:
> > > > On Mon, Oct 31, 2022 at 08:17:34PM +0000, Mark Brown wrote:
> 
> > > > > +			if (user->tpidr2)
> > > > > +				goto invalid;
> 
> > > > > +			if (size != sizeof(*user->tpidr2))
> 
> > > > Why are you requiring an exact match here? Won't that hinder any future
> > > > extension of the structure?
> 
> > > It will but since the structure is explicitly for a single sysreg
> > > that's intentional - the thinking was to just continue to model
> > > any more sysregs we want to report in the signal context in the
> > > same format with their own contexts.  It felt like it fit better
> > > into how everything else in the signal context is extended.
> 
> > I see, but having the usual '<' check wouldn't preclude us from doing
> > what you suggest above, whilst also giving us some flexibility in case
> > things turn out differently from how we expected.
> 
> This actually also how we validate the base fpsimd_context -
> while there is a < check in the switch statement in
> parse_user_sigframe() but we also have an exact size check
> near the top of restore_fpsimd_context() which gets called from
> there, meaning that the check in parse_user_sigframe() is a bit
> redundant.  We do however allow the varibly sized frames to have
> an oversized allocation, though those have internal sizing
> information whereas fpsimd_context doesn't.  My take was that we
> were erroring out here because if userspace thinks it's supplying
> some state that we're ignoring and not restoring then things
> might go badly.  I'm not super wedded to this approach but it is
> consistent with the fpsimd_context handling and I can see some
> justificaton for it being done the way it is.

Hmm, good point about fpsimd, it looks at magic/size twice which is
definitely wrong (userspace could even change those values in between!).

So I'd vote for removing the checks from restore_fpsimd_context() which
raises the same question we were discussing initially: should the check
in parse_user_sigframe() require an exact size match or instead truncate
the structure on the stack by only copying a prefix into the kernel?

I'm actually warming more towards an exact check now that we've spoken
about it a bit... What do you think?

Will