linux-kernel - Re: arch/arm64/kernel/signal.c:1046:36: sparse: sparse: cast removes address space '_

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: <4fb5acc6-350e-4f7b-8512-4e1799cb2a37@sirena.org.uk>
Date: Tue, 10 Dec 2024 12:33:38 +0000
From: Mark Brown <broonie@...nel.org>
To: Catalin Marinas <catalin.marinas@....com>
Cc: kernel test robot <lkp@...el.com>, oe-kbuild-all@...ts.linux.dev,
	linux-kernel@...r.kernel.org,
	Thiago Jung Bauermann <thiago.bauermann@...aro.org>
Subject: Re: arch/arm64/kernel/signal.c:1046:36: sparse: sparse: cast removes
 address space '__user' of expression

On Tue, Dec 10, 2024 at 11:11:47AM +0000, Catalin Marinas wrote:
> On Mon, Dec 09, 2024 at 04:11:00PM +0000, Mark Brown wrote:

> > The value we get from the
> > sysreg is a pointer so it makes the uses of the value clearer if we keep
> > it as a pointer in C code, it seems to be defeating the point of doing
> > static analysis to discard the pointerness to make it happier.

> We have other cases where we treat a reg as u64 and convert it to
> pointer as needed. While not a sysreg, the pt_regs::sp is u64 and we end
> up treating it as a pointer eventually for writing the signal stack.
> Another case is user_insn_read(). It's bit of bikeshedding around the
> primary use in this function, do we need more conversions one way or the
> other? In general I'd consider a sysreg read to be u64, especially as
> the architecture has a habit of adding bits around the actual address
> occasionally.

The sysreg read is a u64, but we then interpret it as a pointer and do
pointer things with it for the remainder of the function until we need
to store it back in the register (which is where the warning comes from,
for some reason there's a cast in write_sysreg_s()).

> Anyway, I'd like to silence sparse on this. The u64 (non-pointer) has
> some precedence in the arm64 code but, if you want, I'm happy to keep it
> as a pointer (and maybe just rename it to shadow_stack or something that
> does not imply a sysreg). I think for the actual warning, we can
> probably fix it with a __force to silence sparse on conversion to u64.

Apparently the rune for this with sparse is to cast the value to an
unsigned long (type safety for the win!).  I wrote a patch last night,
should be done testing by now.  Given sparse we should probably have
that cast in write_sysreg_s() be to unsigned long.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)