linux-kernel - Re: [PATCH] arm64/signal: Silence spurious sparse warning storing GCSPR

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: <Z1xeLCKoSWyn2sko@arm.com>
Date: Fri, 13 Dec 2024 16:17:48 +0000
From: Catalin Marinas <catalin.marinas@....com>
To: Mark Brown <broonie@...nel.org>
Cc: Mark Rutland <mark.rutland@....com>, Will Deacon <will@...nel.org>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	kernel test robot <lkp@...el.com>
Subject: Re: [PATCH] arm64/signal: Silence spurious sparse warning storing
 GCSPR_EL0

On Tue, Dec 10, 2024 at 04:52:49PM +0000, Mark Brown wrote:
> 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 still have a slight preference for treating a sysreg value as an
integer and cast it to pointer when needed rather than using __force.

> > > 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.

'addr' should stay as long in map_shadow_stack(). This matches mmap(),
mremap(), brk() etc. as they handle address space layout manipulation
(these functions also do not accept tagged pointers). map_shadow_stack()
does write a cap to memory but its primary functionality is not user
buffer access but rather memory layout management.

-- 
Catalin