2021-04-30 04:25:40

by Daniel Walker (danielwa)

[permalink] [raw]
Subject: [PATCH 1/3] lib: early_string: allow early usage of some string functions

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: [email protected]
Signed-off-by: Daniel Walker <[email protected]>
---
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

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
--
2.25.1


2021-04-30 04:26:02

by Daniel Walker (danielwa)

[permalink] [raw]
Subject: [PATCH 3/3] x86: switch amd mem encrypt to early string functions

This switched x86 early string users to use the early string variants
and re-enabled KASAN on general string functions use thru out the rest
of the system.

Cc: [email protected]
Signed-off-by: Daniel Walker <[email protected]>
---
arch/x86/mm/mem_encrypt_identity.c | 4 ++--
lib/Makefile | 7 -------
2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/arch/x86/mm/mem_encrypt_identity.c b/arch/x86/mm/mem_encrypt_identity.c
index 6c5eb6f3f14f..212fe90cf5e2 100644
--- a/arch/x86/mm/mem_encrypt_identity.c
+++ b/arch/x86/mm/mem_encrypt_identity.c
@@ -575,9 +575,9 @@ void __init sme_enable(struct boot_params *bp)

cmdline_find_option(cmdline_ptr, cmdline_arg, buffer, sizeof(buffer));

- if (!strncmp(buffer, cmdline_on, sizeof(buffer)))
+ if (!early_strncmp(buffer, cmdline_on, sizeof(buffer)))
sme_me_mask = me_mask;
- else if (!strncmp(buffer, cmdline_off, sizeof(buffer)))
+ else if (!early_strncmp(buffer, cmdline_off, sizeof(buffer)))
sme_me_mask = 0;
else
sme_me_mask = active_by_default ? me_mask : 0;
diff --git a/lib/Makefile b/lib/Makefile
index 25cc664f027e..314db12c0e98 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -28,13 +28,6 @@ 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
-KASAN_SANITIZE_string.o := n
-
-CFLAGS_string.o += -fno-stack-protector
-endif
-
$(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)
--
2.25.1

2021-04-30 04:26:14

by Daniel Walker (danielwa)

[permalink] [raw]
Subject: [PATCH 2/3] powerpc: prom_init: switch to early string functions

This converts the prom_init string users to the early string function
which don't suffer from KASAN or any other debugging enabled.

Cc: [email protected]
Signed-off-by: Daniel Walker <[email protected]>
---
arch/powerpc/kernel/prom_init.c | 185 ++++++-------------------
arch/powerpc/kernel/prom_init_check.sh | 9 +-
2 files changed, 51 insertions(+), 143 deletions(-)

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index ccf77b985c8f..4d4343da1280 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -225,105 +225,6 @@ static bool __prombss rtas_has_query_cpu_stopped;
#define PHANDLE_VALID(p) ((p) != 0 && (p) != PROM_ERROR)
#define IHANDLE_VALID(i) ((i) != 0 && (i) != PROM_ERROR)

