On Mon, Mar 30, 2026 at 02:20:03PM +0100, [email protected] wrote:
> From: David Laight <[email protected]>
> 
> 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 <[email protected]>
> ---
>  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

Reply via email to