[PATCH v3 0/5] Add strprefix() and fix a bug with it

[Alejandro Colomar <alx@kernel.org>]

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

Hi!

str_has_prefix() is based on sizes and lengths, which means it must
overload the valid length 0 to mean not found.  This is a typical
consequence of choosing to design an API around sizes and lengths, and
vanishes when moving to pointer-based similar APIs.

According to its author (Steven R.), str_has_prefix() was never meant to
accept an empty prefix, but that's not documented, and there's no
obvious reason why that would be acceptable for a generic string
function (as opposed to a function very specific to its original use).
Users of strings need a function whose behavior extends naturally to the
empty string, and we can do better, by returning a pointer.  And indeed,
users have ended up using str_has_prefix() with empty prefix, expecting
it to work (as is natural of a generic string API).

This patch set adds strprefix(), deprecates str_has_prefix(), and uses
strprefix() in one place to fix a bug reported by Bartosz.

We can later work on replacing the other uses of str_has_prefix(), which
might hide similar bugs.

This patch set also adds infrastructure to make string functions return
a pointer that is as qualified as the input pointer.

Changes in v2
~~~~~~~~~~~~~

-  Copy the description from str_has_prefix().  [Steve]
-  Add links for the Reported-by tags.
-  Make these APIs const-generic.  [Geert]

I didn't apply some changes requested by reviewers:

-  I didn't remove the deprecation of str_has_prefix(), because I think
   it should not be used in new code.  It remains useful for code that
   is already using it, because rewriting all the surrounding code to
   use pointers instead of offsets might be too risky.  However, for new
   code, strprefix() produces simpler code, and is safer, so I don't see
   why str_has_prefix() would be good in any new code.

-  I didn't rename strprefix() to strsuffix(), because that name would
   be worse (strsuffix() is widely implemented with other semantics, and
   having it different here would lead to confusion and bugs).  Andy
   agreed.

See range-diff below.

Testing
~~~~~~~

I haven't changed the implementation of the function, so haven't re-run
tests.  Since the changes since v2 are only about type safety, with a
new wrapper macro, I have built again with 'allmodconfig', just to make
sure it still compiles.


Have a lovely day!
Alex

v1: <https://lore.kernel.org/linux-hardening/cover.1793876837.git.alx@kernel.org/>
v2: <https://lore.kernel.org/linux-hardening/cover.1793882260.git.alx@kernel.org/>

Alejandro Colomar (5):
  string: Add strprefix(), and deprecate str_has_prefix()
  device property: Fix fwnode_name_eq() for empty node names
  stddef: Add const_cast()
  string: Add [C]QChar_of() and QVoid_of()
  string: Make strprefix() const-qualifier-generic

 drivers/base/property.c  |  5 +---
 include/linux/stddef.h   | 12 ++++++++
 include/linux/string.h   | 63 +++++++++++++++++++++++++++++++++++++---
 lib/tests/string_kunit.c |  9 ++++++
 4 files changed, 81 insertions(+), 8 deletions(-)

Range-diff against v2:
1:  d3b325ecefd1 ! 1:  556766412884 string: Add strprefix(), and deprecate str_has_prefix()
    @@ Commit message
     
         We'll slowly migrate existing code from str_has_prefix() to strprefix().
     
    +    Copy the description from str_has_prefix().
    +
         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 ##
     @@ include/linux/string.h: void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
    - 	       sizeof(*(obj)) - offsetof(typeof(*(obj)), member));	\
      })
      
    -+/**
    + /**
    +- * 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
     + * strprefix - string prefix
     + * @s: The string to be tested
     + * @prefix: The string to see if @s starts with
    -+ *
    -+ * 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()
    -+ *
       * 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().
    ++ *
    ++ * 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
    ++ *
    ++ * DEPRECATED: Use strprefix()
    ++ *
    ++ * 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 strprefix().
       *
       * Returns:
2:  ac9728afa027 ! 2:  eed085c85f36 device property: Fix fwnode_name_eq() for empty node names
    @@ Commit message
     
         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>
     
-:  ------------ > 3:  2bf5cf7baad8 stddef: Add const_cast()
-:  ------------ > 4:  415453a31ebf string: Add [C]QChar_of() and QVoid_of()
-:  ------------ > 5:  35bf6672f84f string: Make strprefix() const-qualifier-generic
-- 
2.55.0


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