Re: [PATCH] arm64/signal: Silence spurious sparse warning storing GCSPR_EL0

[Mark Brown]

On Tue, Dec 10, 2024 at 04:35:57PM +0000, Mark Rutland wrote:
> On Tue, Dec 10, 2024 at 03:44:29PM +0000, Mark Brown wrote:

> > The spuriousness is arguable, from my point of view it's spurious in
> > that we don't have the type of the system register we're writing to.

> All that I'm asking for here is a trivial rewording; make the title say
> something like:

Yes, I had already removed the references to spurious and false positive
locally and changed the unsigned long cast to a __force u64 cast.

> > I find it both safer and clearer to keep values which are userspace
> > pointers as userspace pointers rather than working with them as
> > integers, using integers just sets off alarm bells.  

> Having casts strewn throughout the code sets off more alarm bells for
> me.

With the new code there's only a cast when we store the value to the
register, which is the point where we're discarding the type safety.

> > > Similarly in map_shadow_stack(), it'd be simpler to treat cap_ptr as an
> > > integer type.

> > With map_shadow_stack() it's a bit of an issue with letting users
> > specify a size but yeah, we could do better there.

> I don't follow. The only place where size interacts with cap_ptr is when
> we initialize cap_ptr, and there we're adding size to an integer type:

> 	cap_ptr = (unsigned long __user *)(addr + size -
> 					   (cap_offset * sizeof(unsigned long)));

Ugh, addr is also not a pointer which I'd not noticed but still.  My
main thought there was to move the cap_offset change to a second step so
it was done type safely.

> I was suggesting something along the lines of the diff below.

Yes, I know.

Attachment: signature.asc
Description: PGP signature