[1/3] lib: early_string: allow early usage of some string functions
diff mbox series

Message ID 20210430042217.1198052-1-danielwa@cisco.com
State New, archived
Headers show
Series
  • [1/3] lib: early_string: allow early usage of some string functions
Related show

Commit Message

Daniel Walker April 30, 2021, 4:22 a.m. UTC
This systems allows some string functions to be moved into
lib/early_string.c and they will be prepended with "early_" and compiled
without debugging like KASAN.

This is already done on x86 for,
"AMD Secure Memory Encryption (SME) support"

and on powerpc prom_init.c , and EFI's libstub.

The AMD memory feature disabled KASAN for all string functions, and
prom_init.c and efi libstub implement their own versions of the
functions.

This implementation allows sharing of the string functions without
removing the debugging features for the whole system.

Cc: xe-linux-external@cisco.com
Signed-off-by: Daniel Walker <danielwa@cisco.com>
---
 include/linux/string.h |   6 ++
 lib/Makefile           |  23 +++++-
 lib/early_string.c     | 172 +++++++++++++++++++++++++++++++++++++++++
 lib/string.c           | 154 ------------------------------------
 4 files changed, 200 insertions(+), 155 deletions(-)
 create mode 100644 lib/early_string.c

Comments

Christophe Leroy April 30, 2021, 8:47 a.m. UTC | #1
Le 30/04/2021 à 06:22, Daniel Walker a écrit :
> This systems allows some string functions to be moved into
> lib/early_string.c and they will be prepended with "early_" and compiled
> without debugging like KASAN.
> 
> This is already done on x86 for,
> "AMD Secure Memory Encryption (SME) support"
> 
> and on powerpc prom_init.c , and EFI's libstub.
> 
> The AMD memory feature disabled KASAN for all string functions, and
> prom_init.c and efi libstub implement their own versions of the
> functions.
> 
> This implementation allows sharing of the string functions without
> removing the debugging features for the whole system.

This looks good. I prefer that rather than the way you proposed to do it two years ago.

Only one problem, see below.

