[PATCH v5 0/7] Add strprefix() and fix a bug with it

[PATCH v5 0/7] Add strprefix() and fix a bug with it

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 15288 bytes --]

Hi!

str_has_prefix() is based on sizes and lengths, which means it must
overload the valid length 0 to mean not found.  This is a typical
consequence of choosing to design an API around sizes and lengths, and
vanishes when moving to pointer-based similar APIs.

According to its author (Steven R.), str_has_prefix() was never meant to
accept an empty prefix, but that's not documented, and there's no
obvious reason why that would be acceptable for a generic string
function (as opposed to a function very specific to its original use).
Anyway, as suggested by Steven, I've added a WARN_ON_ONCE() to catch
such uses, and also added documentation stating this limitation.

Users of strings need a function whose behavior extends naturally to the
empty string, and we can do better, by returning a pointer.  And indeed,
users have ended up using str_has_prefix() with empty prefix, expecting
it to work (as is natural of a generic string API).

This patch set adds strprefix(), deprecates str_has_prefix(), and uses
strprefix() in one place to fix a bug reported by Bartosz.

We can later work on replacing the other uses of str_has_prefix(), which
might hide similar bugs.

This patch set also adds infrastructure to make string functions return
a pointer that is as qualified as the input pointer.

Changes in v5
~~~~~~~~~~~~~

-  [1/7]
   -  Fix .rst formatting.  [sashiko]
-  [2/7]
   -  Move the function to "lib/string.h" to be able to use
      WARN_ON_ONCE().  [Andy, Bart]
-  [3/7]  -- no changes --
-  [4/7]
   -  Simplify const_cast(), which makes it more robust.  'T:' didn't
      work, because in case const T is the same type as T, there's
      ambiguity in which branch should be taken, and the compiler
      rejects that.  We could go back to 'default:' (as in v3), but that
      is only needed for casting 'const char **' to 'char **'; since we
      don't need that at the moment, let's keep it simple, and we'll be
      able to add it in the future if we ever want that.
-  [5/7]  -- no changes --
-  [6/7]  -- no changes --
-  [7/7]
   -  New patch; use strprefix() to implement strstarts().

I didn't apply some changes requrested by reviewers:

-  I didn't move strprefix() to "lib/string.h" implementing it as a loop
   with manual byte operations.  The only reason for that seems to be
   performance, but I expect by being inline, the compiler should be
   able to completely drop the strlen(3) call in many cases, making the
   inline function faster.

See range-diff below.

v1: <https://lore.kernel.org/linux-hardening/cover.1793876837.git.alx@kernel.org/>
v2: <https://lore.kernel.org/linux-hardening/cover.1793882260.git.alx@kernel.org/>
v3: <https://lore.kernel.org/linux-hardening/cover.1794323750.git.alx@kernel.org/>
v4: <https://lore.kernel.org/linux-hardening/cover.1794425425.git.alx@kernel.org/>

Testing
~~~~~~~

