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

[Alejandro Colomar <alx@kernel.org>]

[-- 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 --]