> +size_t strlcat(char *dest, const char *src, size_t count)
> +{
> +	size_t dsize = strlen(dest);
> +	size_t len = strlen(src);
> +	size_t res = dsize + len;
> +
> +	/* This would be a bug */
> +	BUG_ON(dsize >= count);

powerpc is not ready to handle BUG_ON() in when in prom_init.

Can you do:

#ifndef __EARLY_STRING_ENABLED
	BUG_ON(dsize >= count);
#endif


> +
> +	dest += dsize;
> +	count -= dsize;
> +	if (len >= count)
> +		len = count-1;
> +	memcpy(dest, src, len);
> +	dest[len] = 0;
> +	return res;
> +}
> +EXPORT_SYMBOL(strlcat);
> +#endif
> +
Christophe Leroy April 30, 2021, 8:50 a.m. UTC | #2
Le 30/04/2021 à 10:47, Christophe Leroy a écrit :
> 
> 
> Le 30/04/2021 à 06:22, Daniel Walker a écrit :
>> This systems allows some string functions to be moved into
>> lib/early_string.c and they will be prepended with "early_" and compiled
>> without debugging like KASAN.
>>
>> This is already done on x86 for,
>> "AMD Secure Memory Encryption (SME) support"
>>
>> and on powerpc prom_init.c , and EFI's libstub.
>>
>> The AMD memory feature disabled KASAN for all string functions, and
>> prom_init.c and efi libstub implement their own versions of the
>> functions.
>>
>> This implementation allows sharing of the string functions without
>> removing the debugging features for the whole system.
> 
> This looks good. I prefer that rather than the way you proposed to do it two years ago.
> 
> Only one problem, see below.
> 
>> +size_t strlcat(char *dest, const char *src, size_t count)
>> +{
>> +    size_t dsize = strlen(dest);
>> +    size_t len = strlen(src);
>> +    size_t res = dsize + len;
>> +
>> +    /* This would be a bug */
>> +    BUG_ON(dsize >= count);
> 
> powerpc is not ready to handle BUG_ON() in when in prom_init.
> 
> Can you do:
> 
> #ifndef __EARLY_STRING_ENABLED
>      BUG_ON(dsize >= count);
> #endif

In fact, should be like in prom_init today:

#ifdef __EARLY_STRING_ENABLED
	if (dsize >= count)
		return count;
#else
	BUG_ON(dsize >= count);
#endif

> 
> 
>> +
>> +    dest += dsize;
>> +    count -= dsize;
>> +    if (len >= count)
>> +        len = count-1;
>> +    memcpy(dest, src, len);
>> +    dest[len] = 0;
>> +    return res;
>> +}
>> +EXPORT_SYMBOL(strlcat);
>> +#endif
>> +
Christophe Leroy May 1, 2021, 7:31 a.m. UTC | #3
Le 30/04/2021 à 10:50, Christophe Leroy a écrit :
> 
> 
> Le 30/04/2021 à 10:47, Christophe Leroy a écrit :
>>
>>
>> Le 30/04/2021 à 06:22, Daniel Walker a écrit :
>>> This systems allows some string functions to be moved into
>>> lib/early_string.c and they will be prepended with "early_" and compiled
>>> without debugging like KASAN.
>>>
>>> This is already done on x86 for,
>>> "AMD Secure Memory Encryption (SME) support"
>>>
>>> and on powerpc prom_init.c , and EFI's libstub.
>>>
>>> The AMD memory feature disabled KASAN for all string functions, and
>>> prom_init.c and efi libstub implement their own versions of the
>>> functions.
>>>
>>> This implementation allows sharing of the string functions without
>>> removing the debugging features for the whole system.
>>
>> This looks good. I prefer that rather than the way you proposed to do it two years ago.
>>
>> Only one problem, see below.
>>
>>> +size_t strlcat(char *dest, const char *src, size_t count)
>>> +{
>>> +    size_t dsize = strlen(dest);
>>> +    size_t len = strlen(src);
>>> +    size_t res = dsize + len;
>>> +
>>> +    /* This would be a bug */
>>> +    BUG_ON(dsize >= count);
>>
>> powerpc is not ready to handle BUG_ON() in when in prom_init.
>>
>> Can you do:
>>
>> #ifndef __EARLY_STRING_ENABLED
>>      BUG_ON(dsize >= count);
>> #endif
> 
> In fact, should be like in prom_init today:
> 
> #ifdef __EARLY_STRING_ENABLED
>      if (dsize >= count)
>          return count;
> #else
>      BUG_ON(dsize >= count);
> #endif

Thinking about it once more, this BUG_ON() is overkill and should be avoided, see 
https://www.kernel.org/doc/html/latest/process/deprecated.html

Therefore, something like the following would make it:

	if (dsize >= count) {
		WARN_ON(!__is_defined(__EARLY_STRING_ENABLED));

		return count;
	}

> 
>>
>>
>>> +
>>> +    dest += dsize;
>>> +    count -= dsize;
>>> +    if (len >= count)
>>> +        len = count-1;
>>> +    memcpy(dest, src, len);
>>> +    dest[len] = 0;
>>> +    return res;
>>> +}
>>> +EXPORT_SYMBOL(strlcat);
>>> +#endif
>>> +
Daniel Walker May 3, 2021, 6:01 p.m. UTC | #4
On Sat, May 01, 2021 at 09:31:47AM +0200, Christophe Leroy wrote:
> 
> > In fact, should be like in prom_init today:
> > 
> > #ifdef __EARLY_STRING_ENABLED
> >      if (dsize >= count)
> >          return count;
> > #else
> >      BUG_ON(dsize >= count);
> > #endif
> 
> Thinking about it once more, this BUG_ON() is overkill and should be
> avoided, see https://www.kernel.org/doc/html/latest/process/deprecated.html
> 
> Therefore, something like the following would make it:
> 
> 	if (dsize >= count) {
> 		WARN_ON(!__is_defined(__EARLY_STRING_ENABLED));
> 
> 		return count;
> 	}

I agree, it's overkill it stop the system for this condition.

how about I do something more like this for my changes,


