Re: [PATCH next 2/3] fortify: Optimise strnlen()

[David Laight <david.laight.linux@gmail.com>]

On Mon, 30 Mar 2026 23:36:07 -0700
Kees Cook <kees@kernel.org> wrote:

> On Mon, Mar 30, 2026 at 02:20:02PM +0100, david.laight.linux@gmail.com wrote:
> > From: David Laight <david.laight.linux@gmail.com>
> > 
> > If the string is constant there is no need to call __real_strlen()
> > even when maxlen is a variable - just return the smaller value.
> > 
> > If the size of the string variable is unknown fortify_panic() can't be
> > called, change the condition so that the compiler can optimise it away.
> > 
> > Change __compiletime_strlen(p) to return a 'non-constant' value
> > for non-constant strings (the same as __builtin_strlen()).
> > Simplify since it is only necessary to check that the size is constant
> > and that the last character is '\0'.
> > Explain why it is different from __builtin_strlen().
> > Update the kunit tests to match.  
> 
> See also
> commit d07c0acb4f41 ("fortify: Fix __compiletime_strlen() under UBSAN_BOUNDS_LOCAL")
> 
> -Kees
> 

It is far more subtle that that;
There shouldn't be a run-time access to __p[__p_len] at all.
And you really don't want one.

The problematic code was:
	if (__builtin_constant(__p[__p_len]) && __p[__p_len] == 0)
If the compiler thinks __p[__p_len] is constant then it will also
think that __p[0] is constant.
So the extra check should really make no difference.

I suspect this is what happened, consider:
	const char *foo;
	if (cond)
		foo = "foo";
	else
		foo = "fubar";
	return __compiletime_strlen(foo);

This is first converted to (ignoring any silly typos):
	const char *foo;
	if (cond)
		foo = "foo";
	else
		foo = "fubar";
	len = __builtin_object_size(foo,1) - 1; // 6 - 1
	if (__builtin_constant(foo[len]) && foo[len] == 0)
		return __builtin_strlen(foo);
	return SIZE_MAX;
Since foo isn't constant that returns SIZE_MAX.

The code is then moved into the conditional giving:
	if (cond) {
		foo = "foo";
		if (__builtin_constant(foo[5]) && foo[5] == 0)
			return __builtin_strlen(foo);
		return SIZE_MAX;
	} else {
		foo = "fubar";
		if (__builtin_constant(foo[5]) && foo[5] == 0)
			return __builtin_strlen(foo);
		return SIZE_MAX;
	}	

Since since foo is now 'pointer to constant' foo[] is constant, giving: 
	if (cond) {
		foo = "foo";
		if (foo[5] == 0)
			return __builtin_strlen(foo);
		return SIZE_MAX;
	} else {
		foo = "fubar";
		if (foo[5] == 0)
			return __builtin_strlen(foo);
		return SIZE_MAX;
	}	

In the bottom bit foo[5] is well defined and known to be zero.
In the top bit foo[5] is UB and gcc leaves the code it, giving:
	if (cond) {
		foo = "foo";
		if (foo[5] == 0)
			return __builtin_strlen(foo);
		return SIZE_MAX;
	} else {
		foo = "fubar";
		return __builtin_strlen(foo);
	}

and you get a real reference off the end of foo[] - which UBSAN_LOCAL_BOUNDS
rightly picks up on.

clang has a habit of silently deleting everything after UB, so might
generate:
	if (cond) {
		return whatever_happens_to_be_in_ax;
	} else {
		foo = "fubar";
		return __builtin_strlen(foo);
	}

The 'fix' of checking __p[0] actually makes no real difference.
I'd guess that the longer code block stops gcc moving the code
into the conditional and hides the bug.
But that could easily change by just breathing on the code somewhere
or in a future compiler version.

I suspect this should be a compiler bug.
But with the compiler behaving this way you can't write
__compiletime_strlen() with a check for the '\0' terminator.

That really means you can only use __builtin_strlen().
Which means you'll get a compile-time error from:
	char foo[3] = "foo";
	__builtin_strlen(foo);
rather the 'not a constant' when checking strscpy(tgt, foo, 3);
At a guess that never happens except in the tests.

	David