(I've indented the output, to better see the commands.)

$ git log --oneline --graph v7.4-rc1^..strprefix5
	* 382a44f7c686 (HEAD -> strprefix, tag: strprefix5) string: Implement strstarts() with strprefix()
	* 0ca0b7aabcfc string: Make strprefix() const-qualifier-generic
	* 1294b3d64440 string: Add [C]QChar_of() and QVoid_of()
	* b615b5afb05e stddef: Add const_cast()
	* 49252e78c9b6 device property: Fix fwnode_name_eq() for empty node names
	* df3a35c469c6 string: Warn if the prefix is the empty string
	* f25bf43d7f18 string: Add strprefix(), and deprecate str_has_prefix()
	* 6628e7d4c6b2 (tag: v7.4-rc1, master) Linux 7.4-rc1
$ cat .kunitconfig
	CONFIG_KUNIT=y
	CONFIG_STRING_KUNIT_TEST=y
$ mkdir -p ~/tmp/strprefix5
$ export TMPDIR=$HOME/tmp/strprefix5/
$ ./tools/testing/kunit/kunit.py run "string*"
	[13:27:47] Configuring KUnit Kernel ...
	[13:27:47] Building KUnit Kernel ...
	Populating config with:
	$ make ARCH=um O=.kunit olddefconfig
	Building with:
	$ make all compile_commands.json scripts_gdb ARCH=um O=.kunit --jobs=24
	[13:28:02] Starting KUnit Kernel (1/1)...
	[13:28:02] ============================================================
	Running tests with:
	$ .kunit/linux 'kunit.filter_glob=string*' kunit.enable=1 mem=1G console=tty kunit_shutdown=halt
	[13:28:02] ============= string-stream-test (12 subtests) =============
	[13:28:02] [PASSED] string_stream_managed_init_test
	[13:28:02] [PASSED] string_stream_unmanaged_init_test
	[13:28:02] [PASSED] string_stream_managed_free_test
	[13:28:02] [PASSED] string_stream_resource_free_test
	[13:28:02] [PASSED] string_stream_line_add_test
	[13:28:02] [PASSED] string_stream_variable_length_line_test
	[13:28:02] [PASSED] string_stream_append_test
	[13:28:02] [PASSED] string_stream_append_auto_newline_test
	[13:28:02] [PASSED] string_stream_append_empty_string_test
	[13:28:02] [PASSED] string_stream_no_auto_newline_test
	[13:28:02] [PASSED] string_stream_auto_newline_test
	[13:28:02] [PASSED] string_stream_performance_test
	[13:28:02] =============== [PASSED] string-stream-test ================
	[13:28:02] =================== string (29 subtests) ===================
	[13:28:02] [PASSED] string_test_memset16
	[13:28:02] [PASSED] string_test_memset32
	[13:28:02] [PASSED] string_test_memset64
	[13:28:02] [PASSED] string_test_strlen
	[13:28:02] [PASSED] string_test_strnlen
	[13:28:02] [PASSED] string_test_strchr
	[13:28:02] [PASSED] string_test_strnchr
	[13:28:02] [PASSED] string_test_strrchr
	[13:28:02] [PASSED] string_test_strspn
	[13:28:02] [PASSED] string_test_strprefix
	[13:28:02] [PASSED] string_test_strcmp
	[13:28:02] [PASSED] string_test_strcmp_long_strings
	[13:28:02] [PASSED] string_test_strncmp
	[13:28:02] [PASSED] string_test_strncmp_long_strings
	[13:28:02] [PASSED] string_test_strcasecmp
	[13:28:02] [PASSED] string_test_strcasecmp_long_strings
	[13:28:02] [PASSED] string_test_strncasecmp
	[13:28:02] [PASSED] string_test_strncasecmp_long_strings
	[13:28:02] [PASSED] string_test_strscpy
	[13:28:02] [PASSED] string_test_strcat
	[13:28:02] [PASSED] string_test_strncat
	[13:28:02] [PASSED] string_test_strlcat
	[13:28:02] [PASSED] string_test_strtomem
	[13:28:02] [PASSED] string_test_memtostr
	[13:28:02] [PASSED] string_test_strends
	[13:28:02] [SKIPPED] string_bench_strlen (not enabled)
	[13:28:02] [SKIPPED] string_bench_strnlen (not enabled)
	[13:28:02] [SKIPPED] string_bench_strchr (not enabled)
	[13:28:02] [SKIPPED] string_bench_strrchr (not enabled)
	[13:28:02] ===================== [PASSED] string ======================
	[13:28:02] =============== string_helpers (4 subtests) ================
	[13:28:02] [PASSED] test_get_size
	[13:28:02] [PASSED] test_upper_lower
	[13:28:02] [PASSED] test_unescape
	[13:28:02] [PASSED] test_escape
	[13:28:02] ================= [PASSED] string_helpers ==================
	[13:28:02] ============================================================
	[13:28:02] Testing complete. Ran 45 tests: passed: 41, skipped: 4
	[13:28:02] Elapsed time: 15.583s total, 0.000s configuring, 15.366s building, 0.172s running

$ make allmodconfig
	  HOSTCC  scripts/basic/fixdep
	  HOSTCC  scripts/kconfig/conf.o
	  HOSTCC  scripts/kconfig/confdata.o
	  HOSTCC  scripts/kconfig/expr.o
	  LEX     scripts/kconfig/lexer.lex.c
	  YACC    scripts/kconfig/parser.tab.[ch]
	  HOSTCC  scripts/kconfig/lexer.lex.o
	  HOSTCC  scripts/kconfig/menu.o
	  HOSTCC  scripts/kconfig/parser.tab.o
	  HOSTCC  scripts/kconfig/preprocess.o
	  HOSTCC  scripts/kconfig/symbol.o
	  HOSTCC  scripts/kconfig/util.o
	  HOSTLD  scripts/kconfig/conf
	#
	# configuration written to .config
	#
$ time make -j24 | ts -s
	00:00:01   GEN     arch/x86/include/generated/asm/orc_hash.h
	00:00:01   UPD     arch/x86/include/generated/uapi/asm/bpf_perf_event.h
	00:00:01   UPD     arch/x86/include/generated/uapi/asm/errno.h
...
	00:21:49   LD [M]  net/qrtr/qrtr-tun.ko
	00:21:49   LD [M]  net/qrtr/qrtr-mhi.ko
	00:21:49   LD [M]  virt/lib/irqbypass.ko

	real	21m48.882s
	user	439m26.908s
	sys	25m20.132s


Have a lovely day!
Alex


Alejandro Colomar (7):
  string: Add strprefix(), and deprecate str_has_prefix()
  string: Warn if the prefix is the empty string
  device property: Fix fwnode_name_eq() for empty node names
  stddef: Add const_cast()
  string: Add [C]QChar_of() and QVoid_of()
  string: Make strprefix() const-qualifier-generic
  string: Implement strstarts() with strprefix()

 drivers/base/property.c  |  5 +---
 include/linux/stddef.h   | 11 +++++++++
 include/linux/string.h   | 52 ++++++++++++++++++++++++++++++++--------
 lib/string.c             | 24 +++++++++++++++++++
 lib/tests/string_kunit.c |  9 +++++++
 5 files changed, 87 insertions(+), 14 deletions(-)

Range-diff against v4:
1:  a74fabd985a8 ! 1:  f25bf43d7f18 string: Add strprefix(), and deprecate str_has_prefix()
    @@ include/linux/string.h: void memcpy_and_pad(void *dest, size_t dest_len, const v
     + * and not a constant.  Instead use strprefix().
     + *
     + * Returns:
    -+ * *  @s+strlen(@prefix) if @s starts with @prefix
    ++ * *  @s + strlen(@prefix) if @s starts with @prefix
     + * *  NULL if @s does not start with @prefix
     + */
     +static inline char *strprefix(const char *s, const char *prefix)
2:  cd09427148a0 ! 2:  df3a35c469c6 string: Warn if the prefix is the empty string
    @@ Commit message
         Warn if it happens.  Also, document that if such corner case
         is needed, strprefix() is preferable.
     
    +    We need to move str_has_prefix() to "lib/string.h" to be able to use
    +    WARN_ON_ONCE().
    +
         Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
         Link: https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com/
         Suggested-by: Steven Rostedt <rostedt@goodmis.org>
    +    Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## include/linux/string.h ##
    +@@ include/linux/string.h: extern char * strstr(const char *, const char *);
    + #ifndef __HAVE_ARCH_STRNSTR
    + extern char * strnstr(const char *, const char *, size_t);
    + #endif
    ++extern size_t str_has_prefix(const char *str, const char *prefix);
    + #ifndef __HAVE_ARCH_STRLEN
    + extern __kernel_size_t strlen(const char *);
    + #endif
     @@ include/linux/string.h: static inline char *strprefix(const char *s, const char *prefix)
    -  * This is similar to strprefix(), but returns an offset instead of
    -  * a pointer.
    -  *
    + 	return strncmp(s, prefix, len) == 0 ? (char *) s + len : NULL;
    + }
    + 
    +-/**
    +- * str_has_prefix - Test if a string has a given prefix
    +- * @str: The string to test
    +- * @prefix: The string to see if @str starts with
    +- *
    +- * This is similar to strprefix(), but returns an offset instead of
    +- * a pointer.
    +- *
    +- * Returns:
    +- * * strlen(@prefix) if @str starts with @prefix
    +- * * 0 if @str does not start with @prefix
    +- */
    +-static __always_inline size_t str_has_prefix(const char *str, const char *prefix)
    +-{
    +-	size_t len = strlen(prefix);
    +-	return strncmp(str, prefix, len) == 0 ? len : 0;
    +-}
    +-
    + /**
    +  * strstarts - does @str start with @prefix?
    +  * @str: string to examine
    +
    + ## lib/string.c ##
    +@@ lib/string.c: char *strnstr(const char *s1, const char *s2, size_t len)
    + EXPORT_SYMBOL(strnstr);
    + #endif
    + 
    ++/**
    ++ * str_has_prefix - Test if a string has a given prefix
    ++ * @str: The string to test
    ++ * @prefix: The string to see if @str starts with
    ++ *
    ++ * This is similar to strprefix(), but returns an offset instead of
    ++ * a pointer.
    ++ *
     + * Unlike for strprefix(), if the prefix is an empty string, the
     + * behavior is undefined.  If you need to support arbitrary prefixes,
     + * use strprefix().
     + *
    -  * Returns:
    -  * * strlen(@prefix) if @str starts with @prefix
    -  * * 0 if @str does not start with @prefix
    -@@ include/linux/string.h: static inline char *strprefix(const char *s, const char *prefix)
    - static __always_inline size_t str_has_prefix(const char *str, const char *prefix)
    - {
    - 	size_t len = strlen(prefix);
    ++ * Returns:
    ++ * * strlen(@prefix) if @str starts with @prefix
    ++ * * 0 if @str does not start with @prefix
    ++ */
    ++size_t str_has_prefix(const char *str, const char *prefix)
    ++{
    ++	size_t len = strlen(prefix);
     +	WARN_ON_ONCE(len == 0);
    - 	return strncmp(str, prefix, len) == 0 ? len : 0;
    - }
    - 
    ++	return strncmp(str, prefix, len) == 0 ? len : 0;
    ++}
    ++EXPORT_SYMBOL(str_has_prefix);
    ++
    + #ifndef __HAVE_ARCH_MEMCHR
    + /**
    +  * memchr - Find a character in an area of memory.
3:  0058b60f662c = 3:  49252e78c9b6 device property: Fix fwnode_name_eq() for empty node names
4:  ce623fe0432b ! 4:  b615b5afb05e stddef: Add const_cast()
    @@ Commit message
         It's similar to C++'s const_cast(), but it is more restrictive: it can
         only cast away 'const', and not arbitrary qualifiers.
     
    -    David made it a bit more robust.
    -
    -    Link: https://lore.kernel.org/linux-hardening/20261111102535.434c2eee@pumpkin/
    -    Co-developed-by: David Laight <david.laight.linux@gmail.com>
    +    Cc: David Laight <david.laight.linux@gmail.com>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## include/linux/stddef.h ##
    @@ include/linux/stddef.h: enum {
     + * @T: Type for the cast.
     + * @p: Pointer to be casted.
     + *
    -+ * If @p is of type const @T, then it is casted to @T.
    -+ * If @p is of type @T, then it is returned without any casts.
    ++ * Cast @p to type @T.  The macro verifies that @p is of type const @T.
     + * This avoids mistakes that change anything other than 'const' in a cast.
     + */
    -+#define const_cast(T, p)  _Generic(p, const T: (T) (p), T: (p))
    ++#define const_cast(T, p)  _Generic(p, const T: (T) (p))
     +
      #endif