> 	if (WARN_ON(dsize >= count && !__is_defined(__EARLY_STRING_ENABLED)))
> 		return count;

and for generic kernel,

> 	if (WARN_ON(dsize >= count))
> 		return count;



Daniel
Daniel Walker May 3, 2021, 6:06 p.m. UTC | #5
On Mon, May 03, 2021 at 11:01:41AM -0700, Daniel Walker wrote:
> On Sat, May 01, 2021 at 09:31:47AM +0200, Christophe Leroy wrote:
> > 
> > > In fact, should be like in prom_init today:
> > > 
> > > #ifdef __EARLY_STRING_ENABLED
> > >      if (dsize >= count)
> > >          return count;
> > > #else
> > >      BUG_ON(dsize >= count);
> > > #endif
> > 
> > Thinking about it once more, this BUG_ON() is overkill and should be
> > avoided, see https://www.kernel.org/doc/html/latest/process/deprecated.html
> > 
> > Therefore, something like the following would make it:
> > 
> > 	if (dsize >= count) {
> > 		WARN_ON(!__is_defined(__EARLY_STRING_ENABLED));
> > 
> > 		return count;
> > 	}
> 
> I agree, it's overkill it stop the system for this condition.
> 
> how about I do something more like this for my changes,
> 
> 
> > 	if (WARN_ON(dsize >= count && !__is_defined(__EARLY_STRING_ENABLED)))
> > 		return count;

I'll have to work on this one..

Daniel

Patch
diff mbox series

diff --git a/include/linux/string.h b/include/linux/string.h
index 9521d8cab18e..c0d45b92ba9e 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -20,6 +20,7 @@  extern void *memdup_user_nul(const void __user *, size_t);
  */
 #include <asm/string.h>
 
+extern char * early_strcpy(char *,const char *);
 #ifndef __HAVE_ARCH_STRCPY
 extern char * strcpy(char *,const char *);
 #endif
@@ -42,12 +43,15 @@  extern char * strcat(char *, const char *);
 #ifndef __HAVE_ARCH_STRNCAT
 extern char * strncat(char *, const char *, __kernel_size_t);
 #endif
+extern size_t early_strlcat(char *, const char *, __kernel_size_t);
 #ifndef __HAVE_ARCH_STRLCAT
 extern size_t strlcat(char *, const char *, __kernel_size_t);
 #endif
+extern int early_strcmp(const char *,const char *);
 #ifndef __HAVE_ARCH_STRCMP
 extern int strcmp(const char *,const char *);
 #endif
+extern int early_strncmp(const char *,const char *,__kernel_size_t);
 #ifndef __HAVE_ARCH_STRNCMP
 extern int strncmp(const char *,const char *,__kernel_size_t);
 #endif
@@ -79,12 +83,14 @@  static inline __must_check char *strstrip(char *str)
 	return strim(str);
 }
 
+extern char * early_strstr(const char *, const char *);
 #ifndef __HAVE_ARCH_STRSTR
 extern char * strstr(const char *, const char *);
 #endif
 #ifndef __HAVE_ARCH_STRNSTR
 extern char * strnstr(const char *, const char *, size_t);
 #endif
+extern __kernel_size_t early_strlen(const char *);
 #ifndef __HAVE_ARCH_STRLEN
 extern __kernel_size_t strlen(const char *);
 #endif
diff --git a/lib/Makefile b/lib/Makefile
index b5307d3eec1a..25cc664f027e 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -9,6 +9,8 @@  ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE)
 # flaky coverage that is not a function of syscall inputs. For example,
 # rbtree can be global and individual rotations don't correlate with inputs.
 KCOV_INSTRUMENT_string.o := n
+KCOV_INSTRUMENT_early_string.o := n
+KCOV_INSTRUMENT_early_string.nosan.o := n
 KCOV_INSTRUMENT_rbtree.o := n
 KCOV_INSTRUMENT_list_debug.o := n
 KCOV_INSTRUMENT_debugobjects.o := n
@@ -19,6 +21,12 @@  KCOV_INSTRUMENT_fault-inject.o := n
 # Use -ffreestanding to ensure that the compiler does not try to "optimize"
 # them into calls to themselves.
 CFLAGS_string.o := -ffreestanding
