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

[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().

[ ... ]

> 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