5:  206d2045abcf = 5:  1294b3d64440 string: Add [C]QChar_of() and QVoid_of()
6:  69d1792e5edf ! 6:  0ca0b7aabcfc string: Make strprefix() const-qualifier-generic
    @@ Commit message
     
      ## include/linux/string.h ##
     @@ include/linux/string.h: void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
    -  * *  @s+strlen(@prefix) if @s starts with @prefix
    +  * *  @s + strlen(@prefix) if @s starts with @prefix
       * *  NULL if @s does not start with @prefix
       */
     -static inline char *strprefix(const char *s, const char *prefix)
    @@ include/linux/string.h: void memcpy_and_pad(void *dest, size_t dest_len, const v
     +#define strprefix(s, pfx)  const_cast(CQChar_of(s) *, strprefix(s, pfx))
      
      /**
    -  * str_has_prefix - Test if a string has a given prefix
    +  * strstarts - does @str start with @prefix?
-:  ------------ > 7:  382a44f7c686 string: Implement strstarts() with strprefix()
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v5 1/7] string: Add strprefix(), and deprecate str_has_prefix()

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 3860 bytes --]

As Bartosz reported, str_has_prefix() has a bug: it can't differentiate
between an empty prefix matching an empty string from a non-matching
prefix.  This is a fundamental design flaw in this API, and not
something that can be fixed.

As always, this is a consequence of choosing sizes instead of pointers.
Sizes are often the wrong choice.  In this specific case, pointers are
more capable of differentiating these cases, as NULL is a special
sentinel value that doesn't need to overload a valid value.

Let's implement strprefix(), a function that returns a pointer
right after the prefix if found, and a null pointer if not found.

We'll slowly migrate existing code from str_has_prefix() to strprefix().

Move the description from str_has_prefix(), and describe
str_has_prefix() by comparing it to strprefix().

Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Link: https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com/
Cc: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 include/linux/string.h   | 28 +++++++++++++++++++++++-----
 lib/tests/string_kunit.c |  9 +++++++++
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index 6cb5cdd01158..04e8a8c0f05a 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -545,16 +545,34 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
 	       sizeof(*(obj)) - offsetof(typeof(*(obj)), member));	\
 })
 
+/**
+ * strprefix - string prefix
+ * @s: The string to be tested
+ * @prefix: The string to see if @s starts with
+ *
+ * A common way to test a prefix of a string is to do:
+ *  strncmp(s, prefix, sizeof(prefix) - 1)
+ *
+ * But this can lead to bugs due to typos, or if prefix is a pointer
+ * and not a constant.  Instead use strprefix().
+ *
+ * Returns:
+ * *  @s + strlen(@prefix) if @s starts with @prefix
+ * *  NULL if @s does not start with @prefix
+ */
+static inline char *strprefix(const char *s, const char *prefix)
+{
+	size_t len = strlen(prefix);
+	return strncmp(s, prefix, len) == 0 ? (char *) s + len : NULL;
+}
+
 /**
  * str_has_prefix - Test if a string has a given prefix
  * @str: The string to test
  * @prefix: The string to see if @str starts with
  *
- * A common way to test a prefix of a string is to do:
- *  strncmp(str, prefix, sizeof(prefix) - 1)
- *
- * But this can lead to bugs due to typos, or if prefix is a pointer
- * and not a constant. Instead use str_has_prefix().
+ * This is similar to strprefix(), but returns an offset instead of
+ * a pointer.
  *
  * Returns:
  * * strlen(@prefix) if @str starts with @prefix
diff --git a/lib/tests/string_kunit.c b/lib/tests/string_kunit.c
index 0819ace5b027..73d2614b1474 100644
--- a/lib/tests/string_kunit.c
+++ b/lib/tests/string_kunit.c
@@ -287,6 +287,14 @@ static void string_test_strspn(struct kunit *test)
 	}
 }
 
+static void string_test_strprefix(struct kunit *test)
+{
+	KUNIT_ASSERT_EQ(test, 0, strcmp(strprefix("foobar", "foo"), "bar"));
+	KUNIT_ASSERT_EQ(test, 0, strcmp(strprefix("foobar", ""), "foobar"));
+	KUNIT_ASSERT_EQ(test, 0, strcmp(strprefix("foobar", "foobar"), ""));
+	KUNIT_ASSERT_NULL(test, strprefix("foobar", "bar"));
+}
+
 static char strcmp_buffer1[STRCMP_LARGE_BUF_LEN];
 static char strcmp_buffer2[STRCMP_LARGE_BUF_LEN];
 
@@ -891,6 +899,7 @@ static struct kunit_case string_test_cases[] = {
 	KUNIT_CASE(string_test_strnchr),
 	KUNIT_CASE(string_test_strrchr),
 	KUNIT_CASE(string_test_strspn),
+	KUNIT_CASE(string_test_strprefix),
 	KUNIT_CASE(string_test_strcmp),
 	KUNIT_CASE(string_test_strcmp_long_strings),
 	KUNIT_CASE(string_test_strncmp),
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v5 2/7] string: Warn if the prefix is the empty string

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 3098 bytes --]

This corner case is mishandled by this function.
Warn if it happens.  Also, document that if such corner case
is needed, strprefix() is preferable.

We need to move str_has_prefix() to "lib/string.h" to be able to use
WARN_ON_ONCE().

Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Link: https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com/
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 include/linux/string.h | 19 +------------------
 lib/string.c           | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index 04e8a8c0f05a..dcda1b7801bb 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -194,6 +194,7 @@ extern char * strstr(const char *, const char *);
 #ifndef __HAVE_ARCH_STRNSTR
 extern char * strnstr(const char *, const char *, size_t);
 #endif
+extern size_t str_has_prefix(const char *str, const char *prefix);
 #ifndef __HAVE_ARCH_STRLEN
 extern __kernel_size_t strlen(const char *);
 #endif
@@ -566,24 +567,6 @@ static inline char *strprefix(const char *s, const char *prefix)
 	return strncmp(s, prefix, len) == 0 ? (char *) s + len : NULL;
 }
 
-/**
- * str_has_prefix - Test if a string has a given prefix
- * @str: The string to test
- * @prefix: The string to see if @str starts with
- *
- * This is similar to strprefix(), but returns an offset instead of
- * a pointer.
- *
- * Returns:
- * * strlen(@prefix) if @str starts with @prefix
- * * 0 if @str does not start with @prefix
- */
-static __always_inline size_t str_has_prefix(const char *str, const char *prefix)
-{
-	size_t len = strlen(prefix);
-	return strncmp(str, prefix, len) == 0 ? len : 0;
-}
-
 /**
  * strstarts - does @str start with @prefix?
  * @str: string to examine
diff --git a/lib/string.c b/lib/string.c
index a3778d5aab4a..61a1e79c1b87 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -774,6 +774,30 @@ char *strnstr(const char *s1, const char *s2, size_t len)
 EXPORT_SYMBOL(strnstr);
 #endif
 
+/**
+ * str_has_prefix - Test if a string has a given prefix
+ * @str: The string to test
+ * @prefix: The string to see if @str starts with
+ *
+ * This is similar to strprefix(), but returns an offset instead of
+ * a pointer.
+ *
+ * Unlike for strprefix(), if the prefix is an empty string, the
+ * behavior is undefined.  If you need to support arbitrary prefixes,
+ * use strprefix().
+ *
+ * Returns:
+ * * strlen(@prefix) if @str starts with @prefix
+ * * 0 if @str does not start with @prefix
+ */
+size_t str_has_prefix(const char *str, const char *prefix)
+{
+	size_t len = strlen(prefix);
+	WARN_ON_ONCE(len == 0);
+	return strncmp(str, prefix, len) == 0 ? len : 0;
+}
+EXPORT_SYMBOL(str_has_prefix);
+
 #ifndef __HAVE_ARCH_MEMCHR
 /**
  * memchr - Find a character in an area of memory.
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v5 3/7] device property: Fix fwnode_name_eq() for empty node names

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1549 bytes --]

str_has_prefix() returns the prefix length on match and 0 on mismatch,
so the mistmatch case can't be told from "empty prefix matched an empty
string".

For instance: the OF root node's name as returned by
fwnode_get_name() is "" and will return 0 for any name matched against
it and since len is 0 too, fwnode_name_eq() will return true.

Let's use strprefix() instead, which also makes this function simpler.

Fixes: a9c8c738066b7 ("device property: Add fwnode_name_eq()")
Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Link: https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com/
Cc: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 drivers/base/property.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 51109794d727..7728116e2968 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -692,15 +692,12 @@ const char *fwnode_get_name_prefix(const struct fwnode_handle *fwnode)
 bool fwnode_name_eq(const struct fwnode_handle *fwnode, const char *name)
 {
 	const char *node_name;
-	ptrdiff_t len;
 
 	node_name = fwnode_get_name(fwnode);
 	if (!node_name)
 		return false;
 
-	len = strchrnul(node_name, '@') - node_name;
-
-	return str_has_prefix(node_name, name) == len;
+	return strprefix(node_name, name) == strchrnul(node_name, '@');
 }
 EXPORT_SYMBOL_GPL(fwnode_name_eq);
 
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v5 4/7] stddef: Add const_cast()

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]

This macro serves to cast const away without risking unwanted
conversions due to mistakes in the cast.

It's similar to C++'s const_cast(), but it is more restrictive: it can
only cast away 'const', and not arbitrary qualifiers.

Cc: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 include/linux/stddef.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index e1851c50c89b..26c657465e60 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -194,4 +194,15 @@ enum {
 #define TRAILING_OVERLAP(TYPE, NAME, FAM, MEMBERS)				\
 	__TRAILING_OVERLAP(TYPE, NAME, FAM, /* no attrs */, MEMBERS)
 
