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

[David Laight]

On Mon, 30 Mar 2026 16:54:27 -0700
Kees Cook <kees@xxxxxxxxxx> wrote:

> On Mon, Mar 30, 2026 at 02:20:02PM +0100, david.laight.linux@xxxxxxxxx wrote:
> > From: David Laight <david.laight.linux@xxxxxxxxx>
> > 
> > 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.  
> 
> This will need very very careful checking. It took a lot of tweaking to
> get __compiletime_strlen() to behave correctly across all supported
> versions of GCC and Clang. It has some very ugly glitches possible. I'm
> not saying your replacement is broken: I mean to say I'm going to need
> to spend a bunch of time proving to myself that this is a safe
> replacement.

There are two differences:
1) I removed the check for p[0] being constant, just p[size - 1].
2) The return value is 'not constant' (rather than SIZE_MAX) for
   non-constant strings.
   This includes any case where __builtin_strlen() returns a non-constant.

I did wonder what happens if you have:
static char foo[] = "foo";
then later:
	foo[1] = 0;
The compiler could determine that both foo[0] and foo[3] are never
changed, so (both versions of) __compiletime_strlen() would return
the non-constant result of __builtin_strlen().
The current compilers don't report foo[3] as being constant unless
the string itself in constant, but they might get 'better'. 

> Outside of that, though, I don't think dropping the various SIZE_MAX
> tests is correct. Again, I'll need to spend some time double-checking.

They needed changing to match the function returning 'not constant'
rather than SIZE_MAX.

Actually, as I noted in the other email (after writing, but not sending,
this one) __compiletime_strlen() is fundamentally broken with the current
compilers.
Any uses should be replaced by __builtin_strlen().

	David

