linux-kernel - Re: [PATCH v4 0/3] Hardening perf subsystem

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: <20240614101708.GO8774@noisy.programming.kicks-ass.net>
Date: Fri, 14 Jun 2024 12:17:08 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Kees Cook <kees@...nel.org>
Cc: Erick Archer <erick.archer@...look.com>, Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Namhyung Kim <namhyung@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	"Liang, Kan" <kan.liang@...ux.intel.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"Gustavo A. R. Silva" <gustavoars@...nel.org>,
	Nathan Chancellor <nathan@...nel.org>,
	Nick Desaulniers <ndesaulniers@...gle.com>,
	Bill Wendling <morbo@...gle.com>,
	Justin Stitt <justinstitt@...gle.com>,
	Christophe JAILLET <christophe.jaillet@...adoo.fr>,
	Matthew Wilcox <mawilcox@...rosoft.com>, x86@...nel.org,
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-hardening@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH v4 0/3] Hardening perf subsystem

On Wed, Jun 12, 2024 at 04:23:31PM -0700, Kees Cook wrote:
> On Thu, Jun 13, 2024 at 12:08:21AM +0200, Peter Zijlstra wrote:
> > On Wed, Jun 12, 2024 at 12:01:19PM -0700, Kees Cook wrote:
> > > I'm happy to take patches. And for this bikeshed, this would be better
> > > named under the size_*() helpers which are trying to keep size_t
> > > calculations from overflowing (by saturating). i.e.:
> > > 
> > > 	size_add_mult(sizeof(*p), sizeof(*p->member), num)
> > 
> > Fine I suppose, but what if we want something not size_t? Are we waiting
> > for the type system extension?
> 
> Because of C's implicit promotion/truncation, we can't do anything
> sanely with return values of arbitrary type size; we have to capture the
> lvalue type somehow so the checking can happen without C doing silent
> garbage.

So sizeof() returns the native (built-in) size_t, right? If that type
the nooverflow qualifier on, then:

	sizeof(*p) + num*sizeof(p->foo[0])

should all get the nooverflow semantics right? Because size_t is
effectively 'nooverflow unsigned long' the multiplication should promote
'num' to some 'long'.

Now, I've re-read the rules and I don't see qualifiers mentioned, so
can't we state that the overflow/nooverflow qualifiers are to be
preserved on (implicit) promotion and when nooverflow and overflow are
combined the 'safe' nooverflow takes precedence?

I mean, when we're adding qualifiers we can make up rules about them
too, right?

If 'people' don't want to adorn the built-in size_t, we can always do
something like:

#define sizeof(x) ((nooverflow unsigned long)(sizeof(x)))

and 'fix' it ourselves.

> > But none of that is showing me generated asm for the various cases. As
> > such, I don't consider myself informed enough.
> 
> Gotcha. For the compile-time stuff it's all just looking at
> known-at-compile-time sizes. So for something like this, we get a
> __compiletime_warning() emitted:
> 
> 	const char src[] = "Hello there";
> 	char dst[10];
> 
> 	strscpy(dst, src); /* Compiler yells since src is bigger than dst. */
> 
> For run-time checks it's basically just using the regular WARN()
> infrastructure with __builtin_dynamic_object_size(). Here's a simplified
> userspace example with assert():
> 
> https://godbolt.org/z/zMrKnMxn5
> 
> The kernel's FORTIFY_SOURCE is much more complex in how it does the
> checking, how it does the reporting (for helping people figure out what's
> gone weird), etc.

Thanks, I'll go have a look at that.