+CFLAGS_early_string.o := -ffreestanding
+CFLAGS_early_string.nosan.o := -ffreestanding -D__EARLY_STRING_ENABLED
+
+KASAN_SANITIZE_early_string.nosan.o := n
+
+CFLAGS_early_string.nosan.o += -fno-stack-protector
 
 # Early boot use of cmdline, don't instrument it
 ifdef CONFIG_AMD_MEM_ENCRYPT
@@ -27,7 +35,20 @@  KASAN_SANITIZE_string.o := n
 CFLAGS_string.o += -fno-stack-protector
 endif
 
-lib-y := ctype.o string.o vsprintf.o cmdline.o \
+$(obj)/early_string.nosan.o: $(src)/early_string.c $(recordmcount_source) $(objtool_dep) FORCE
+	$(call if_changed_rule,cc_o_c)
+	$(call cmd,force_checksrc)
+	$(Q)$(OBJCOPY) \
+		--rename-section .text=.init.text \
+		--redefine-sym strcmp=early_strcmp \
+		--redefine-sym strncmp=early_strncmp \
+		--redefine-sym strcpy=early_strcpy \
+		--redefine-sym strlcat=early_strlcat \
+		--redefine-sym strlen=early_strlen \
+		--redefine-sym strstr=early_strstr \
+		--redefine-sym memcmp=early_memcmp $@
+
+lib-y := ctype.o string.o early_string.o early_string.nosan.o vsprintf.o cmdline.o \
 	 rbtree.o radix-tree.o timerqueue.o xarray.o \
 	 idr.o extable.o sha1.o irq_regs.o argv_split.o \
 	 flex_proportions.o ratelimit.o show_mem.o \