-/* Copied from lib/string.c and lib/kstrtox.c */
-
-static int __init prom_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;
-}
-
-static char __init *prom_strcpy(char *dest, const char *src)
-{
- char *tmp = dest;
-
- while ((*dest++ = *src++) != '\0')
- /* nothing */;
- return tmp;
-}
-
-static int __init prom_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;
-}
-
-static size_t __init prom_strlen(const char *s)
-{
- const char *sc;
-
- for (sc = s; *sc != '\0'; ++sc)
- /* nothing */;
- return sc - s;
-}
-
-static int __init prom_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;
-}
-
-static char __init *prom_strstr(const char *s1, const char *s2)
-{
- size_t l1, l2;
-
- l2 = prom_strlen(s2);
- if (!l2)
- return (char *)s1;
- l1 = prom_strlen(s1);
- while (l1 >= l2) {
- l1--;
- if (!prom_memcmp(s1, s2, l2))
- return (char *)s1;
- s1++;
- }
- return NULL;
-}
-
-static size_t __init prom_strlcat(char *dest, const char *src, size_t count)
-{
- size_t dsize = prom_strlen(dest);
- size_t len = prom_strlen(src);
- size_t res = dsize + len;
-
- /* This would be a bug */
- if (dsize >= count)
- return count;
-
- dest += dsize;
- count -= dsize;
- if (len >= count)
- len = count-1;
- memcpy(dest, src, len);
- dest[len] = 0;
- return res;
-
-}
-
#ifdef CONFIG_PPC_PSERIES
static int __init prom_strtobool(const char *s, bool *res)
{
@@ -694,7 +595,7 @@ static int __init prom_setprop(phandle node, const char *nodename,
add_string(&p, tohex((u32)(unsigned long) value));
add_string(&p, tohex(valuelen));
add_string(&p, tohex(ADDR(pname)));
- add_string(&p, tohex(prom_strlen(pname)));
+ add_string(&p, tohex(early_strlen(pname)));
add_string(&p, "property");
*p = 0;
return call_prom("interpret", 1, 1, (u32)(unsigned long) cmd);
@@ -779,25 +680,25 @@ static void __init early_cmdline_parse(void)
l = prom_getprop(prom.chosen, "bootargs", p, COMMAND_LINE_SIZE-1);

if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || l <= 0 || p[0] == '\0')
- prom_strlcat(prom_cmd_line, " " CONFIG_CMDLINE,
+ early_strlcat(prom_cmd_line, " " CONFIG_CMDLINE,
sizeof(prom_cmd_line));

prom_printf("command line: %s\n", prom_cmd_line);

#ifdef CONFIG_PPC64
- opt = prom_strstr(prom_cmd_line, "iommu=");
+ opt = early_strstr(prom_cmd_line, "iommu=");
if (opt) {
prom_printf("iommu opt is: %s\n", opt);
opt += 6;
while (*opt && *opt == ' ')
opt++;
- if (!prom_strncmp(opt, "off", 3))
+ if (!early_strncmp(opt, "off", 3))
prom_iommu_off = 1;
- else if (!prom_strncmp(opt, "force", 5))
+ else if (!early_strncmp(opt, "force", 5))
prom_iommu_force_on = 1;
}
#endif
- opt = prom_strstr(prom_cmd_line, "mem=");
+ opt = early_strstr(prom_cmd_line, "mem=");
if (opt) {
opt += 4;
prom_memory_limit = prom_memparse(opt, (const char **)&opt);
@@ -809,7 +710,7 @@ static void __init early_cmdline_parse(void)

#ifdef CONFIG_PPC_PSERIES
prom_radix_disable = !IS_ENABLED(CONFIG_PPC_RADIX_MMU_DEFAULT);
- opt = prom_strstr(prom_cmd_line, "disable_radix");
+ opt = early_strstr(prom_cmd_line, "disable_radix");
if (opt) {
opt += 13;
if (*opt && *opt == '=') {
@@ -825,13 +726,13 @@ static void __init early_cmdline_parse(void)
if (prom_radix_disable)
prom_debug("Radix disabled from cmdline\n");

- opt = prom_strstr(prom_cmd_line, "radix_hcall_invalidate=on");
+ opt = early_strstr(prom_cmd_line, "radix_hcall_invalidate=on");
if (opt) {
prom_radix_gtse_disable = true;
prom_debug("Radix GTSE disabled from cmdline\n");
}

- opt = prom_strstr(prom_cmd_line, "xive=off");
+ opt = early_strstr(prom_cmd_line, "xive=off");
if (opt) {
prom_xive_disable = true;
prom_debug("XIVE disabled from cmdline\n");
@@ -839,7 +740,7 @@ static void __init early_cmdline_parse(void)
#endif /* CONFIG_PPC_PSERIES */

#ifdef CONFIG_PPC_SVM
- opt = prom_strstr(prom_cmd_line, "svm=");
+ opt = early_strstr(prom_cmd_line, "svm=");
if (opt) {
bool val;

@@ -1199,7 +1100,7 @@ static int __init prom_count_smt_threads(void)
type[0] = 0;
prom_getprop(node, "device_type", type, sizeof(type));

- if (prom_strcmp(type, "cpu"))
+ if (early_strcmp(type, "cpu"))
continue;
/*
* There is an entry for each smt thread, each entry being
@@ -1650,7 +1551,7 @@ static void __init prom_init_mem(void)
*/
prom_getprop(node, "name", type, sizeof(type));
}
- if (prom_strcmp(type, "memory"))
+ if (early_strcmp(type, "memory"))
continue;

plen = prom_getprop(node, "reg", regbuf, sizeof(regbuf));
@@ -1972,19 +1873,19 @@ static void __init prom_initialize_tce_table(void)
prom_getprop(node, "device_type", type, sizeof(type));
prom_getprop(node, "model", model, sizeof(model));

- if ((type[0] == 0) || (prom_strstr(type, "pci") == NULL))
+ if ((type[0] == 0) || (early_strstr(type, "pci") == NULL))
continue;

/* Keep the old logic intact to avoid regression. */
if (compatible[0] != 0) {
- if ((prom_strstr(compatible, "python") == NULL) &&
- (prom_strstr(compatible, "Speedwagon") == NULL) &&
- (prom_strstr(compatible, "Winnipeg") == NULL))
+ if ((early_strstr(compatible, "python") == NULL) &&
+ (early_strstr(compatible, "Speedwagon") == NULL) &&
+ (early_strstr(compatible, "Winnipeg") == NULL))
continue;
} else if (model[0] != 0) {
- if ((prom_strstr(model, "ython") == NULL) &&
- (prom_strstr(model, "peedwagon") == NULL) &&
- (prom_strstr(model, "innipeg") == NULL))
+ if ((early_strstr(model, "ython") == NULL) &&
+ (early_strstr(model, "peedwagon") == NULL) &&
+ (early_strstr(model, "innipeg") == NULL))
continue;
}

@@ -2133,12 +2034,12 @@ static void __init prom_hold_cpus(void)

type[0] = 0;
prom_getprop(node, "device_type", type, sizeof(type));
- if (prom_strcmp(type, "cpu") != 0)
+ if (early_strcmp(type, "cpu") != 0)
continue;

/* Skip non-configured cpus. */
if (prom_getprop(node, "status", type, sizeof(type)) > 0)
- if (prom_strcmp(type, "okay") != 0)
+ if (early_strcmp(type, "okay") != 0)
continue;

reg = cpu_to_be32(-1); /* make sparse happy */
@@ -2214,9 +2115,9 @@ static void __init prom_find_mmu(void)
return;
version[sizeof(version) - 1] = 0;
/* XXX might need to add other versions here */
- if (prom_strcmp(version, "Open Firmware, 1.0.5") == 0)
+ if (early_strcmp(version, "Open Firmware, 1.0.5") == 0)
of_workarounds = OF_WA_CLAIM;
- else if (prom_strncmp(version, "FirmWorks,3.", 12) == 0) {
+ else if (early_strncmp(version, "FirmWorks,3.", 12) == 0) {
of_workarounds = OF_WA_CLAIM | OF_WA_LONGTRAIL;
call_prom("interpret", 1, 1, "dev /memory 0 to allow-reclaim");
} else
@@ -2249,7 +2150,7 @@ static void __init prom_init_stdout(void)
call_prom("instance-to-path", 3, 1, prom.stdout, path, 255);
prom_printf("OF stdout device is: %s\n", of_stdout_device);
prom_setprop(prom.chosen, "/chosen", "linux,stdout-path",
- path, prom_strlen(path) + 1);
+ path, early_strlen(path) + 1);

/* instance-to-package fails on PA-Semi */
stdout_node = call_prom("instance-to-package", 1, 1, prom.stdout);
@@ -2259,7 +2160,7 @@ static void __init prom_init_stdout(void)
/* If it's a display, note it */
memset(type, 0, sizeof(type));
prom_getprop(stdout_node, "device_type", type, sizeof(type));
- if (prom_strcmp(type, "display") == 0)
+ if (early_strcmp(type, "display") == 0)
prom_setprop(stdout_node, path, "linux,boot-display", NULL, 0);
}
}
@@ -2280,19 +2181,19 @@ static int __init prom_find_machine_type(void)
compat[len] = 0;
while (i < len) {
char *p = &compat[i];
- int sl = prom_strlen(p);
+ int sl = early_strlen(p);
if (sl == 0)
break;
- if (prom_strstr(p, "Power Macintosh") ||
- prom_strstr(p, "MacRISC"))
+ if (early_strstr(p, "Power Macintosh") ||
+ early_strstr(p, "MacRISC"))
return PLATFORM_POWERMAC;
#ifdef CONFIG_PPC64
/* We must make sure we don't detect the IBM Cell
* blades as pSeries due to some firmware issues,
* so we do it here.
*/
- if (prom_strstr(p, "IBM,CBEA") ||
- prom_strstr(p, "IBM,CPBW-1.0"))
+ if (early_strstr(p, "IBM,CBEA") ||
+ early_strstr(p, "IBM,CPBW-1.0"))
return PLATFORM_GENERIC;
#endif /* CONFIG_PPC64 */
i += sl + 1;
@@ -2309,7 +2210,7 @@ static int __init prom_find_machine_type(void)
compat, sizeof(compat)-1);
if (len <= 0)
return PLATFORM_GENERIC;
- if (prom_strcmp(compat, "chrp"))
+ if (early_strcmp(compat, "chrp"))
return PLATFORM_GENERIC;

/* Default to pSeries. We need to know if we are running LPAR */
@@ -2371,7 +2272,7 @@ static void __init prom_check_displays(void)
for (node = 0; prom_next_node(&node); ) {
memset(type, 0, sizeof(type));
prom_getprop(node, "device_type", type, sizeof(type));
- if (prom_strcmp(type, "display") != 0)
+ if (early_strcmp(type, "display") != 0)
continue;

/* It seems OF doesn't null-terminate the path :-( */
@@ -2485,9 +2386,9 @@ static unsigned long __init dt_find_string(char *str)
s = os = (char *)dt_string_start;
s += 4;
while (s < (char *)dt_string_end) {
- if (prom_strcmp(s, str) == 0)
+ if (early_strcmp(s, str) == 0)
return s - os;
- s += prom_strlen(s) + 1;
+ s += early_strlen(s) + 1;
}
return 0;
}
@@ -2520,7 +2421,7 @@ static void __init scan_dt_build_strings(phandle node,
}

/* skip "name" */
- if (prom_strcmp(namep, "name") == 0) {
+ if (early_strcmp(namep, "name") == 0) {
*mem_start = (unsigned long)namep;
prev_name = "name";
continue;
@@ -2532,7 +2433,7 @@ static void __init scan_dt_build_strings(phandle node,
namep = sstart + soff;
} else {
/* Trim off some if we can */
- *mem_start = (unsigned long)namep + prom_strlen(namep) + 1;
+ *mem_start = (unsigned long)namep + early_strlen(namep) + 1;
dt_string_end = *mem_start;
}
prev_name = namep;
@@ -2601,7 +2502,7 @@ static void __init scan_dt_build_struct(phandle node, unsigned long *mem_start,
break;

/* skip "name" */
- if (prom_strcmp(pname, "name") == 0) {
+ if (early_strcmp(pname, "name") == 0) {
prev_name = "name";
continue;
}
@@ -2632,7 +2533,7 @@ static void __init scan_dt_build_struct(phandle node, unsigned long *mem_start,
call_prom("getprop", 4, 1, node, pname, valp, l);
*mem_start = ALIGN(*mem_start, 4);

- if (!prom_strcmp(pname, "phandle"))
+ if (!early_strcmp(pname, "phandle"))
has_phandle = 1;
}

@@ -2702,8 +2603,8 @@ static void __init flatten_device_tree(void)

/* Add "phandle" in there, we'll need it */
namep = make_room(&mem_start, &mem_end, 16, 1);
- prom_strcpy(namep, "phandle");
- mem_start = (unsigned long)namep + prom_strlen(namep) + 1;
+ early_strcpy(namep, "phandle");
+ mem_start = (unsigned long)namep + early_strlen(namep) + 1;

/* Build string array */
prom_printf("Building dt strings...\n");
@@ -3025,7 +2926,7 @@ static void __init fixup_device_tree_efika(void)
rv = prom_getprop(node, "model", prop, sizeof(prop));
if (rv == PROM_ERROR)
return;
- if (prom_strcmp(prop, "EFIKA5K2"))
+ if (early_strcmp(prop, "EFIKA5K2"))
return;

prom_printf("Applying EFIKA device tree fixups\n");
@@ -3033,13 +2934,13 @@ static void __init fixup_device_tree_efika(void)
/* Claiming to be 'chrp' is death */
node = call_prom("finddevice", 1, 1, ADDR("/"));
rv = prom_getprop(node, "device_type", prop, sizeof(prop));
- if (rv != PROM_ERROR && (prom_strcmp(prop, "chrp") == 0))
+ if (rv != PROM_ERROR && (early_strcmp(prop, "chrp") == 0))
prom_setprop(node, "/", "device_type", "efika", sizeof("efika"));

/* CODEGEN,description is exposed in /proc/cpuinfo so
fix that too */
rv = prom_getprop(node, "CODEGEN,description", prop, sizeof(prop));
- if (rv != PROM_ERROR && (prom_strstr(prop, "CHRP")))
+ if (rv != PROM_ERROR && (early_strstr(prop, "CHRP")))
prom_setprop(node, "/", "CODEGEN,description",
"Efika 5200B PowerPC System",
sizeof("Efika 5200B PowerPC System"));
diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
index b183ab9c5107..bbdeba750d52 100644
--- a/arch/powerpc/kernel/prom_init_check.sh
+++ b/arch/powerpc/kernel/prom_init_check.sh
@@ -27,7 +27,14 @@ __secondary_hold_acknowledge __secondary_hold_spinloop __start
logo_linux_clut224 btext_prepare_BAT
reloc_got2 kernstart_addr memstart_addr linux_banner _stext
__prom_init_toc_start __prom_init_toc_end btext_setup_display TOC.
-relocate"
+relocate
+early_strcmp
+early_strcpy
+early_strlcat
+early_strlen
+early_strncmp
+early_strstr
+"

NM="$1"
OBJ="$2"
--
2.25.1

2021-04-30 08:48:35

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH 1/3] lib: early_string: allow early usage of some string functions



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
> +

2021-04-30 08:52:01

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH 1/3] lib: early_string: allow early usage of some string functions



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
>> +

2021-04-30 08:52:40

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH 2/3] powerpc: prom_init: switch to early string functions



Le 30/04/2021 à 06:22, Daniel Walker a écrit :
> This converts the prom_init string users to the early string function
> which don't suffer from KASAN or any other debugging enabled.
>
> Cc: [email protected]
> Signed-off-by: Daniel Walker <[email protected]>
> ---
> arch/powerpc/kernel/prom_init.c | 185 ++++++-------------------
> arch/powerpc/kernel/prom_init_check.sh | 9 +-
> 2 files changed, 51 insertions(+), 143 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index ccf77b985c8f..4d4343da1280 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -225,105 +225,6 @@ static bool __prombss rtas_has_query_cpu_stopped;
> #define PHANDLE_VALID(p) ((p) != 0 && (p) != PROM_ERROR)
> #define IHANDLE_VALID(i) ((i) != 0 && (i) != PROM_ERROR)
>
> -/* Copied from lib/string.c and lib/kstrtox.c */

Please leave the second part of the comment as you have not removed prom_strtobool()

2021-05-01 07:52:08

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH 1/3] lib: early_string: allow early usage of some string functions



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
>>> +

2021-05-03 18:16:41

by Daniel Walker (danielwa)

[permalink] [raw]
Subject: Re: [PATCH 1/3] lib: early_string: allow early usage of some string functions

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

2021-05-03 18:16:56

by Daniel Walker (danielwa)

[permalink] [raw]
Subject: Re: [PATCH 1/3] lib: early_string: allow early usage of some string functions

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