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

[Kees Cook]

On Tue, Jun 18, 2024 at 10:22:42AM +0200, Peter Zijlstra wrote:
> On Mon, Jun 17, 2024 at 10:28:20AM -0700, Kees Cook wrote:
> 
> > But, using type attributes we have much more flexibility. Hence, the
> > proposed "wraps" attribute:
> > https://github.com/llvm/llvm-project/pull/86618
> 
> So I still think that's going about things backwards.
> 
> unsigned explicitly wraps. signed is UB.
> 
> When using -fwrapv signed is well defined as 2s complement, which takes
> away the UB and makes it implicitly wrap.
> 
> When extending the language, it is important to not break existing code,
> so the default must remain wrap. This in turn means you need to add a
> 'nowrap' thingy.

Well, we still disagree about this, but I guess we'll see how the size_t
work comes along. Maybe I will come to agree with you. :)

> Also, I would very much not make this an attribute, but a full type
> qualifier. Furthermore, add type promotion rules to ensure nowrap takes
> precedence and is preserved throughout expressions. Such that:
> 
>   'long' + 'nowrap int' -> 'nowrap long'
> 
> Then, once you have this, you can go do things like:
> 
> typedef nowrap unsigned long size_t;
> #define sizeof(x) ((size_t)sizeof(x))
> 
> and things will just work. Hmm?

Right. Currently this is being plumbed through the sanitizers, so the
type selection will happen there (for this version of it). The current
experiment is to basically tell the sanitizers to ignore all types
except a given list, and our initial list will be just size_t.

Before this, Justin is finishing up the initial set of idiom exclusions.
.e.g.:

	size_t var, offset;
	...
	if (var + offset < var) { ... }

This will not freak out if "var + offset" wraps.


And "negative unsigned constants" will be ignored:

	#define MASK	-2ULL
	...
	size_t mask = MASK;


And loop post decrements wrapping will get ignored too:

	size_t count = ...;
	...
	while (count--) { ... }


After we get it all nailed down, we'll see if anything new pops up. :)

-- 
Kees Cook