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

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.

-Kees

> 
> Signed-off-by: David Laight <[email protected]>
> ---
>  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
> 

-- 
Kees Cook

Reply via email to