Received-SPF: pass (google.com: domain of linux-kernel+bounces-215932-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <c8234b8e2c9c1122eec73c97901e36586160f0cb.camel@tugraz.at>
Subject: Re: [PATCH v4 0/3] Hardening perf subsystem
From: Martin Uecker <uecker@tugraz.at>
To: Peter Zijlstra <peterz@infradead.org>, Kees Cook <kees@kernel.org>
Cc: Erick Archer <erick.archer@outlook.com>, Ingo Molnar <mingo@redhat.com>,
  Arnaldo Carvalho de Melo <acme@kernel.org>, Namhyung Kim
 <namhyung@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Alexander
 Shishkin <alexander.shishkin@linux.intel.com>,  Jiri Olsa
 <jolsa@kernel.org>, Ian Rogers <irogers@google.com>, Adrian Hunter
 <adrian.hunter@intel.com>,  "Liang, Kan" <kan.liang@linux.intel.com>,
 Thomas Gleixner <tglx@linutronix.de>, Borislav Petkov <bp@alien8.de>,  Dave
 Hansen <dave.hansen@linux.intel.com>, "H. Peter Anvin" <hpa@zytor.com>,
 "Gustavo A. R. Silva" <gustavoars@kernel.org>, Nathan Chancellor
 <nathan@kernel.org>, Nick Desaulniers <ndesaulniers@google.com>, Bill
 Wendling <morbo@google.com>, Justin Stitt <justinstitt@google.com>,
 Christophe JAILLET <christophe.jaillet@wanadoo.fr>,  Matthew Wilcox
 <mawilcox@microsoft.com>, x86@kernel.org, linux-perf-users@vger.kernel.org,
  linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org, 
 llvm@lists.linux.dev
Date: Sat, 15 Jun 2024 18:09:07 +0200
In-Reply-To: <20240614101708.GO8774@noisy.programming.kicks-ass.net>
References: 
	<AS8PR02MB7237F5BFDAA793E15692B3998BFD2@AS8PR02MB7237.eurprd02.prod.outlook.com>
	 <202406101010.E1C77AE9D@keescook>
	 <20240610200544.GY8774@noisy.programming.kicks-ass.net>
	 <202406101438.BC43514F@keescook>
	 <20240611075542.GD8774@noisy.programming.kicks-ass.net>
	 <202406121148.688240B@keescook>
	 <20240612220821.GA18881@noisy.programming.kicks-ass.net>
	 <202406121530.D9DB956C8@keescook>
	 <20240614101708.GO8774@noisy.programming.kicks-ass.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4-2 
Precedence: bulk
MIME-Version: 1.0

Am Freitag, dem 14.06.2024 um 12:17 +0200 schrieb Peter Zijlstra:
> 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 bet=
ter
> > > > named under the size_*() helpers which are trying to keep size_t
> > > > calculations from overflowing (by saturating). i.e.:
> > > >=20
> > > > 	size_add_mult(sizeof(*p), sizeof(*p->member), num)
> > >=20
> > > Fine I suppose, but what if we want something not size_t? Are we wait=
ing
> > > for the type system extension?
> >=20
> > Because of C's implicit promotion/truncation, we can't do anything
> > sanely with return values of arbitrary type size; we have to capture th=
e
> > lvalue type somehow so the checking can happen without C doing silent
> > garbage.

What is the specific problem here?

>=20
> So sizeof() returns the native (built-in) size_t, right? If that type
> the nooverflow qualifier on, then:
>=20
> 	sizeof(*p) + num*sizeof(p->foo[0])
>=20
> should all get the nooverflow semantics right? Because size_t is
> effectively 'nooverflow unsigned long' the multiplication should promote
> 'num' to some 'long'.
>=20
> 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?
>=20
> I mean, when we're adding qualifiers we can make up rules about them
> too, right?

It should probably be a type attribute.

>=20
> If 'people' don't want to adorn the built-in size_t, we can always do
> something like:
>=20
> #define sizeof(x) ((nooverflow unsigned long)(sizeof(x)))
>=20
> and 'fix' it ourselves.

This is likely a stupid question, but making it signed=C2=A0
wouldn't work?   Or is a signed size_t too small=C2=A0=C2=A0
or some architectures? Or would this change break too much?


Martin

>=20
> > > But none of that is showing me generated asm for the various cases. A=
s
> > > such, I don't consider myself informed enough.
> >=20
> > 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:
> >=20
> > 	const char src[] =3D "Hello there";
> > 	char dst[10];
> >=20
> > 	strscpy(dst, src); /* Compiler yells since src is bigger than dst. */
> >=20
> > For run-time checks it's basically just using the regular WARN()
> > infrastructure with __builtin_dynamic_object_size(). Here's a simplifie=
d
> > userspace example with assert():
> >=20
> > https://godbolt.org/z/zMrKnMxn5
> >=20
> > 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.
>=20
> Thanks, I'll go have a look at that.