+/**
+ * const_cast() - At most, drop const in the left-most derivation of a pointer
+ *
+ * @T: Type for the cast.
+ * @p: Pointer to be casted.
+ *
+ * Cast @p to type @T.  The macro verifies that @p is of type const @T.
+ * This avoids mistakes that change anything other than 'const' in a cast.
+ */
+#define const_cast(T, p)  _Generic(p, const T: (T) (p))
+
 #endif
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v5 5/7] string: Add [C]QChar_of() and QVoid_of()

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1782 bytes --]

These macros serve to implement qualifier-generic macros as those
specified in ISO C23.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 include/linux/string.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index dcda1b7801bb..5f3ac760df2e 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -14,6 +14,36 @@
 #include <linux/stdarg.h>
 #include <uapi/linux/string.h>
 
+/**
+ * QVoid_of - qualified void of
+ *
+ * @p: Pointer to test for qualifiers
+ *
+ * This macro produces the type name 'void',
+ * qualified with any qualifiers present in the element of @p.
+ */
+#define QVoid_of(p)       typeof(*(__QVoidptr_of(p)){0})
+#define __QVoidptr_of(p)  typeof(1?(p):(void*){0})
+
+/**
+ * QChar_of - qualified char of
+ *
+ * @Q: Qualifier to test.
+ * @p: Pointer to test for the @Q qualifier
+ *
+ * This macro produces the type name 'char',
+ * @Q-qualified iff the input pointer points to a @Q-qualified element.
+ */
+#define QChar_of(Q, p)  typeof                                        \
+(                                                                     \
+	_Generic((QVoid_of(p) *){0},                                  \
+		Q void *:  (Q char){0},                               \
+		void *:    (  char){0}                                \
+	)                                                             \
+)
+// CQChar_of - const qualified char of
+#define CQChar_of(p)  QChar_of(const, p)
+
 extern char *strndup_user(const char __user *, long);
 extern void *memdup_user(const void __user *, size_t) __realloc_size(2);
 extern void *vmemdup_user(const void __user *, size_t) __realloc_size(2);
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v5 6/7] string: Make strprefix() const-qualifier-generic

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1522 bytes --]

