Re: [PATCH next 3/3] fortify: Simplify strlen() logic

[Kees Cook <kees@kernel.org>]

On Mon, Mar 30, 2026 at 02:20:03PM +0100, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
> 
> The __builtin_choose_expr() doesn't gain you anything, replace with
> a simple ?: operator.
> Then __is_constexpr() can then be replaced with __builtin_constant_p().
> This still works for static initialisers - the expression can contain
> a function call - provided it isn't actually called.
> 
> Calling the strnlen() wrapper just add a lot more logic to read through.
> Replace with a call to __real_strnlen().
> 
> However the compiler can decide that __builtin_constant_p(__builtin_strlen(p))
> is false, but split as ret = __builtin_strlen(p); __builtin_constant_p(ret)
> and it suddenly becomes true.
> So an additional check is needed before calling __real_strnlen().

Ah, there it is, exactly the issue I'm remembering, see
commit 4f3d1be4c2f8 ("compiler.h: add const_true()")

Instead of this patch, I should likely replace the open-coded versions
of const_true() here.

Regardless, we should not change this or the compiletime_strlen() macro
as you've suggested, IMO. They both work as they already are, so I see
no reason re-open this can of worms without a good reason.

What was the specific code that caused an issue for you with
__fortify_strlen ?

-Kees

> 
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
>  include/linux/fortify-string.h | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> index 758afd7c5f8a..6cd670492270 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -230,9 +230,8 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size
>  }
>  
>  /*
> - * Defined after fortified strnlen to reuse it. However, it must still be
> - * possible for strlen() to be used on compile-time strings for use in
> - * static initializers (i.e. as a constant expression).
> + * strlen() of a compile-time string needs to be a constant expression
> + * so it can be used, for example, as a static initializer.
>   */
>  /**
>   * strlen - Return count of characters in a NUL-terminated string
> @@ -247,9 +246,9 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size
>   * Returns number of characters in @p (NOT including the final NUL).
>   *
>   */
> -#define strlen(p)							\
> -	__builtin_choose_expr(__is_constexpr(__builtin_strlen(p)),	\
> -		__builtin_strlen(p), __fortify_strlen(p))
> +#define strlen(p)					\
> +	(__builtin_constant_p(__builtin_strlen(p)) ?	\
> +		__builtin_strlen(p) : __fortify_strlen(p))
>  __FORTIFY_INLINE __diagnose_as(__builtin_strlen, 1)
>  __kernel_size_t __fortify_strlen(const char * const POS p)
>  {
> @@ -259,7 +258,14 @@ __kernel_size_t __fortify_strlen(const char * const POS p)
>  	/* Give up if we don't know how large p is. */
>  	if (p_size == SIZE_MAX)
>  		return __underlying_strlen(p);
> -	ret = strnlen(p, p_size);
> +	/*
> +	 * 'ret' can be constant here even though the __builtin_constant_p(__builtin_strlen(p))
> +	 * in the #define wrapper is false.
> +	 */
> +	ret = __builtin_strlen(p);
> +	if (__builtin_constant_p(ret))
> +		return ret;
> +	ret = __real_strnlen(p, p_size);
>  	if (p_size <= ret)
>  		fortify_panic(FORTIFY_FUNC_strlen, FORTIFY_READ, p_size, ret + 1, ret);
>  	return ret;
> -- 
> 2.39.5
> 

-- 
Kees Cook