Re: [PATCH v6 09/19] string: Make strstarts() a legacy alias for strprefix()

[Alejandro Colomar <alx+linux-hardening@kernel.org>]

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

> Date: 2026-11-14 16:51:58+0100
> From: Alejandro Colomar <alx@kernel.org>
>
> > Date: 2026-11-14 15:33:19+0000
> > From: sashiko-bot@kernel.org
> >
> > Thank you for your contribution! Sashiko AI review found 1 potential
> > issue to consider:
> > - [High] string: strstarts() macro expansion shadowed by local strprefix
> >   variable
> > --
> > 
> > commit e4b277f22320c7ff4dbc9e8cc88221ad43eb8ba6
> > Author: Alejandro Colomar <alx@kernel.org>
> > 
> > string: Make strstarts() a legacy alias for strprefix()
> > 
> > This commit replaces the inline strstarts() function with a macro alias for
> > strprefix().
> > 
> > > diff --git a/include/linux/string.h b/include/linux/string.h
> > > index 013c6169def05..0a466360dbb39 100644
> > > --- a/include/linux/string.h
> > > +++ b/include/linux/string.h
> > > @@ -609,6 +609,7 @@ static inline const char *strprefix(const char *s, const char *prefix)
> > >  	return strncmp(s, prefix, len) == 0 ? s + len : NULL;
> > >  }
> > >  #define strprefix(s, pfx)  const_cast(CQChar_of(s) *, strprefix(s, pfx))
> > > +#define strstarts(s, pfx)  strprefix(s, pfx)  // legacy alias
> > 
> > [Severity: High]
> > Does expanding strstarts() to strprefix() break the build when a caller has
> > a local variable named strprefix?
> > 
> > drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c:hp_populate_security_buffer() {
> > 	u16 *auth = authbuf;
> > 	char *strprefix = NULL;
> > 	int ret = 0;
> > 
> > 	if (strstarts(authentication, BEAM_PREFIX)) {
> >     ...
> > }
> > 
> > Because the strprefix() macro expands to a call to the inline function
> > strprefix() in the caller's scope, the local pointer variable strprefix
> > shadows the function identifier and breaks compilation when
> > CONFIG_HP_BIOSCFG is enabled.  A later patch in this series, commit
> > "platform/x86: hp-bioscfg: Rename local variable", renames that variable to
> > prefix and should be ordered before this commit to keep the series
> > bisectable.
> 
> This reivew is wrong.  A local variable cannot shadow a macro.  Also,
> a function-like macro is only expanded if it's followed by an opening
> parenthesis.  In the case of spmobj-attributes.c, the local variable
> doesn't have a parenthesis, and thus doesn't trigger expansion.

After looking at it a second time, while the report from sashiko is
wrong, the code is actually wrong, and reordering the commits would be
appropriate.  I'll reorder them in v7.

> 
> > 
> > -- 
> > Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794667407.git.alx@kernel.org?part=9
> 
> -- 
> <https://www.alejandro-colomar.es>



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

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