Add a macro of the same name, which returns a pointer that is
const-qualified iff the input string is const-qualified.  The return
type is a possibly-const-qualified pointer to char, even if the input
is a possibly-const-qualified pointer to void, which is why we require
the CQChar_of() macro.

The function now needs to return 'const char *', because const_cast()
can only drop const, but never add it.

Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Joe Perches <joe@perches.com>
Cc: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 include/linux/string.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index 5f3ac760df2e..b1468d3913f6 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -591,11 +591,12 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
  * *  @s + strlen(@prefix) if @s starts with @prefix
  * *  NULL if @s does not start with @prefix
  */
-static inline char *strprefix(const char *s, const char *prefix)
+static inline const char *strprefix(const char *s, const char *prefix)
 {
 	size_t len = strlen(prefix);
-	return strncmp(s, prefix, len) == 0 ? (char *) s + len : NULL;
+	return strncmp(s, prefix, len) == 0 ? s + len : NULL;
 }
+#define strprefix(s, pfx)  const_cast(CQChar_of(s) *, strprefix(s, pfx))
 
 /**
  * strstarts - does @str start with @prefix?
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v5 7/7] string: Implement strstarts() with strprefix()

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]

The compiler should be able to optimize out the unused pointer
arithmetic done at the end, so performance-wise this shouldn't change
anything.

This change de-duplicates the almost-identical implementations,
and better shows the relationship between the two functions.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 include/linux/string.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index b1468d3913f6..015d2c7ee6c2 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -608,7 +608,7 @@ static inline const char *strprefix(const char *s, const char *prefix)
  */
 static inline bool strstarts(const char *str, const char *prefix)
 {
-	return strncmp(str, prefix, strlen(prefix)) == 0;
+	return strprefix(str, prefix);
 }
 
 /**
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v5 7/7] string: Implement strstarts() with strprefix()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] string: strstarts() via strprefix() prevents optimizing out
  pointer offset
--

commit 8cdc2e9c96584305ea94313f2618d17d8e28f303
Author: Alejandro Colomar <alx@kernel.org>

string: Implement strstarts() with strprefix()

This commit changes strstarts() to call strprefix() instead of calling
strncmp() and strlen() directly.

> diff --git a/include/linux/string.h b/include/linux/string.h
> index 8c6a79d320474..8ca0deda61606 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -595,7 +595,7 @@ static inline const char *strprefix(const char *s, const char *prefix)
>   */
>  static inline bool strstarts(const char *str, const char *prefix)
>  {
> -	return strncmp(str, prefix, strlen(prefix)) == 0;
> +	return strprefix(str, prefix);
>  }

[Severity: Low]
Can the compiler actually optimize away the s + len pointer arithmetic in
strprefix() here?

strprefix() returns strncmp(s, prefix, len) == 0 ? s + len : NULL, and
converting that result to bool requires proving that s + len is non-NULL
whenever strncmp() returns 0.  Because the kernel builds with
-fno-delete-null-pointer-checks and -fno-strict-overflow, the compiler
cannot assume that a runtime pointer is non-NULL or that s + len does not
wrap to NULL, so callers of strstarts() end up keeping str and len live
across strncmp() and emitting the extra addition and NULL check.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794574496.git.alx@kernel.org?part=7

Re: [PATCH v5 2/7] string: Warn if the prefix is the empty string

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] string: moving str_has_prefix() to lib/string.c bypasses
  FORTIFY_SOURCE
- [Low] string: commit message cites non-existent lib/string.h and omits
  function name
--

commit 018109c1aa9a2d5e3def240ae4b9134b27f492d5
Author: Alejandro Colomar <alx@kernel.org>

> string: Warn if the prefix is the empty string
>
> This corner case is mishandled by this function.
> Warn if it happens.  Also, document that if such corner case
> is needed, strprefix() is preferable.
>
> We need to move str_has_prefix() to "lib/string.h" to be able to use
> WARN_ON_ONCE().
>
> Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> Link: https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com/
> Suggested-by: Steven Rostedt <rostedt@goodmis.org>
> Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
> Signed-off-by: Alejandro Colomar <alx@kernel.org>

[Severity: Low]
This isn't a bug, but the commit message says str_has_prefix() is moved to
"lib/string.h", which does not exist; the patch moves it to lib/string.c.
Also, neither the subject line nor the opening sentence names
str_has_prefix().

[ ... ]

> diff --git a/lib/string.c b/lib/string.c
> index a3778d5aab4af..61a1e79c1b875 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -774,6 +774,30 @@ char *strnstr(const char *s1, const char *s2, size_t len)
>  EXPORT_SYMBOL(strnstr);
>  #endif
>  
> +/**
> + * str_has_prefix - Test if a string has a given prefix
> + * @str: The string to test
> + * @prefix: The string to see if @str starts with
> + *
> + * This is similar to strprefix(), but returns an offset instead of
> + * a pointer.
> + *
> + * Unlike for strprefix(), if the prefix is an empty string, the
> + * behavior is undefined.  If you need to support arbitrary prefixes,
> + * use strprefix().
> + *
> + * Returns:
> + * * strlen(@prefix) if @str starts with @prefix
> + * * 0 if @str does not start with @prefix
> + */
> +size_t str_has_prefix(const char *str, const char *prefix)
> +{
> +	size_t len = strlen(prefix);
> +	WARN_ON_ONCE(len == 0);
> +	return strncmp(str, prefix, len) == 0 ? len : 0;
> +}
> +EXPORT_SYMBOL(str_has_prefix);

