linux-kernel - Re: [PATCH] arm64/sve: Document that __SVE_VQ

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: <ZcN8OltRDUlDlTHQ@finisterre.sirena.org.uk>
Date: Wed, 7 Feb 2024 12:48:58 +0000
From: Mark Brown <broonie@...nel.org>
To: Dave Martin <Dave.Martin@....com>
Cc: Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will@...nel.org>, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64/sve: Document that __SVE_VQ_MAX is much larger
 than needed

On Wed, Feb 07, 2024 at 12:01:43PM +0000, Dave Martin wrote:
> On Tue, Feb 06, 2024 at 04:27:01PM +0000, Mark Brown wrote:

> > +/*
> > + * Note that for future proofing __SVE_VQ_MAX is defined much larger
> > + * than the actual architecture maximum of 16.
> > + */

> I think that putting shadow #defines in comments in UAPI headers is a
> really bad idea...  is this a normative statement about the user API,
> or what?

Well, the only reason I'm mentioning the constant here is that
__SVE_VQ_MIN is defined too and has a perfectly good value, things look
a bit neater with a shared comment block.  I'm not sure there's a hugely
meaingful difference between having a comment adjacent to a named
constant in a header and one a couple of lines away that mentions the
constant by name.

> My concern is that if we muddy the waters here different bits of
> software will do different things and we will get a mess with no
> advantages.

> Portability issues may ensue if userspace software feels it can
> substitute some other value for this constant, since we can't control
> what userspace uses it for.

I don't think we want people using this at all, ideally we'd remove it
but it's in the uapi.

> Would it be sufficient to say something like:

> /*
>  * Yes, this is 512 QUADWORDS.
>  * Never allocate memory or size structures based on the value of this
>  * constant.
>  */
> >  #define __SVE_VQ_MAX		512

I think the fact that this vector length is more than an order of
magnitude more than is architecturally supported at present needs to be
conveyed, it's perfectly reasonable for people to not want to do dynamic
allocation/sizing of buffers in some applications and the above sounds
more like stylistic guidance about using dynamic sizing to improve
memory usage.

> Though comments might be better placed alongsize SVE_VQ_MAX at al., in
> ptrace.h and sigcontext.h rather than here.  The leading __ should at
> least be a hint that __SVE_VQ_MAX shouldn't be used directly by
> anyone...

Yeah, the multiple layers of indirection aren't helpful here.  We
probably need to comment it in both places TBH.

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