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

[Steven Rostedt]

On Thu, 5 Nov 2026 17:33:02 +0100
Alejandro Colomar <alx+linux-hardening@xxxxxxxxxx> wrote:

> Hi Steve,
> 
> > Date: 2026-11-05 11:14:23-0500
> > From: Steven Rostedt <rostedt@xxxxxxxxxxx>
> >
> > On Thu, 5 Nov 2026 12:13:03 +0100
> > Alejandro Colomar <alx@xxxxxxxxxx> 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().
> > > 
> > > Reported-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxxxxxxxx>  
> > 
> > Is there a link to the report? Please add that.  
> 
> Message-Id: <20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@xxxxxxxxxxxxxxxx>
> 
> or in link format,
> 
> Link: <https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@xxxxxxxxxxxxxxxx/>
> 
> Which one do you prefer?

The correct method is:

 Link: https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@xxxxxxxxxxxxxxxx/



> 
> >   
> > >  
> > > +/**
> > > + * strprefix - string prefix
> > > + * @s: The string to be tested
> > > + * @prefix: The string to see if @s starts with
> > > + *  
> > 
> > Needs a better description. Feel free to use the one for str_has_prefix().  
> 
> Ok.
> 
> > > + * Returns:
> > > + * *  @s+strlen(@prefix) if @s starts with @prefix
> > > + * *  NULL if @s does not start with @prefix
> > > + */
> > > +static __always_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
> > >   *
> > > + * DEPRECATED: Use strprefix()  
> > 
> > No, it is still useful. Converting trace_event_filter() to strprefix()
> > would be very painful, and likely error prone.  
> 
> What is trace_event_filter()?

Oops, typo, I meant trace_event_filter.c

> 
> 	alx@devuan:~/src/linux/linux/master$ grep -rn '\<trace_event_filter\>'
> 	alx@devuan:~/src/linux/linux/master$ grepc trace_event_filter .
> 	alx@devuan:~/src/linux/linux/master$ grep -rn trace_event_filter
> 	kernel/trace/trace_events.c:2833:static const struct file_operations ftrace_event_filter_fops = {
> 	kernel/trace/trace_events.c:3222:			*fops = &ftrace_event_filter_fops;
> 
> If you point me to a specific line of code, I can attempt a rewrite.

I rather not touch that code.

> 
> > > + *
> > >   * 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().
> > > + * and not a constant. Instead use strprefix().  
> > 
> > The above can be replaced with:
> > 
> >  * This is useful for testing for a prefix and returning the length.
> >  * If you only need to test the prefix and perhaps get the pointer to the
> >  * position of the string after the prefix, then use strprefix().  
> 
> Ok, conditional to being convinced that str_has_prefix() is
> actually useful in some case.

Note, the only "bug" is that it can't handle an empty prefix case, which
was never what it was designed to do. The place that the above fixes, was a
helper function that used this inappropriately.

Hence, the use of str_has_prefix() was never meant for an empty string. I
should have added (and can still add)

	/* Empty prefix is undefined */
	WARN_ON_ONCE(!prefix[0]);

The fwnode_name_eq() could have done:

	/* Empty prefix always matches */
	return name[0] || str_has_prefix(node_name, name) == len;

Instead.

-- Steve