[Severity: Low]
Does moving str_has_prefix() out-of-line into lib/string.c bypass
CONFIG_FORTIFY_SOURCE and prevent compile-time constant folding of
strlen(prefix)?

lib/string.c is built with __NO_FORTIFY and its top-of-file comment notes
that it is reserved for routines that have architecture-specific
alternatives in <asm/string.h> or get overloaded by FORTIFY_SOURCE, while
other helpers belong in lib/string_helpers.c.

When str_has_prefix() was a static __always_inline helper in
include/linux/string.h (introduced in 72921427d46b ("string.h: Add
str_has_prefix() helper function")), strlen(prefix) folded string-literal
prefixes into constants at compile time and ran fortified bounds checks on
non-constant arrays.  Moving it to lib/string.c forces an out-of-line call
and an unfortified runtime strlen() scan on every invocation, and shares a
single WARN_ON_ONCE() state across all callers in the kernel.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794574496.git.alx@kernel.org?part=2

Re: [PATCH v5 7/7] string: Implement strstarts() with strprefix()

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 4131 bytes --]

> Date: 2026-11-13 13:20:16+0000
> From: sashiko-bot@kernel.org
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Low] string: strstarts() via strprefix() prevents optimizing out
>   pointer offset
> --
> 
> commit 8cdc2e9c96584305ea94313f2618d17d8e28f303
> Author: Alejandro Colomar <alx@kernel.org>
> 
> string: Implement strstarts() with strprefix()
> 
> This commit changes strstarts() to call strprefix() instead of calling
> strncmp() and strlen() directly.
> 
> > diff --git a/include/linux/string.h b/include/linux/string.h
> > index 8c6a79d320474..8ca0deda61606 100644
> > --- a/include/linux/string.h
> > +++ b/include/linux/string.h
> > @@ -595,7 +595,7 @@ static inline const char *strprefix(const char *s, const char *prefix)
> >   */
> >  static inline bool strstarts(const char *str, const char *prefix)
> >  {
> > -	return strncmp(str, prefix, strlen(prefix)) == 0;
> > +	return strprefix(str, prefix);
> >  }
> 
> [Severity: Low]
> Can the compiler actually optimize away the s + len pointer arithmetic in
> strprefix() here?
> 
> strprefix() returns strncmp(s, prefix, len) == 0 ? s + len : NULL, and
> converting that result to bool requires proving that s + len is non-NULL
> whenever strncmp() returns 0.  Because the kernel builds with
> -fno-delete-null-pointer-checks and -fno-strict-overflow, the compiler
> cannot assume that a runtime pointer is non-NULL or that s + len does not
> wrap to NULL, so callers of strstarts() end up keeping str and len live
> across strncmp() and emitting the extra addition and NULL check.

Hmmm, it seems it's right; the compiler can't optimize out.  I hadn't
taken those flags into account.

Here's a test which seems to corroborate:

	alx@devuan:~/tmp$ cat strstarts.c 
	#include <string.h>
	bool strstarts(const char *s, const char *prefix)
	{
		return strncmp(s, prefix, strlen(prefix)) == 0;
	}
	alx@devuan:~/tmp$ cat strprefix.c 
	#include <string.h>
	bool strstarts(const char *s, const char *prefix)
	{
		size_t len = strlen(prefix);
		return strncmp(s, prefix, len) == 0 ? s + len : NULL;
	}
	alx@devuan:~/tmp$ gcc -Wall -Wextra -O2 -fno-delete-null-pointer-checks -fno-strict-overflow -S strstarts.c 
	alx@devuan:~/tmp$ gcc -Wall -Wextra -O2 -fno-delete-null-pointer-checks -fno-strict-overflow -S strprefix.c 
	strprefix.c: In function ‘strstarts’:
	strprefix.c:5:9: warning: the comparison will always evaluate as ‘true’ for the pointer operand in ‘s + (sizetype)len’ must not be NULL [-Waddress]
	    5 |         return strncmp(s, prefix, len) == 0 ? s + len : NULL;
	      |         ^~~~~~
	alx@devuan:~/tmp$ diff -u strstarts.s strprefix.s
	--- strstarts.s	2026-11-13 14:33:48.478848453 +0100
	+++ strprefix.s	2026-11-13 14:33:51.802880696 +0100
	@@ -1,4 +1,4 @@
	-	.file	"strstarts.c"
	+	.file	"strprefix.c"
		.text
		.p2align 4
		.globl	strstarts
	@@ -6,24 +6,45 @@
	 strstarts:
	 .LFB0:
		.cfi_startproc
	-	pushq	%rbx
	+	pushq	%rbp
		.cfi_def_cfa_offset 16
	-	.cfi_offset 3, -16
	+	.cfi_offset 6, -16
	+	pushq	%rbx
	+	.cfi_def_cfa_offset 24
	+	.cfi_offset 3, -24
		movq	%rdi, %rbx
		movq	%rsi, %rdi
	-	subq	$16, %rsp
	-	.cfi_def_cfa_offset 32
	+	subq	$24, %rsp
	+	.cfi_def_cfa_offset 48
		movq	%rsi, 8(%rsp)
		call	strlen@PLT
		movq	8(%rsp), %rsi
		movq	%rbx, %rdi
		movq	%rax, %rdx
	+	movq	%rax, %rbp
		call	strncmp@PLT
		testl	%eax, %eax
	-	sete	%al
	-	addq	$16, %rsp
	+	jne	.L4
	+	addq	%rbp, %rbx
	+	setne	%al
	+	addq	$24, %rsp
	+	.cfi_remember_state
	+	.cfi_def_cfa_offset 24
	+	popq	%rbx
		.cfi_def_cfa_offset 16
	+	popq	%rbp
	+	.cfi_def_cfa_offset 8
	+	ret
	+	.p2align 4,,10
	+	.p2align 3
	+.L4:
	+	.cfi_restore_state
	+	addq	$24, %rsp
	+	.cfi_def_cfa_offset 24
	+	xorl	%eax, %eax
		popq	%rbx
	+	.cfi_def_cfa_offset 16
	+	popq	%rbp
		.cfi_def_cfa_offset 8
		ret
		.cfi_endproc

I'll drop patch 7/7 in v6.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794574496.git.alx@kernel.org?part=7

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v5 2/7] string: Warn if the prefix is the empty string

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 4086 bytes --]

