* [PATCH v5 1/7] string: Add strprefix(), and deprecate str_has_prefix()
2026-11-13 13:10 [PATCH v5 0/7] Add strprefix() and fix a bug with it Alejandro Colomar
@ 2026-11-13 13:10 ` Alejandro Colomar
2026-11-13 14:33 ` Andy Shevchenko
2026-11-13 13:10 ` [PATCH v5 2/7] string: Warn if the prefix is the empty string Alejandro Colomar
` (5 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Alejandro Colomar @ 2026-11-13 13:10 UTC (permalink / raw)
To: linux-hardening
Cc: Alejandro Colomar, Krzysztof Kozlowski, Maciej W. Rozycki,
Geert Uytterhoeven, Andreas Schwab, Andy Shevchenko,
Bartosz Golaszewski, Daniel Scally, Danilo Krummrich,
Greg Kroah-Hartman, Hans Verkuil, Heikki Krogerus, Joe Perches,
Johan Hovold, Kees Cook, Laurent Pinchart, Len Brown,
Linus Torvalds, Linus Walleij, Namhyung Kim, Rafael J. Wysocki,
Rob Herring, Sakari Ailus, Saravana Kannan, Steven Rostedt,
Tom Zanussi, linux-acpi, driver-core, linux-kernel, devicetree,
linux-gpio, Bartosz Golaszewski
[-- 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 --]
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v5 1/7] string: Add strprefix(), and deprecate str_has_prefix()
2026-11-13 13:10 ` [PATCH v5 1/7] string: Add strprefix(), and deprecate str_has_prefix() Alejandro Colomar
@ 2026-11-13 14:33 ` Andy Shevchenko
2026-11-13 14:34 ` Andy Shevchenko
0 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2026-11-13 14:33 UTC (permalink / raw)
To: Alejandro Colomar
Cc: linux-hardening, Krzysztof Kozlowski, Maciej W. Rozycki,
Geert Uytterhoeven, Andreas Schwab, Andy Shevchenko,
Bartosz Golaszewski, Daniel Scally, Danilo Krummrich,
Greg Kroah-Hartman, Hans Verkuil, Heikki Krogerus, Joe Perches,
Johan Hovold, Kees Cook, Laurent Pinchart, Len Brown,
Linus Torvalds, Linus Walleij, Namhyung Kim, Rafael J. Wysocki,
Rob Herring, Sakari Ailus, Saravana Kannan, Steven Rostedt,
Tom Zanussi, linux-acpi, driver-core, linux-kernel, devicetree,
linux-gpio, Bartosz Golaszewski
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 1/7] string: Add strprefix(), and deprecate str_has_prefix()
2026-11-13 14:33 ` Andy Shevchenko
@ 2026-11-13 14:34 ` Andy Shevchenko
2026-11-13 14:40 ` Alejandro Colomar
0 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2026-11-13 14:34 UTC (permalink / raw)
To: Alejandro Colomar
Cc: linux-hardening, Krzysztof Kozlowski, Maciej W. Rozycki,
Geert Uytterhoeven, Andreas Schwab, Andy Shevchenko,
Bartosz Golaszewski, Daniel Scally, Danilo Krummrich,
Greg Kroah-Hartman, Hans Verkuil, Heikki Krogerus, Joe Perches,
Johan Hovold, Kees Cook, Laurent Pinchart, Len Brown,
Linus Torvalds, Linus Walleij, Namhyung Kim, Rafael J. Wysocki,
Rob Herring, Sakari Ailus, Saravana Kannan, Steven Rostedt,
Tom Zanussi, linux-acpi, driver-core, linux-kernel, devicetree,
linux-gpio, Bartosz Golaszewski
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 1/7] string: Add strprefix(), and deprecate str_has_prefix()
2026-11-13 14:34 ` Andy Shevchenko
@ 2026-11-13 14:40 ` Alejandro Colomar
0 siblings, 0 replies; 18+ messages in thread
From: Alejandro Colomar @ 2026-11-13 14:40 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-hardening, Krzysztof Kozlowski, Maciej W. Rozycki,
Geert Uytterhoeven, Andreas Schwab, Andy Shevchenko,
Bartosz Golaszewski, Daniel Scally, Danilo Krummrich,
Greg Kroah-Hartman, Hans Verkuil, Heikki Krogerus, Joe Perches,
Johan Hovold, Kees Cook, Laurent Pinchart, Len Brown,
Linus Torvalds, Linus Walleij, Namhyung Kim, Rafael J. Wysocki,
Rob Herring, Sakari Ailus, Saravana Kannan, Steven Rostedt,
Tom Zanussi, linux-acpi, driver-core, linux-kernel, devicetree,
linux-gpio, Bartosz Golaszewski
[-- 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 --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v5 2/7] string: Warn if the prefix is the empty string
2026-11-13 13:10 [PATCH v5 0/7] Add strprefix() and fix a bug with it Alejandro Colomar
2026-11-13 13:10 ` [PATCH v5 1/7] string: Add strprefix(), and deprecate str_has_prefix() Alejandro Colomar
@ 2026-11-13 13:10 ` Alejandro Colomar
2026-11-13 13:26 ` sashiko-bot
2026-11-13 13:10 ` [PATCH v5 3/7] device property: Fix fwnode_name_eq() for empty node names Alejandro Colomar
` (4 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Alejandro Colomar @ 2026-11-13 13:10 UTC (permalink / raw)
To: linux-hardening
Cc: Alejandro Colomar, Krzysztof Kozlowski, Maciej W. Rozycki,
Geert Uytterhoeven, Andreas Schwab, Andy Shevchenko,
Bartosz Golaszewski, Daniel Scally, Danilo Krummrich,
Greg Kroah-Hartman, Hans Verkuil, Heikki Krogerus, Joe Perches,
Johan Hovold, Kees Cook, Laurent Pinchart, Len Brown,
Linus Torvalds, Linus Walleij, Namhyung Kim, Rafael J. Wysocki,
Rob Herring, Sakari Ailus, Saravana Kannan, Steven Rostedt,
Tom Zanussi, linux-acpi, driver-core, linux-kernel, devicetree,
linux-gpio, Bartosz Golaszewski, Andy Shevchenko
[-- 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 --]
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v5 2/7] string: Warn if the prefix is the empty string
2026-11-13 13:10 ` [PATCH v5 2/7] string: Warn if the prefix is the empty string Alejandro Colomar
@ 2026-11-13 13:26 ` sashiko-bot
2026-11-13 13:48 ` Alejandro Colomar
0 siblings, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-11-13 13:26 UTC (permalink / raw)
To: Alejandro Colomar; +Cc: devicetree, linux-gpio, robh, conor+dt
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
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v5 2/7] string: Warn if the prefix is the empty string
2026-11-13 13:26 ` sashiko-bot
@ 2026-11-13 13:48 ` Alejandro Colomar
2026-11-13 14:25 ` Andy Shevchenko
0 siblings, 1 reply; 18+ messages in thread
From: Alejandro Colomar @ 2026-11-13 13:48 UTC (permalink / raw)
To: sashiko-reviews, linux-hardening
Cc: devicetree, linux-gpio, robh, conor+dt, Bartosz Golaszewski,
Andy Shevchenko
[-- 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 --]
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v5 2/7] string: Warn if the prefix is the empty string
2026-11-13 13:48 ` Alejandro Colomar
@ 2026-11-13 14:25 ` Andy Shevchenko
2026-11-13 14:32 ` Alejandro Colomar
0 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2026-11-13 14:25 UTC (permalink / raw)
To: Alejandro Colomar
Cc: sashiko-reviews, linux-hardening, devicetree, linux-gpio, robh,
conor+dt, Bartosz Golaszewski, 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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 2/7] string: Warn if the prefix is the empty string
2026-11-13 14:25 ` Andy Shevchenko
@ 2026-11-13 14:32 ` Alejandro Colomar
0 siblings, 0 replies; 18+ messages in thread
From: Alejandro Colomar @ 2026-11-13 14:32 UTC (permalink / raw)
To: Andy Shevchenko
Cc: sashiko-reviews, linux-hardening, devicetree, linux-gpio, robh,
conor+dt, Bartosz Golaszewski, Andy Shevchenko
[-- 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 --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v5 3/7] device property: Fix fwnode_name_eq() for empty node names
2026-11-13 13:10 [PATCH v5 0/7] Add strprefix() and fix a bug with it Alejandro Colomar
2026-11-13 13:10 ` [PATCH v5 1/7] string: Add strprefix(), and deprecate str_has_prefix() Alejandro Colomar
2026-11-13 13:10 ` [PATCH v5 2/7] string: Warn if the prefix is the empty string Alejandro Colomar
@ 2026-11-13 13:10 ` Alejandro Colomar
2026-11-13 13:10 ` [PATCH v5 4/7] stddef: Add const_cast() Alejandro Colomar
` (3 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Alejandro Colomar @ 2026-11-13 13:10 UTC (permalink / raw)
To: linux-hardening
Cc: Alejandro Colomar, Krzysztof Kozlowski, Maciej W. Rozycki,
Geert Uytterhoeven, Andreas Schwab, Andy Shevchenko,
Bartosz Golaszewski, Daniel Scally, Danilo Krummrich,
Greg Kroah-Hartman, Hans Verkuil, Heikki Krogerus, Joe Perches,
Johan Hovold, Kees Cook, Laurent Pinchart, Len Brown,
Linus Torvalds, Linus Walleij, Namhyung Kim, Rafael J. Wysocki,
Rob Herring, Sakari Ailus, Saravana Kannan, Steven Rostedt,
Tom Zanussi, linux-acpi, driver-core, linux-kernel, devicetree,
linux-gpio, Bartosz Golaszewski
[-- 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 --]
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v5 4/7] stddef: Add const_cast()
2026-11-13 13:10 [PATCH v5 0/7] Add strprefix() and fix a bug with it Alejandro Colomar
` (2 preceding siblings ...)
2026-11-13 13:10 ` [PATCH v5 3/7] device property: Fix fwnode_name_eq() for empty node names Alejandro Colomar
@ 2026-11-13 13:10 ` Alejandro Colomar
2026-11-13 13:11 ` [PATCH v5 5/7] string: Add [C]QChar_of() and QVoid_of() Alejandro Colomar
` (2 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Alejandro Colomar @ 2026-11-13 13:10 UTC (permalink / raw)
To: linux-hardening
Cc: Alejandro Colomar, Krzysztof Kozlowski, Maciej W. Rozycki,
Geert Uytterhoeven, Andreas Schwab, Andy Shevchenko,
Bartosz Golaszewski, Daniel Scally, Danilo Krummrich,
Greg Kroah-Hartman, Hans Verkuil, Heikki Krogerus, Joe Perches,
Johan Hovold, Kees Cook, Laurent Pinchart, Len Brown,
Linus Torvalds, Linus Walleij, Namhyung Kim, Rafael J. Wysocki,
Rob Herring, Sakari Ailus, Saravana Kannan, Steven Rostedt,
Tom Zanussi, linux-acpi, driver-core, linux-kernel, devicetree,
linux-gpio, David Laight
[-- 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 --]
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v5 5/7] string: Add [C]QChar_of() and QVoid_of()
2026-11-13 13:10 [PATCH v5 0/7] Add strprefix() and fix a bug with it Alejandro Colomar
` (3 preceding siblings ...)
2026-11-13 13:10 ` [PATCH v5 4/7] stddef: Add const_cast() Alejandro Colomar
@ 2026-11-13 13:11 ` Alejandro Colomar
2026-11-13 13:11 ` [PATCH v5 6/7] string: Make strprefix() const-qualifier-generic Alejandro Colomar
2026-11-13 13:11 ` [PATCH v5 7/7] string: Implement strstarts() with strprefix() Alejandro Colomar
6 siblings, 0 replies; 18+ messages in thread
From: Alejandro Colomar @ 2026-11-13 13:11 UTC (permalink / raw)
To: linux-hardening
Cc: Alejandro Colomar, Krzysztof Kozlowski, Maciej W. Rozycki,
Geert Uytterhoeven, Andreas Schwab, Andy Shevchenko,
Bartosz Golaszewski, Daniel Scally, Danilo Krummrich,
Greg Kroah-Hartman, Hans Verkuil, Heikki Krogerus, Joe Perches,
Johan Hovold, Kees Cook, Laurent Pinchart, Len Brown,
Linus Torvalds, Linus Walleij, Namhyung Kim, Rafael J. Wysocki,
Rob Herring, Sakari Ailus, Saravana Kannan, Steven Rostedt,
Tom Zanussi, linux-acpi, driver-core, linux-kernel, devicetree,
linux-gpio
[-- 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 --]
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v5 6/7] string: Make strprefix() const-qualifier-generic
2026-11-13 13:10 [PATCH v5 0/7] Add strprefix() and fix a bug with it Alejandro Colomar
` (4 preceding siblings ...)
2026-11-13 13:11 ` [PATCH v5 5/7] string: Add [C]QChar_of() and QVoid_of() Alejandro Colomar
@ 2026-11-13 13:11 ` Alejandro Colomar
2026-11-13 13:11 ` [PATCH v5 7/7] string: Implement strstarts() with strprefix() Alejandro Colomar
6 siblings, 0 replies; 18+ messages in thread
From: Alejandro Colomar @ 2026-11-13 13:11 UTC (permalink / raw)
To: linux-hardening
Cc: Alejandro Colomar, Krzysztof Kozlowski, Maciej W. Rozycki,
Geert Uytterhoeven, Andreas Schwab, Andy Shevchenko,
Bartosz Golaszewski, Daniel Scally, Danilo Krummrich,
Greg Kroah-Hartman, Hans Verkuil, Heikki Krogerus, Joe Perches,
Johan Hovold, Kees Cook, Laurent Pinchart, Len Brown,
Linus Torvalds, Linus Walleij, Namhyung Kim, Rafael J. Wysocki,
Rob Herring, Sakari Ailus, Saravana Kannan, Steven Rostedt,
Tom Zanussi, linux-acpi, driver-core, linux-kernel, devicetree,
linux-gpio, David Laight
[-- 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 --]
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v5 7/7] string: Implement strstarts() with strprefix()
2026-11-13 13:10 [PATCH v5 0/7] Add strprefix() and fix a bug with it Alejandro Colomar
` (5 preceding siblings ...)
2026-11-13 13:11 ` [PATCH v5 6/7] string: Make strprefix() const-qualifier-generic Alejandro Colomar
@ 2026-11-13 13:11 ` Alejandro Colomar
2026-11-13 13:20 ` sashiko-bot
2026-11-13 14:44 ` Andy Shevchenko
6 siblings, 2 replies; 18+ messages in thread
From: Alejandro Colomar @ 2026-11-13 13:11 UTC (permalink / raw)
To: linux-hardening
Cc: Alejandro Colomar, Krzysztof Kozlowski, Maciej W. Rozycki,
Geert Uytterhoeven, Andreas Schwab, Andy Shevchenko,
Bartosz Golaszewski, Daniel Scally, Danilo Krummrich,
Greg Kroah-Hartman, Hans Verkuil, Heikki Krogerus, Joe Perches,
Johan Hovold, Kees Cook, Laurent Pinchart, Len Brown,
Linus Torvalds, Linus Walleij, Namhyung Kim, Rafael J. Wysocki,
Rob Herring, Sakari Ailus, Saravana Kannan, Steven Rostedt,
Tom Zanussi, linux-acpi, driver-core, linux-kernel, devicetree,
linux-gpio
[-- 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 --]
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v5 7/7] string: Implement strstarts() with strprefix()
2026-11-13 13:11 ` [PATCH v5 7/7] string: Implement strstarts() with strprefix() Alejandro Colomar
@ 2026-11-13 13:20 ` sashiko-bot
2026-11-13 13:36 ` Alejandro Colomar
2026-11-13 14:44 ` Andy Shevchenko
1 sibling, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-11-13 13:20 UTC (permalink / raw)
To: Alejandro Colomar; +Cc: devicetree, linux-gpio, robh, conor+dt
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
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v5 7/7] string: Implement strstarts() with strprefix()
2026-11-13 13:20 ` sashiko-bot
@ 2026-11-13 13:36 ` Alejandro Colomar
0 siblings, 0 replies; 18+ messages in thread
From: Alejandro Colomar @ 2026-11-13 13:36 UTC (permalink / raw)
To: sashiko-reviews; +Cc: devicetree, linux-gpio, robh, conor+dt, linux-hardening
[-- 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 --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 7/7] string: Implement strstarts() with strprefix()
2026-11-13 13:11 ` [PATCH v5 7/7] string: Implement strstarts() with strprefix() Alejandro Colomar
2026-11-13 13:20 ` sashiko-bot
@ 2026-11-13 14:44 ` Andy Shevchenko
1 sibling, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2026-11-13 14:44 UTC (permalink / raw)
To: Alejandro Colomar
Cc: linux-hardening, Krzysztof Kozlowski, Maciej W. Rozycki,
Geert Uytterhoeven, Andreas Schwab, Andy Shevchenko,
Bartosz Golaszewski, Daniel Scally, Danilo Krummrich,
Greg Kroah-Hartman, Hans Verkuil, Heikki Krogerus, Joe Perches,
Johan Hovold, Kees Cook, Laurent Pinchart, Len Brown,
Linus Torvalds, Linus Walleij, Namhyung Kim, Rafael J. Wysocki,
Rob Herring, Sakari Ailus, Saravana Kannan, Steven Rostedt,
Tom Zanussi, linux-acpi, driver-core, linux-kernel, devicetree,
linux-gpio
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
^ permalink raw reply [flat|nested] 18+ messages in thread