diff --git a/lib/early_string.c b/lib/early_string.c
new file mode 100644
index 000000000000..21004e82159c
--- /dev/null
+++ b/lib/early_string.c
@@ -0,0 +1,172 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  linux/lib/string.c
+ *
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+#include <linux/types.h>
+#include <linux/export.h>
+#include <linux/bug.h>
+
+#ifdef __EARLY_STRING_ENABLED
+#undef EXPORT_SYMBOL
+#define EXPORT_SYMBOL(x)
+#endif
+
+#include <linux/string.h>
+
+#if !defined(__HAVE_ARCH_MEMCMP) || defined(__EARLY_STRING_ENABLED)
+/**
+ * memcmp - Compare two areas of memory
+ * @cs: One area of memory
+ * @ct: Another area of memory
+ * @count: The size of the area.
+ */
+#undef memcmp
+__visible int memcmp(const void *cs, const void *ct, size_t count)
+{
+	const unsigned char *su1, *su2;
+	int res = 0;
+
+	for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--)
+		if ((res = *su1 - *su2) != 0)
+			break;
+	return res;
+}
+EXPORT_SYMBOL(memcmp);
+#endif
+
+#if !defined(HAVE_ARCH_STRCMP) || defined(__EARLY_STRING_ENABLED)
+/**
+ * strcmp - Compare two strings
+ * @cs: One string
+ * @ct: Another string
+ */
+int strcmp(const char *cs, const char *ct)
+{
+	unsigned char c1, c2;
+
+	while (1) {
+		c1 = *cs++;
+		c2 = *ct++;
+		if (c1 != c2)
+			return c1 < c2 ? -1 : 1;
+		if (!c1)
+			break;
+	}
+	return 0;
+}
+EXPORT_SYMBOL(strcmp);
+#endif
+
+#if !defined(__HAVE_ARCH_STRCPY) || defined(__EARLY_STRING_ENABLED)
+/**
+ * strcpy - Copy a %NUL terminated string
+ * @dest: Where to copy the string to
+ * @src: Where to copy the string from
+ */
+char *strcpy(char *dest, const char *src)
+{
+	char *tmp = dest;
+
+	while ((*dest++ = *src++) != '\0')
+		/* nothing */;
+	return tmp;
+}
+EXPORT_SYMBOL(strcpy);
+#endif
+
+#if !defined(__HAVE_ARCH_STRLCAT) || defined(__EARLY_STRING_ENABLED)
+/**
+ * strlcat - Append a length-limited, C-string to another
+ * @dest: The string to be appended to
+ * @src: The string to append to it
+ * @count: The size of the destination buffer.
+ */
+size_t strlcat(char *dest, const char *src, size_t count)
+{
+	size_t dsize = strlen(dest);
+	size_t len = strlen(src);
+	size_t res = dsize + len;
+
+	/* This would be a bug */
+	BUG_ON(dsize >= count);
+
+	dest += dsize;
+	count -= dsize;
+	if (len >= count)
+		len = count-1;
+	memcpy(dest, src, len);
+	dest[len] = 0;
+	return res;
+}
+EXPORT_SYMBOL(strlcat);
+#endif
+
+#if !defined(__HAVE_ARCH_STRLEN) || defined(__EARLY_STRING_ENABLED)
+/**
+ * strlen - Find the length of a string
+ * @s: The string to be sized
+ */
+size_t strlen(const char *s)
+{
+	const char *sc;
+
+	for (sc = s; *sc != '\0'; ++sc)
+		/* nothing */;
+	return sc - s;
+}
+EXPORT_SYMBOL(strlen);
+#endif
+
+#if !defined(__HAVE_ARCH_STRNCMP) || defined(__EARLY_STRING_ENABLED)
+/**
+ * strncmp - Compare two length-limited strings
+ * @cs: One string
+ * @ct: Another string
+ * @count: The maximum number of bytes to compare
+ */
+int strncmp(const char *cs, const char *ct, size_t count)
+{
+	unsigned char c1, c2;
+
+	while (count) {
+		c1 = *cs++;
+		c2 = *ct++;
+		if (c1 != c2)
+			return c1 < c2 ? -1 : 1;
+		if (!c1)
+			break;
+		count--;
+	}
+	return 0;
+}
+EXPORT_SYMBOL(strncmp);
+#endif
+
+#if !defined(__HAVE_ARCH_STRSTR) || defined(__EARLY_STRING_ENABLED)
+/**
+ * strstr - Find the first substring in a %NUL terminated string
+ * @s1: The string to be searched
+ * @s2: The string to search for
+ */
+char *strstr(const char *s1, const char *s2)
+{
+	size_t l1, l2;
+
+	l2 = strlen(s2);
+	if (!l2)
+		return (char *)s1;
+	l1 = strlen(s1);
+	while (l1 >= l2) {
+		l1--;
+		if (!memcmp(s1, s2, l2))
+			return (char *)s1;
+		s1++;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL(strstr);
+#endif
+
diff --git a/lib/string.c b/lib/string.c
index 7548eb715ddb..328649ccb34d 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -79,23 +79,6 @@  int strcasecmp(const char *s1, const char *s2)
 EXPORT_SYMBOL(strcasecmp);
 #endif
 
-#ifndef __HAVE_ARCH_STRCPY
-/**
- * strcpy - Copy a %NUL terminated string
- * @dest: Where to copy the string to
- * @src: Where to copy the string from
- */
-char *strcpy(char *dest, const char *src)
-{
-	char *tmp = dest;
-
-	while ((*dest++ = *src++) != '\0')
-		/* nothing */;
-	return tmp;
-}
-EXPORT_SYMBOL(strcpy);
-#endif
-
 #ifndef __HAVE_ARCH_STRNCPY
 /**
  * strncpy - Copy a length-limited, C-string
@@ -343,81 +326,6 @@  char *strncat(char *dest, const char *src, size_t count)
 EXPORT_SYMBOL(strncat);
 #endif
 
-#ifndef __HAVE_ARCH_STRLCAT
-/**
- * strlcat - Append a length-limited, C-string to another
- * @dest: The string to be appended to
- * @src: The string to append to it
- * @count: The size of the destination buffer.
- */
-size_t strlcat(char *dest, const char *src, size_t count)
-{
-	size_t dsize = strlen(dest);
-	size_t len = strlen(src);
-	size_t res = dsize + len;
-
-	/* This would be a bug */
-	BUG_ON(dsize >= count);
-
-	dest += dsize;
-	count -= dsize;
-	if (len >= count)
-		len = count-1;
-	memcpy(dest, src, len);
-	dest[len] = 0;
-	return res;
-}
-EXPORT_SYMBOL(strlcat);
-#endif
-
-#ifndef __HAVE_ARCH_STRCMP
-/**
- * strcmp - Compare two strings
- * @cs: One string
- * @ct: Another string
- */
-int strcmp(const char *cs, const char *ct)
-{
-	unsigned char c1, c2;
-
-	while (1) {
-		c1 = *cs++;
-		c2 = *ct++;
-		if (c1 != c2)
-			return c1 < c2 ? -1 : 1;
-		if (!c1)
-			break;
-	}
-	return 0;
-}
-EXPORT_SYMBOL(strcmp);
-#endif
-
-#ifndef __HAVE_ARCH_STRNCMP
-/**
- * strncmp - Compare two length-limited strings
- * @cs: One string
- * @ct: Another string
- * @count: The maximum number of bytes to compare
- */
-int strncmp(const char *cs, const char *ct, size_t count)
-{
-	unsigned char c1, c2;
-
-	while (count) {
-		c1 = *cs++;
-		c2 = *ct++;
-		if (c1 != c2)
-			return c1 < c2 ? -1 : 1;
-		if (!c1)
-			break;
-		count--;
-	}
-	return 0;
-}
-EXPORT_SYMBOL(strncmp);
-#endif
-
 #ifndef __HAVE_ARCH_STRCHR
 /**
  * strchr - Find the first occurrence of a character in a string
@@ -553,22 +461,6 @@  char *strim(char *s)
 }
 EXPORT_SYMBOL(strim);
 
-#ifndef __HAVE_ARCH_STRLEN
-/**
- * strlen - Find the length of a string
- * @s: The string to be sized
- */
-size_t strlen(const char *s)
-{
-	const char *sc;
-
-	for (sc = s; *sc != '\0'; ++sc)
-		/* nothing */;
-	return sc - s;
-}
-EXPORT_SYMBOL(strlen);
-#endif
-
 #ifndef __HAVE_ARCH_STRNLEN
 /**
  * strnlen - Find the length of a length-limited string
@@ -922,27 +814,6 @@  void *memmove(void *dest, const void *src, size_t count)
 EXPORT_SYMBOL(memmove);
 #endif
 
-#ifndef __HAVE_ARCH_MEMCMP
-/**
- * memcmp - Compare two areas of memory
- * @cs: One area of memory
- * @ct: Another area of memory
- * @count: The size of the area.
- */
-#undef memcmp
-__visible int memcmp(const void *cs, const void *ct, size_t count)
-{
-	const unsigned char *su1, *su2;
-	int res = 0;
-
-	for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--)
-		if ((res = *su1 - *su2) != 0)
-			break;
-	return res;
-}
-EXPORT_SYMBOL(memcmp);
-#endif
-
 #ifndef __HAVE_ARCH_BCMP
 /**
  * bcmp - returns 0 if and only if the buffers have identical contents.
@@ -987,31 +858,6 @@  void *memscan(void *addr, int c, size_t size)
 EXPORT_SYMBOL(memscan);
 #endif
 
-#ifndef __HAVE_ARCH_STRSTR
-/**
- * strstr - Find the first substring in a %NUL terminated string
- * @s1: The string to be searched
- * @s2: The string to search for
- */
-char *strstr(const char *s1, const char *s2)
-{
-	size_t l1, l2;
-
-	l2 = strlen(s2);
-	if (!l2)
-		return (char *)s1;
-	l1 = strlen(s1);
-	while (l1 >= l2) {
-		l1--;
-		if (!memcmp(s1, s2, l2))
-			return (char *)s1;
-		s1++;
-	}
-	return NULL;
-}
-EXPORT_SYMBOL(strstr);
-#endif
-
 #ifndef __HAVE_ARCH_STRNSTR
 /**
  * strnstr - Find the first substring in a length-limited string