linux-kernel - Re: [for-next][PATCH 23/24] string.h: Add strncmp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 21 Dec 2018 15:35:26 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Joe Perches <joe@...ches.com>,
        Tom Zanussi <zanussi@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [for-next][PATCH 23/24] string.h: Add strncmp_prefix() helper
 macro

On Fri, 21 Dec 2018 12:01:48 -0800
Linus Torvalds <torvalds@...ux-foundation.org> wrote:

> On Fri, Dec 21, 2018 at 11:40 AM Steven Rostedt <rostedt@...dmis.org> wrote:
> >
> > OK, what about if we just use strlen() and say that this macro is not
> > safe for parameters with side effects.  
> 
> I think gcc follows simple assignments just fine, and does the
> optimized strlen() for them too.
> 
> So why not just do
> 
>    #define have_prefix(str,prefix) ({ \
>         const char *__pfx = prefix; \
>         size_t __pfxlen = strlen(__pfx); \
>         strncmp(str, __pfx, __pfxlen) ? 0 : __pfxlen); })
> 
> and be done with it safely?
> 
> The above is ENTIRELY untested.
>

At first I thought this would have issues, but with a slight change...

#define have_prefix(str, prefix) ({ \
	const char *__pfx = (const char *)prefix; \


And the rest the same, it appears to work.

Need the cast because if for some reason someone passed in something
like "const unsigned char" then it wouldn't work. But that's just a nit.

So something like this then?

-- Steve

diff --git a/include/linux/string.h b/include/linux/string.h
index 27d0482e5e05..4586fee60194 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -14,6 +14,28 @@ extern void *memdup_user(const void __user *, size_t);
 extern void *vmemdup_user(const void __user *, size_t);
 extern void *memdup_user_nul(const void __user *, size_t);
 
+/**
+ * have_prefix - Test if a string has a given prefix
+ * @str: The string to test
+ * @prefix: The string to see if @str starts with
+ *
+ * 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 has_prefix().
+ *
+ * Returns: 0 if @str does not start with @prefix
+         strlen(@prefix) if @str does start with @prefix
+ */
+#define has_prefix(str, prefix)						\
+	({								\
+		const char *____prefix____ = (const char *)(prefix);	\
+		int ____len____ = strlen(____prefix____);		\
+		strncmp(str, ____prefix____, ____len____) == 0 ?	\
+			____len____ : 0;				\
+	})
+
 /*
  * Include machine specific inline routines
  */