> 
> -Kees
> 
> > 
> > Signed-off-by: David Laight <david.laight.linux@xxxxxxxxx>
> > ---
> >  include/linux/fortify-string.h | 44 +++++++++++++++++-----------------
> >  lib/tests/fortify_kunit.c      |  8 +++----
> >  2 files changed, 26 insertions(+), 26 deletions(-)
> > 
> > diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> > index 214d237214d5..758afd7c5f8a 100644
> > --- a/include/linux/fortify-string.h
> > +++ b/include/linux/fortify-string.h
> > @@ -58,19 +58,22 @@ void __read_overflow2_field(size_t avail, size_t wanted) __compiletime_warning("
> >  void __write_overflow(void) __compiletime_error("detected write beyond size of object (1st parameter)");
> >  void __write_overflow_field(size_t avail, size_t wanted) __compiletime_warning("detected write beyond size of field (1st parameter); maybe use struct_group()?");
> >  
> > -#define __compiletime_strlen(p)					\
> > -({								\
> > -	char *__p = (char *)(p);				\
> > -	size_t __ret = SIZE_MAX;				\
> > -	const size_t __p_size = __member_size(p);		\
> > -	if (__p_size != SIZE_MAX &&				\
> > -	    __builtin_constant_p(*__p)) {			\
> > -		size_t __p_len = __p_size - 1;			\
> > -		if (__builtin_constant_p(__p[__p_len]) &&	\
> > -		    __p[__p_len] == '\0')			\
> > -			__ret = __builtin_strlen(__p);		\
> > -	}							\
> > -	__ret;							\
> > +/*
> > + * __builtin_strlen() generates a compile-time error for 'const char foo[4] = "abcd";'.
> > + * But that is a valid source for both strnlen() and strscpy() with a constant
> > + * length less than or equal to 4.
> > + * __compiletime_strlen() returns a non-constant for such items.
> > + * Beware of strings with embedded '\0', __builtin_strlen() can be much smaller
> > + * than __member_size();
> > + * The return value must only be used when it is a constant.
> > + */
> > +extern size_t __fortify_undefined;
> > +#define __compiletime_strlen(p)							\
> > +({										\
> > +	char *__p = (char *)(p);						\
> > +	const size_t __p_size = __member_size(p);				\
> > +	__p_size == SIZE_MAX || !statically_true(__p[__p_size - 1] == '\0') ?	\
> > +		__fortify_undefined : __builtin_strlen(__p);			\
> >  })
> >  
> >  #if defined(__SANITIZE_ADDRESS__)
> > @@ -215,16 +218,13 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size
> >  	const size_t p_len = __compiletime_strlen(p);
> >  	size_t ret;
> >  
> > -	/* We can take compile-time actions when maxlen is const. */
> > -	if (__builtin_constant_p(maxlen) && p_len != SIZE_MAX) {
> > -		/* If p is const, we can use its compile-time-known len. */
> > -		if (maxlen >= p_size)
> > -			return p_len;
> > -	}
> > +	/* If p is const, we can use its compile-time-known len. */
> > +	if (__builtin_constant_p(p_len))
> > +		return p_len < maxlen ? p_len : maxlen;
> >  
> >  	/* Do not check characters beyond the end of p. */
> > -	ret = __real_strnlen(p, maxlen < p_size ? maxlen : p_size);
> > -	if (p_size <= ret && maxlen != ret)
> > +	ret = __real_strnlen(p, p_size < maxlen ? p_size : maxlen);
> > +	if (ret == p_size && p_size < maxlen)
> >  		fortify_panic(FORTIFY_FUNC_strnlen, FORTIFY_READ, p_size, ret + 1, ret);
> >  	return ret;
> >  }
> > @@ -289,7 +289,7 @@ __FORTIFY_INLINE ssize_t sized_strscpy(char * const POS p, const char * const PO
> >  	if (statically_true(p_size < SIZE_MAX)) {
> >  		len = __compiletime_strlen(q);
> >  
> > -		if (len < SIZE_MAX && statically_true(len < size)) {
> > +		if (statically_true(len < size)) {
> >  			__underlying_memcpy(p, q, len + 1);
> >  			return len;
> >  		}
> > diff --git a/lib/tests/fortify_kunit.c b/lib/tests/fortify_kunit.c
> > index fc9c76f026d6..9b3b7201c02d 100644
> > --- a/lib/tests/fortify_kunit.c
> > +++ b/lib/tests/fortify_kunit.c
> > @@ -102,11 +102,11 @@ static void fortify_test_known_sizes(struct kunit *test)
> >  	KUNIT_EXPECT_EQ(test, __compiletime_strlen(unchanging_12), 12);
> >  
> >  	KUNIT_EXPECT_FALSE(test, __is_constexpr(__builtin_strlen(array_unknown)));
> > -	KUNIT_EXPECT_EQ(test, __compiletime_strlen(array_unknown), SIZE_MAX);
> > +	KUNIT_EXPECT_FALSE(test, __is_constexpr(__compiletime_strlen(array_unknown)));
> >  
> >  	/* Externally defined and dynamically sized string pointer: */
> >  	KUNIT_EXPECT_FALSE(test, __is_constexpr(__builtin_strlen(test->name)));
> > -	KUNIT_EXPECT_EQ(test, __compiletime_strlen(test->name), SIZE_MAX);
> > +	KUNIT_EXPECT_FALSE(test, __is_constexpr(__compiletime_strlen(test->name)));
> >  }
> >  
> >  /* This is volatile so the optimizer can't perform DCE below. */
> > @@ -128,12 +128,12 @@ static noinline size_t want_minus_one(int pick)
> >  		str = "1";
> >  		break;
> >  	}
> > -	return __compiletime_strlen(str);
> > +	return __builtin_constant_p(__compiletime_strlen(str));
> >  }
> >  
> >  static void fortify_test_control_flow_split(struct kunit *test)
> >  {
> > -	KUNIT_EXPECT_EQ(test, want_minus_one(pick), SIZE_MAX);
> > +	KUNIT_EXPECT_FALSE(test, want_minus_one(pick));
> >  }
> >  
> >  #define KUNIT_EXPECT_BOS(test, p, expected, name)			\
> > -- 
> > 2.39.5
> >   
>