> Date: 2026-11-13 13:26:21+0000
> From: sashiko-bot@kernel.org
>
> Thank you for your contribution! Sashiko AI review found 2 potential
> issues to consider:
> - [Low] string: moving str_has_prefix() to lib/string.c bypasses
>   FORTIFY_SOURCE
> - [Low] string: commit message cites non-existent lib/string.h and omits
>   function name
> --
> 
> commit 018109c1aa9a2d5e3def240ae4b9134b27f492d5
> Author: Alejandro Colomar <alx@kernel.org>
> 
> > string: Warn if the prefix is the empty string
> >
> > This corner case is mishandled by this function.
> > Warn if it happens.  Also, document that if such corner case
> > is needed, strprefix() is preferable.
> >
> > We need to move str_has_prefix() to "lib/string.h" to be able to use
> > WARN_ON_ONCE().
> >
> > Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> > Link: https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com/
> > Suggested-by: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
> > Signed-off-by: Alejandro Colomar <alx@kernel.org>
> 
> [Severity: Low]
> This isn't a bug, but the commit message says str_has_prefix() is moved to
> "lib/string.h", which does not exist; the patch moves it to lib/string.c.
> Also, neither the subject line nor the opening sentence names
> str_has_prefix().

Oops, typo and thinko.  Will fix for v6.

> 
> [ ... ]
> 
> > diff --git a/lib/string.c b/lib/string.c
> > index a3778d5aab4af..61a1e79c1b875 100644
> > --- a/lib/string.c
> > +++ b/lib/string.c
> > @@ -774,6 +774,30 @@ char *strnstr(const char *s1, const char *s2, size_t len)
> >  EXPORT_SYMBOL(strnstr);
> >  #endif
> >  
> > +/**
> > + * str_has_prefix - Test if a string has a given prefix
> > + * @str: The string to test
> > + * @prefix: The string to see if @str starts with
> > + *
> > + * This is similar to strprefix(), but returns an offset instead of
> > + * a pointer.
> > + *
> > + * Unlike for strprefix(), if the prefix is an empty string, the
> > + * behavior is undefined.  If you need to support arbitrary prefixes,
> > + * use strprefix().
> > + *
> > + * Returns:
> > + * * strlen(@prefix) if @str starts with @prefix
> > + * * 0 if @str does not start with @prefix
> > + */
> > +size_t str_has_prefix(const char *str, const char *prefix)
> > +{
> > +	size_t len = strlen(prefix);
> > +	WARN_ON_ONCE(len == 0);
> > +	return strncmp(str, prefix, len) == 0 ? len : 0;
> > +}
> > +EXPORT_SYMBOL(str_has_prefix);
> 
> [Severity: Low]
> Does moving str_has_prefix() out-of-line into lib/string.c bypass
> CONFIG_FORTIFY_SOURCE and prevent compile-time constant folding of
> strlen(prefix)?

Preventing the compile-time constant folding of strlen(3) is indeed
a performance degradation, but that's necessary to be able to
WARN_ON_ONCE(), so there isn't anything we can do.

> 
> lib/string.c is built with __NO_FORTIFY and its top-of-file comment notes
> that it is reserved for routines that have architecture-specific
> alternatives in <asm/string.h> or get overloaded by FORTIFY_SOURCE, while
> other helpers belong in lib/string_helpers.c.

Do we want to move it to "lib/string_helpers.c"?  I don't know enough
about the organization in these files, so just let me know what I should
do.

> When str_has_prefix() was a static __always_inline helper in
> include/linux/string.h (introduced in 72921427d46b ("string.h: Add
> str_has_prefix() helper function")), strlen(prefix) folded string-literal
> prefixes into constants at compile time and ran fortified bounds checks on
> non-constant arrays.  Moving it to lib/string.c forces an out-of-line call
> and an unfortified runtime strlen() scan on every invocation, and shares a
> single WARN_ON_ONCE() state across all callers in the kernel.

I guess that's bonus points for strprefix().  :)

> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794574496.git.alx@kernel.org?part=2

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v5 2/7] string: Warn if the prefix is the empty string

[Andy Shevchenko]

On Fri, Nov 13, 2026 at 02:48:01PM +0100, Alejandro Colomar wrote:

...

> > [Severity: Low]
> > Does moving str_has_prefix() out-of-line into lib/string.c bypass
> > CONFIG_FORTIFY_SOURCE and prevent compile-time constant folding of
> > strlen(prefix)?
> 
> Preventing the compile-time constant folding of strlen(3) is indeed
> a performance degradation, but that's necessary to be able to
> WARN_ON_ONCE(), so there isn't anything we can do.
> 
> > lib/string.c is built with __NO_FORTIFY and its top-of-file comment notes
> > that it is reserved for routines that have architecture-specific
> > alternatives in <asm/string.h> or get overloaded by FORTIFY_SOURCE, while
> > other helpers belong in lib/string_helpers.c.
> 
> Do we want to move it to "lib/string_helpers.c"?  I don't know enough
> about the organization in these files, so just let me know what I should
> do.

string library (generic) is for broad use, so it should really be clean from
kernel things. string_helpers is exactly for some custom abstractions we use
in kernel. Now you should answer these questions (or more than that):
- Are there or potentially be users that needs this API earlier at the boot?
- Is this API is detached from the rest of string library semantically?
  I.o.w. can we group semantically coupled APIs and consider them as a bundle?
- Due to the above, ask the first Q against semantically grouped APIs.

My personal feeling that what you are trying to do is no go for generic string
as is. If you confirm that these APIs are not used / needed at early stages at
boot, they can be moved completely to string_helpers.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 2/7] string: Warn if the prefix is the empty string

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 2188 bytes --]

Hi Andy,

> Date: 2026-11-13 16:25:35+0200
> From: Andy Shevchenko <andriy.shevchenko@intel.com>
>
> On Fri, Nov 13, 2026 at 02:48:01PM +0100, Alejandro Colomar wrote:
> 
> ...
> 
> > > [Severity: Low]
> > > Does moving str_has_prefix() out-of-line into lib/string.c bypass
> > > CONFIG_FORTIFY_SOURCE and prevent compile-time constant folding of
> > > strlen(prefix)?
> > 
> > Preventing the compile-time constant folding of strlen(3) is indeed
> > a performance degradation, but that's necessary to be able to
> > WARN_ON_ONCE(), so there isn't anything we can do.
> > 
> > > lib/string.c is built with __NO_FORTIFY and its top-of-file comment notes
> > > that it is reserved for routines that have architecture-specific
> > > alternatives in <asm/string.h> or get overloaded by FORTIFY_SOURCE, while
> > > other helpers belong in lib/string_helpers.c.
> > 
> > Do we want to move it to "lib/string_helpers.c"?  I don't know enough
> > about the organization in these files, so just let me know what I should
> > do.
> 
> string library (generic) is for broad use, so it should really be clean from
> kernel things. string_helpers is exactly for some custom abstractions we use
> in kernel. Now you should answer these questions (or more than that):
> - Are there or potentially be users that needs this API earlier at the boot?
> - Is this API is detached from the rest of string library semantically?
>   I.o.w. can we group semantically coupled APIs and consider them as a bundle?
> - Due to the above, ask the first Q against semantically grouped APIs.
> 
> My personal feeling that what you are trying to do is no go for generic string
> as is. If you confirm that these APIs are not used / needed at early stages at
> boot, they can be moved completely to string_helpers.

Thanks!  I guess I'll just drop the addition of WARN_ON_ONCE().  Let's
leave the function as is, and document its problems.  It's not worth the
effort.  After all, I think it's a bad function, and should not be used
in new code.


Have a lovely day!
Alex

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v5 1/7] string: Add strprefix(), and deprecate str_has_prefix()

[Andy Shevchenko]

On Fri, Nov 13, 2026 at 02:10:29PM +0100, Alejandro Colomar wrote:
> As Bartosz reported, str_has_prefix() has a bug: it can't differentiate
> between an empty prefix matching an empty string from a non-matching
> prefix.  This is a fundamental design flaw in this API, and not
> something that can be fixed.
> 
> As always, this is a consequence of choosing sizes instead of pointers.
> Sizes are often the wrong choice.  In this specific case, pointers are
> more capable of differentiating these cases, as NULL is a special
> sentinel value that doesn't need to overload a valid value.
> 
> Let's implement strprefix(), a function that returns a pointer
> right after the prefix if found, and a null pointer if not found.
> 
> We'll slowly migrate existing code from str_has_prefix() to strprefix().
> 
> Move the description from str_has_prefix(), and describe
> str_has_prefix() by comparing it to strprefix().
> 
> Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> Link: https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com/

It's unreferred Link tag, spurious? Please, refer to it in the commit message
to understand why it's here. Use Link: $URL [1] and use [1] in the text.

> Cc: Andy Shevchenko <andy@kernel.org>

No need to put my name explicitly in Cc list, I'm a designated reviewer for
string library, you should get it via `b4` or `get_maintainer.pl`. And in
general is better to just move the Cc after the '---' line.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 1/7] string: Add strprefix(), and deprecate str_has_prefix()

[Andy Shevchenko]

On Fri, Nov 13, 2026 at 04:33:37PM +0200, Andy Shevchenko wrote:
> On Fri, Nov 13, 2026 at 02:10:29PM +0100, Alejandro Colomar wrote:
> > As Bartosz reported, str_has_prefix() has a bug: it can't differentiate
> > between an empty prefix matching an empty string from a non-matching
> > prefix.  This is a fundamental design flaw in this API, and not
> > something that can be fixed.
> > 
> > As always, this is a consequence of choosing sizes instead of pointers.
> > Sizes are often the wrong choice.  In this specific case, pointers are
> > more capable of differentiating these cases, as NULL is a special
> > sentinel value that doesn't need to overload a valid value.
> > 
> > Let's implement strprefix(), a function that returns a pointer
> > right after the prefix if found, and a null pointer if not found.
> > 
> > We'll slowly migrate existing code from str_has_prefix() to strprefix().
> > 
> > Move the description from str_has_prefix(), and describe
> > str_has_prefix() by comparing it to strprefix().
> > 
> > Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> > Link: https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com/

> It's unreferred Link tag, spurious? Please, refer to it in the commit message
> to understand why it's here. Use Link: $URL [1] and use [1] in the text.

Ah, perhaps you wanted Closes tag instead?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 1/7] string: Add strprefix(), and deprecate str_has_prefix()

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1979 bytes --]

Hi Andy,

> Date: 2026-11-13 16:34:13+0200
> From: Andy Shevchenko <andriy.shevchenko@intel.com>
>
> On Fri, Nov 13, 2026 at 04:33:37PM +0200, Andy Shevchenko wrote:
> > On Fri, Nov 13, 2026 at 02:10:29PM +0100, Alejandro Colomar wrote:
> > > As Bartosz reported, str_has_prefix() has a bug: it can't differentiate
> > > between an empty prefix matching an empty string from a non-matching
> > > prefix.  This is a fundamental design flaw in this API, and not
> > > something that can be fixed.
> > > 
> > > As always, this is a consequence of choosing sizes instead of pointers.
> > > Sizes are often the wrong choice.  In this specific case, pointers are
> > > more capable of differentiating these cases, as NULL is a special
> > > sentinel value that doesn't need to overload a valid value.
> > > 
> > > Let's implement strprefix(), a function that returns a pointer
> > > right after the prefix if found, and a null pointer if not found.
> > > 
> > > We'll slowly migrate existing code from str_has_prefix() to strprefix().
> > > 
> > > Move the description from str_has_prefix(), and describe
> > > str_has_prefix() by comparing it to strprefix().
> > > 
> > > Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> > > Link: https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com/
> 
> > It's unreferred Link tag, spurious? Please, refer to it in the commit message
> > to understand why it's here. Use Link: $URL [1] and use [1] in the text.
> 
> Ah, perhaps you wanted Closes tag instead?

Yeah, it was for linking to Bart's report.  BTW, is it okay to have two
consecutive patches with the same Closes tag?  I guess it is, because
they close different aspects of the same issue.  But at the same time,
the first one doesn't fully close the link.  I guess yeah, I should just
put it in both.  Thanks!


Cheers,
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v5 7/7] string: Implement strstarts() with strprefix()

[Andy Shevchenko]

On Fri, Nov 13, 2026 at 02:11:17PM +0100, Alejandro Colomar wrote:
> The compiler should be able to optimize out the unused pointer
> arithmetic done at the end, so performance-wise this shouldn't change
> anything.

If strprefix() is not inline how would it be done? Or I missed the point?

> This change de-duplicates the almost-identical implementations,
> and better shows the relationship between the two functions.

...

>  static inline bool strstarts(const char *str, const char *prefix)
>  {
> -	return strncmp(str, prefix, strlen(prefix)) == 0;
> +	return strprefix(str, prefix);
>  }

There is no sense to keep strstarts() at this point. Can it be simply
redefinition? However, after looking at the uses in the kernel, I prefer
to see the strprefix() be not implemented at all, just optimization against
strstarts()/strends() if required can be performed.

-- 
With Best Regards,
Andy Shevchenko