[PATCH v3 0/4] RISC-V: Parse DT for Zkr to seed KASLR

[PATCH v3 0/4] RISC-V: Parse DT for Zkr to seed KASLR

[Jesse Taube]

Add functions to pi/fdt_early.c to help parse the FDT to check if
the isa string has the Zkr extension. Then use the Zkr extension to
seed the KASLR base address.

The first two patches fix the visibility of symbols.

V1 -> V2:
 - Add RISC-V: pi: Force hidden visibility for all symbol references
 - Add RISC-V: pi: Add kernel/pi/pi.h
 - Rewrite archrandom_early.c to parse DT over checking the csr
V2 -> V3:
 - Add RISC-V: lib: Add pi aliases for string functions
 - Rewrite isa_string_contains in third patch

Jesse Taube (4):
  RISC-V: pi: Force hidden visibility for all symbol references
  RISC-V: lib: Add pi aliases for string functions
  RISC-V: pi: Add kernel/pi/pi.h
  RISC-V: Use Zkr to seed KASLR base address

 arch/riscv/kernel/pi/Makefile           |   3 +-
 arch/riscv/kernel/pi/archrandom_early.c |  30 +++++
 arch/riscv/kernel/pi/cmdline_early.c    |  10 +-
 arch/riscv/kernel/pi/fdt_early.c        | 167 +++++++++++++++++++++++-
 arch/riscv/kernel/pi/pi.h               |  20 +++
 arch/riscv/lib/memset.S                 |   2 +
 arch/riscv/lib/strcmp.S                 |   1 +
 arch/riscv/lib/strncmp.S                |   1 +
 arch/riscv/mm/init.c                    |   5 +-
 9 files changed, 223 insertions(+), 16 deletions(-)
 create mode 100644 arch/riscv/kernel/pi/archrandom_early.c
 create mode 100644 arch/riscv/kernel/pi/pi.h

-- 
2.45.2

[PATCH v3 1/4] RISC-V: pi: Force hidden visibility for all symbol references

[Jesse Taube]

Eliminate all GOT entries in the .pi section, by forcing hidden
visibility for all symbol references, which informs the compiler that
such references will be resolved at link time without the need for
allocating GOT entries.

Include linux/hidden.h in Makefile, like arm64, for the
hidden visibility attribute.

Signed-off-by: Jesse Taube <jesse@rivosinc.com>
Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
V1 -> V2:
 - New patch
V2 -> V3:
 - No changes
---
 arch/riscv/kernel/pi/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/riscv/kernel/pi/Makefile b/arch/riscv/kernel/pi/Makefile
index 50bc5ef7dd2f..1ef7584be0c3 100644
--- a/arch/riscv/kernel/pi/Makefile
+++ b/arch/riscv/kernel/pi/Makefile
@@ -5,6 +5,7 @@ KBUILD_CFLAGS	:= $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) -fpie \
 		   -Os -DDISABLE_BRANCH_PROFILING $(DISABLE_STACKLEAK_PLUGIN) \
 		   $(call cc-option,-mbranch-protection=none) \
 		   -I$(srctree)/scripts/dtc/libfdt -fno-stack-protector \
+		   -include $(srctree)/include/linux/hidden.h \
 		   -D__DISABLE_EXPORTS -ffreestanding \
 		   -fno-asynchronous-unwind-tables -fno-unwind-tables \
 		   $(call cc-option,-fno-addrsig)
-- 
2.45.2

[PATCH v3 2/4] RISC-V: lib: Add pi aliases for string functions

[Jesse Taube]

memset, strcmp, and strncmp are all used in the the __pi_ section.
add SYM_FUNC_ALIAS for them.

When KASAN is enabled in <asm/string.h> __pi___memset is also needed.

Suggested-by: Charlie Jenkins <charlie@rivosinc.com>
Signed-off-by: Jesse Taube <jesse@rivosinc.com>
---
V2 -> V3:
 - New patch
---
 arch/riscv/lib/memset.S  | 2 ++
 arch/riscv/lib/strcmp.S  | 1 +
 arch/riscv/lib/strncmp.S | 1 +
 3 files changed, 4 insertions(+)

diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
index 35f358e70bdb..da23b8347e2d 100644
--- a/arch/riscv/lib/memset.S
+++ b/arch/riscv/lib/memset.S
@@ -111,3 +111,5 @@ SYM_FUNC_START(__memset)
 	ret
 SYM_FUNC_END(__memset)
 SYM_FUNC_ALIAS_WEAK(memset, __memset)
+SYM_FUNC_ALIAS(__pi_memset, __memset)
+SYM_FUNC_ALIAS(__pi___memset, __memset)
diff --git a/arch/riscv/lib/strcmp.S b/arch/riscv/lib/strcmp.S
index 687b2bea5c43..bc73325b2fd1 100644
--- a/arch/riscv/lib/strcmp.S
+++ b/arch/riscv/lib/strcmp.S
@@ -120,3 +120,4 @@ strcmp_zbb:
 .option pop
 #endif
 SYM_FUNC_END(strcmp)
+SYM_FUNC_ALIAS(__pi_strcmp, strcmp)
diff --git a/arch/riscv/lib/strncmp.S b/arch/riscv/lib/strncmp.S
index aba5b3148621..b36325a57f6a 100644
--- a/arch/riscv/lib/strncmp.S
+++ b/arch/riscv/lib/strncmp.S
@@ -136,3 +136,4 @@ strncmp_zbb:
 .option pop
 #endif
 SYM_FUNC_END(strncmp)
+SYM_FUNC_ALIAS(__pi_strncmp, strncmp)
-- 
2.45.2

[PATCH v3 3/4] RISC-V: pi: Add kernel/pi/pi.h

[Jesse Taube]

Add pi.h header for declarations of the kernel/pi prefixed functions
and any other related declarations.

Suggested-by: Charlie Jenkins <charlie@rivosinc.com>
Signed-off-by: Jesse Taube <jesse@rivosinc.com>
Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
V1 -> V2:
 - New patch
V2 -> V3:
 - Spelling
---
 arch/riscv/kernel/pi/cmdline_early.c | 10 ++--------
 arch/riscv/kernel/pi/fdt_early.c     |  7 +------
 arch/riscv/kernel/pi/pi.h            | 17 +++++++++++++++++
 3 files changed, 20 insertions(+), 14 deletions(-)
 create mode 100644 arch/riscv/kernel/pi/pi.h

diff --git a/arch/riscv/kernel/pi/cmdline_early.c b/arch/riscv/kernel/pi/cmdline_early.c
index f6d4dedffb84..fbcdc9e4e143 100644
--- a/arch/riscv/kernel/pi/cmdline_early.c
+++ b/arch/riscv/kernel/pi/cmdline_early.c
@@ -6,15 +6,9 @@
 #include <asm/pgtable.h>
 #include <asm/setup.h>
 
-static char early_cmdline[COMMAND_LINE_SIZE];
+#include "pi.h"
 
-/*
- * Declare the functions that are exported (but prefixed) here so that LLVM
- * does not complain it lacks the 'static' keyword (which, if added, makes
- * LLVM complain because the function is actually unused in this file).
- */
-u64 set_satp_mode_from_cmdline(uintptr_t dtb_pa);
-bool set_nokaslr_from_cmdline(uintptr_t dtb_pa);
+static char early_cmdline[COMMAND_LINE_SIZE];
 
 static char *get_early_cmdline(uintptr_t dtb_pa)
 {
diff --git a/arch/riscv/kernel/pi/fdt_early.c b/arch/riscv/kernel/pi/fdt_early.c
index 899610e042ab..40ee299702bf 100644
--- a/arch/riscv/kernel/pi/fdt_early.c
+++ b/arch/riscv/kernel/pi/fdt_early.c
@@ -3,12 +3,7 @@
 #include <linux/init.h>
 #include <linux/libfdt.h>
 
-/*
- * Declare the functions that are exported (but prefixed) here so that LLVM
- * does not complain it lacks the 'static' keyword (which, if added, makes
- * LLVM complain because the function is actually unused in this file).
- */
-u64 get_kaslr_seed(uintptr_t dtb_pa);
+#include "pi.h"
 
 u64 get_kaslr_seed(uintptr_t dtb_pa)
 {
diff --git a/arch/riscv/kernel/pi/pi.h b/arch/riscv/kernel/pi/pi.h
new file mode 100644
index 000000000000..493c8cb7c0e6
--- /dev/null
+++ b/arch/riscv/kernel/pi/pi.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _RISCV_PI_H_
+#define _RISCV_PI_H_
+
+#include <linux/types.h>
+
+/*
+ * The following functions are exported (but prefixed). Declare them here so
+ * that LLVM does not complain it lacks the 'static' keyword (which, if
+ * added, makes LLVM complain because the function is unused).
+ */
+
+u64 get_kaslr_seed(uintptr_t dtb_pa);
+bool set_nokaslr_from_cmdline(uintptr_t dtb_pa);
+u64 set_satp_mode_from_cmdline(uintptr_t dtb_pa);
+
+#endif /* _RISCV_PI_H_ */
-- 
2.45.2

[PATCH v3 4/4] RISC-V: Use Zkr to seed KASLR base address

[Jesse Taube]

Parse the device tree for Zkr in the isa string.
If Zkr is present, use it to seed the kernel base address.

On an ACPI system, as of this commit, there is no easy way to check if
Zkr is present. Blindly running the instruction isn't an option as;
we have to be able to trust the firmware.

Signed-off-by: Jesse Taube <jesse@rivosinc.com>
---
V1 -> V2:
 - Almost entire rewrite
V2 -> V3:
 - Dont parse iscv,isa-base
 - Move fdt_early_match_extension_isa in pi.h under comment
 - Only check enabled cpus
 - Rename early_isa_str to fdt_early_match_extension_isa
 - Rename get_ext_named to early_cpu_isa_ext_available
 - Rewrite isa_string_contains
 - Update commit description
 - Use fdt_stringlist_contains for riscv,isa-extensions
---
 arch/riscv/kernel/pi/Makefile           |   2 +-
 arch/riscv/kernel/pi/archrandom_early.c |  30 +++++
 arch/riscv/kernel/pi/fdt_early.c        | 160 ++++++++++++++++++++++++
 arch/riscv/kernel/pi/pi.h               |   3 +
 arch/riscv/mm/init.c                    |   5 +-
 5 files changed, 198 insertions(+), 2 deletions(-)
 create mode 100644 arch/riscv/kernel/pi/archrandom_early.c

diff --git a/arch/riscv/kernel/pi/Makefile b/arch/riscv/kernel/pi/Makefile
index 1ef7584be0c3..dba902f2a538 100644
--- a/arch/riscv/kernel/pi/Makefile
+++ b/arch/riscv/kernel/pi/Makefile
@@ -33,5 +33,5 @@ $(obj)/string.o: $(srctree)/lib/string.c FORCE
 $(obj)/ctype.o: $(srctree)/lib/ctype.c FORCE
 	$(call if_changed_rule,cc_o_c)
 
-obj-y		:= cmdline_early.pi.o fdt_early.pi.o string.pi.o ctype.pi.o lib-fdt.pi.o lib-fdt_ro.pi.o
+obj-y		:= cmdline_early.pi.o fdt_early.pi.o string.pi.o ctype.pi.o lib-fdt.pi.o lib-fdt_ro.pi.o archrandom_early.pi.o
 extra-y		:= $(patsubst %.pi.o,%.o,$(obj-y))
diff --git a/arch/riscv/kernel/pi/archrandom_early.c b/arch/riscv/kernel/pi/archrandom_early.c
new file mode 100644
index 000000000000..3f05d3cf3b7b
--- /dev/null
+++ b/arch/riscv/kernel/pi/archrandom_early.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <asm/csr.h>
+#include <linux/processor.h>
+
+#include "pi.h"
+
+/*
+ * To avoid rewriting code include asm/archrandom.h and create macros
+ * for the functions that won't be included.
+ */
+#undef riscv_has_extension_unlikely
+#define riscv_has_extension_likely(...) false
+#undef pr_err_once
+#define pr_err_once(...)
+
+#include <asm/archrandom.h>
+
+u64 get_kaslr_seed_zkr(const uintptr_t dtb_pa)
+{
+	unsigned long seed = 0;
+
+	if (!fdt_early_match_extension_isa((const void *)dtb_pa, "zkr"))
+		return 0;
+
+	if (!csr_seed_long(&seed))
+		return 0;
+
+	return seed;
+}
diff --git a/arch/riscv/kernel/pi/fdt_early.c b/arch/riscv/kernel/pi/fdt_early.c
index 40ee299702bf..49ff5360bf87 100644
--- a/arch/riscv/kernel/pi/fdt_early.c
+++ b/arch/riscv/kernel/pi/fdt_early.c
@@ -2,6 +2,7 @@
 #include <linux/types.h>
 #include <linux/init.h>
 #include <linux/libfdt.h>
+#include <linux/ctype.h>
 
 #include "pi.h"
 
@@ -23,3 +24,162 @@ u64 get_kaslr_seed(uintptr_t dtb_pa)
 	*prop = 0;
 	return ret;
 }
+
+/**
+ *  fdt_device_is_available - check if a device is available for use
+ *
+ * @fdt: pointer to the device tree blob
+ * @node: offset of the node whose property to find
+ *
+ *  Returns true if the status property is absent or set to "okay" or "ok",
+ *  false otherwise
+ */
+static bool fdt_device_is_available(const void *fdt, int node)
+{
+	const char *status;
+	int statlen;
+
+	status = fdt_getprop(fdt, node, "status", &statlen);
+	if (!status)
+		return true;
+
+	if (statlen > 0) {
+		if (!strcmp(status, "okay") || !strcmp(status, "ok"))
+			return true;
+	}
+
+	return false;
+}
+
+/* Copy of fdt_nodename_eq_ */
+static int fdt_node_name_eq(const void *fdt, int offset,
+			    const char *s)
+{
+	int olen;
+	int len = strlen(s);
+	const char *p = fdt_get_name(fdt, offset, &olen);
+
+	if (!p || olen < len)
+		/* short match */
+		return 0;
+
+	if (memcmp(p, s, len) != 0)
+		return 0;
+
+	if (p[len] == '\0')
+		return 1;
+	else if (!memchr(s, '@', len) && (p[len] == '@'))
+		return 1;
+	else
+		return 0;
+}
+
+/**
+ *  isa_string_contains - check if isa string contains an extension
+ *
+ * @isa_str: isa string to search
+ * @ext_name: the extension to search for
+ *
+ *  Returns true if the extension is in the given isa string,
+ *  false otherwise
+ */
+static bool isa_string_contains(const char *isa_str, const char *ext_name)
+{
+	size_t i, single_end, len = strlen(ext_name);
+	char ext_end;
+
+	/* Error must contain rv32/64 */
+	if (strlen(isa_str) < 4)
+		return false;
+
+	if (len == 1) {
+		single_end = strcspn(isa_str, "sSxXzZ");
+		/* Search for single chars between rv32/64 and multi-letter extensions */
+		for (i = 4; i < single_end; i++) {
+			if (tolower(isa_str[i]) == ext_name[0])
+				return true;
+		}
+		return false;
+	}
+
+	/* Skip to start of multi-letter extensions */
+	isa_str = strpbrk(isa_str, "sSxXzZ");
+	while (isa_str) {
+		if (strncasecmp(isa_str, ext_name, len) == 0) {
+			ext_end = isa_str[len];
+			/* Check if matches the whole extension excluding version. */
+			if (ext_end == '\0' || ext_end == '_' || isdigit(ext_end))
+				return true;
+		}
+		/* Multi-letter extensions must be split from other multi-letter
+		 * extensions with an "_", the end of a multi-letter extension will
+		 * either be the null character or the "_" at the start of the next
+		 * multi-letter extension.
+		 */
+		isa_str = strchr(isa_str, '_');
+		if (isa_str)
+			isa_str++;
+	}
+
+	return false;
+}
+
+/**
+ *  early_cpu_isa_ext_available - check if cpu node has an extension
+ *
+ * @fdt: pointer to the device tree blob
+ * @node: offset of the cpu node
+ * @ext_name: the extension to search for
+ *
+ *  Returns true if the cpu node has the extension,
+ *  false otherwise
+ */
+static bool early_cpu_isa_ext_available(const void *fdt, int node, const char *ext_name)
+{
+	const void *prop;
+	int len;
+
+	prop = fdt_getprop(fdt, node, "riscv,isa-extensions", &len);
+	if (prop && fdt_stringlist_contains(prop, len, ext_name))
+		return true;
+
+	prop = fdt_getprop(fdt, node, "riscv,isa", &len);
+	if (prop && isa_string_contains(prop, ext_name))
+		return true;
+
+	return false;
+}
+
+/**
+ *  fdt_early_match_extension_isa - check if all cpu nodes have an extension
+ *
+ * @fdt: pointer to the device tree blob
+ * @ext_name: the extension to search for
+ *
+ *  Returns true if the all available the cpu nodes have the extension,
+ *  false otherwise
+ */
+bool fdt_early_match_extension_isa(const void *fdt, const char *ext_name)
+{
+	int node, parent;
+	bool ret = false;
+
+	parent = fdt_path_offset(fdt, "/cpus");
+	if (parent < 0)
+		return false;
+
+	fdt_for_each_subnode(node, fdt, parent) {
+		if (!fdt_node_name_eq(fdt, node, "cpu"))
+			continue;
+
+		if (!fdt_device_is_available(fdt, node))
+			continue;
+
+		if (!early_cpu_isa_ext_available(fdt, node, ext_name))
+			return false;
+
+		ret = true;
+	}
+
+	return ret;
+}
diff --git a/arch/riscv/kernel/pi/pi.h b/arch/riscv/kernel/pi/pi.h
index 493c8cb7c0e6..21141d84fea6 100644
--- a/arch/riscv/kernel/pi/pi.h
+++ b/arch/riscv/kernel/pi/pi.h
@@ -11,7 +11,10 @@
  */
 
 u64 get_kaslr_seed(uintptr_t dtb_pa);
+u64 get_kaslr_seed_zkr(const uintptr_t dtb_pa);
 bool set_nokaslr_from_cmdline(uintptr_t dtb_pa);
 u64 set_satp_mode_from_cmdline(uintptr_t dtb_pa);
 
+bool fdt_early_match_extension_isa(const void *fdt, const char *ext_name);
+
 #endif /* _RISCV_PI_H_ */
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 9940171c79f0..bfb068dc4a64 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -1025,6 +1025,7 @@ static void __init pt_ops_set_late(void)
 #ifdef CONFIG_RANDOMIZE_BASE
 extern bool __init __pi_set_nokaslr_from_cmdline(uintptr_t dtb_pa);
 extern u64 __init __pi_get_kaslr_seed(uintptr_t dtb_pa);
+extern u64 __init __pi_get_kaslr_seed_zkr(const uintptr_t dtb_pa);
 
 static int __init print_nokaslr(char *p)
 {
@@ -1045,10 +1046,12 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
 
 #ifdef CONFIG_RANDOMIZE_BASE
 	if (!__pi_set_nokaslr_from_cmdline(dtb_pa)) {
-		u64 kaslr_seed = __pi_get_kaslr_seed(dtb_pa);
+		u64 kaslr_seed = __pi_get_kaslr_seed_zkr(dtb_pa);
 		u32 kernel_size = (uintptr_t)(&_end) - (uintptr_t)(&_start);
 		u32 nr_pos;
 
+		if (kaslr_seed == 0)
+			kaslr_seed = __pi_get_kaslr_seed(dtb_pa);
 		/*
 		 * Compute the number of positions available: we are limited
 		 * by the early page table that only has one PUD and we must
-- 
2.45.2

Re: [PATCH v3 2/4] RISC-V: lib: Add pi aliases for string functions

[Charlie Jenkins]

On Mon, Jul 01, 2024 at 02:51:30PM -0400, Jesse Taube wrote:
> memset, strcmp, and strncmp are all used in the the __pi_ section.
> add SYM_FUNC_ALIAS for them.
> 
> When KASAN is enabled in <asm/string.h> __pi___memset is also needed.
> 
> Suggested-by: Charlie Jenkins <charlie@rivosinc.com>
> Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> ---
> V2 -> V3:
>  - New patch
> ---
>  arch/riscv/lib/memset.S  | 2 ++
>  arch/riscv/lib/strcmp.S  | 1 +
>  arch/riscv/lib/strncmp.S | 1 +
>  3 files changed, 4 insertions(+)
> 
> diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
> index 35f358e70bdb..da23b8347e2d 100644
> --- a/arch/riscv/lib/memset.S
> +++ b/arch/riscv/lib/memset.S
> @@ -111,3 +111,5 @@ SYM_FUNC_START(__memset)
>  	ret
>  SYM_FUNC_END(__memset)
>  SYM_FUNC_ALIAS_WEAK(memset, __memset)
> +SYM_FUNC_ALIAS(__pi_memset, __memset)
> +SYM_FUNC_ALIAS(__pi___memset, __memset)
> diff --git a/arch/riscv/lib/strcmp.S b/arch/riscv/lib/strcmp.S
> index 687b2bea5c43..bc73325b2fd1 100644
> --- a/arch/riscv/lib/strcmp.S
> +++ b/arch/riscv/lib/strcmp.S
> @@ -120,3 +120,4 @@ strcmp_zbb:
>  .option pop
>  #endif
>  SYM_FUNC_END(strcmp)
> +SYM_FUNC_ALIAS(__pi_strcmp, strcmp)
> diff --git a/arch/riscv/lib/strncmp.S b/arch/riscv/lib/strncmp.S
> index aba5b3148621..b36325a57f6a 100644
> --- a/arch/riscv/lib/strncmp.S
> +++ b/arch/riscv/lib/strncmp.S
> @@ -136,3 +136,4 @@ strncmp_zbb:
>  .option pop
>  #endif
>  SYM_FUNC_END(strncmp)
> +SYM_FUNC_ALIAS(__pi_strncmp, strncmp)
> -- 
> 2.45.2
> 

Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>

Re: [PATCH v3 4/4] RISC-V: Use Zkr to seed KASLR base address

[Charlie Jenkins]

On Mon, Jul 01, 2024 at 02:51:32PM -0400, Jesse Taube wrote:
> Parse the device tree for Zkr in the isa string.
> If Zkr is present, use it to seed the kernel base address.
> 
> On an ACPI system, as of this commit, there is no easy way to check if
> Zkr is present. Blindly running the instruction isn't an option as;
> we have to be able to trust the firmware.
> 
> Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> ---
> V1 -> V2:
>  - Almost entire rewrite
> V2 -> V3:
>  - Dont parse iscv,isa-base
>  - Move fdt_early_match_extension_isa in pi.h under comment
>  - Only check enabled cpus
>  - Rename early_isa_str to fdt_early_match_extension_isa
>  - Rename get_ext_named to early_cpu_isa_ext_available
>  - Rewrite isa_string_contains
>  - Update commit description
>  - Use fdt_stringlist_contains for riscv,isa-extensions
> ---
>  arch/riscv/kernel/pi/Makefile           |   2 +-
>  arch/riscv/kernel/pi/archrandom_early.c |  30 +++++
>  arch/riscv/kernel/pi/fdt_early.c        | 160 ++++++++++++++++++++++++
>  arch/riscv/kernel/pi/pi.h               |   3 +
>  arch/riscv/mm/init.c                    |   5 +-
>  5 files changed, 198 insertions(+), 2 deletions(-)
>  create mode 100644 arch/riscv/kernel/pi/archrandom_early.c
> 
> diff --git a/arch/riscv/kernel/pi/Makefile b/arch/riscv/kernel/pi/Makefile
> index 1ef7584be0c3..dba902f2a538 100644
> --- a/arch/riscv/kernel/pi/Makefile
> +++ b/arch/riscv/kernel/pi/Makefile
> @@ -33,5 +33,5 @@ $(obj)/string.o: $(srctree)/lib/string.c FORCE
>  $(obj)/ctype.o: $(srctree)/lib/ctype.c FORCE
>  	$(call if_changed_rule,cc_o_c)
>  
> -obj-y		:= cmdline_early.pi.o fdt_early.pi.o string.pi.o ctype.pi.o lib-fdt.pi.o lib-fdt_ro.pi.o
> +obj-y		:= cmdline_early.pi.o fdt_early.pi.o string.pi.o ctype.pi.o lib-fdt.pi.o lib-fdt_ro.pi.o archrandom_early.pi.o

When CONFIG_FORTIFY_SOURCE is enabled, the string functions try to
reference `__fortify_panic` which gets aliased to `__pi___fortify_panic`
and causes lld to fail:

ld.lld: error: undefined hidden symbol: __pi___fortify_panic
>>> referenced by fdt_early.c:24 (arch/riscv/kernel/pi/fdt_early.c:24)
>>>               arch/riscv/kernel/pi/fdt_early.pi.o:(__pi_fdt_early_match_extension_isa) in archive vmlinux.a

This early in boot __fortify_panic isn't supported. Can you add
`CFLAGS_fdt_early.o += -D__NO_FORTIFY` next to the same addition for
cmdline_early and fdt_early?

The rest of this looks go to me (just send a new version with this simple fix).

Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>

>  extra-y		:= $(patsubst %.pi.o,%.o,$(obj-y))
> diff --git a/arch/riscv/kernel/pi/archrandom_early.c b/arch/riscv/kernel/pi/archrandom_early.c
> new file mode 100644
> index 000000000000..3f05d3cf3b7b
> --- /dev/null
> +++ b/arch/riscv/kernel/pi/archrandom_early.c
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <asm/csr.h>
> +#include <linux/processor.h>
> +
> +#include "pi.h"
> +
> +/*
> + * To avoid rewriting code include asm/archrandom.h and create macros
> + * for the functions that won't be included.
> + */
> +#undef riscv_has_extension_unlikely
> +#define riscv_has_extension_likely(...) false
> +#undef pr_err_once
> +#define pr_err_once(...)
> +
> +#include <asm/archrandom.h>
> +
> +u64 get_kaslr_seed_zkr(const uintptr_t dtb_pa)
> +{
> +	unsigned long seed = 0;
> +
> +	if (!fdt_early_match_extension_isa((const void *)dtb_pa, "zkr"))
> +		return 0;
> +
> +	if (!csr_seed_long(&seed))
> +		return 0;
> +
> +	return seed;
> +}
> diff --git a/arch/riscv/kernel/pi/fdt_early.c b/arch/riscv/kernel/pi/fdt_early.c
> index 40ee299702bf..49ff5360bf87 100644
> --- a/arch/riscv/kernel/pi/fdt_early.c
> +++ b/arch/riscv/kernel/pi/fdt_early.c
> @@ -2,6 +2,7 @@
>  #include <linux/types.h>
>  #include <linux/init.h>
>  #include <linux/libfdt.h>
> +#include <linux/ctype.h>
>  
>  #include "pi.h"
>  
> @@ -23,3 +24,162 @@ u64 get_kaslr_seed(uintptr_t dtb_pa)
>  	*prop = 0;
>  	return ret;
>  }
> +
> +/**
> + *  fdt_device_is_available - check if a device is available for use
> + *
> + * @fdt: pointer to the device tree blob
> + * @node: offset of the node whose property to find
> + *
> + *  Returns true if the status property is absent or set to "okay" or "ok",
> + *  false otherwise
> + */
> +static bool fdt_device_is_available(const void *fdt, int node)
> +{
> +	const char *status;
> +	int statlen;
> +
> +	status = fdt_getprop(fdt, node, "status", &statlen);
> +	if (!status)
> +		return true;
> +
> +	if (statlen > 0) {
> +		if (!strcmp(status, "okay") || !strcmp(status, "ok"))
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +/* Copy of fdt_nodename_eq_ */
> +static int fdt_node_name_eq(const void *fdt, int offset,
> +			    const char *s)
> +{
> +	int olen;
> +	int len = strlen(s);
> +	const char *p = fdt_get_name(fdt, offset, &olen);
> +
> +	if (!p || olen < len)
> +		/* short match */
> +		return 0;
> +
> +	if (memcmp(p, s, len) != 0)
> +		return 0;
> +
> +	if (p[len] == '\0')
> +		return 1;
> +	else if (!memchr(s, '@', len) && (p[len] == '@'))
> +		return 1;
> +	else
> +		return 0;
> +}
> +
> +/**
> + *  isa_string_contains - check if isa string contains an extension
> + *
> + * @isa_str: isa string to search
> + * @ext_name: the extension to search for
> + *
> + *  Returns true if the extension is in the given isa string,
> + *  false otherwise
> + */
> +static bool isa_string_contains(const char *isa_str, const char *ext_name)
> +{
> +	size_t i, single_end, len = strlen(ext_name);
> +	char ext_end;
> +
> +	/* Error must contain rv32/64 */
> +	if (strlen(isa_str) < 4)
> +		return false;
> +
> +	if (len == 1) {
> +		single_end = strcspn(isa_str, "sSxXzZ");
> +		/* Search for single chars between rv32/64 and multi-letter extensions */
> +		for (i = 4; i < single_end; i++) {
> +			if (tolower(isa_str[i]) == ext_name[0])
> +				return true;
> +		}
> +		return false;
> +	}
> +
> +	/* Skip to start of multi-letter extensions */
> +	isa_str = strpbrk(isa_str, "sSxXzZ");
> +	while (isa_str) {
> +		if (strncasecmp(isa_str, ext_name, len) == 0) {
> +			ext_end = isa_str[len];
> +			/* Check if matches the whole extension excluding version. */
> +			if (ext_end == '\0' || ext_end == '_' || isdigit(ext_end))
> +				return true;
> +		}
> +		/* Multi-letter extensions must be split from other multi-letter
> +		 * extensions with an "_", the end of a multi-letter extension will
> +		 * either be the null character or the "_" at the start of the next
> +		 * multi-letter extension.
> +		 */
> +		isa_str = strchr(isa_str, '_');
> +		if (isa_str)
> +			isa_str++;
> +	}
> +
> +	return false;
> +}
> +
> +/**
> + *  early_cpu_isa_ext_available - check if cpu node has an extension
> + *
> + * @fdt: pointer to the device tree blob
> + * @node: offset of the cpu node
> + * @ext_name: the extension to search for
> + *
> + *  Returns true if the cpu node has the extension,
> + *  false otherwise
> + */
> +static bool early_cpu_isa_ext_available(const void *fdt, int node, const char *ext_name)
> +{
> +	const void *prop;
> +	int len;
> +
> +	prop = fdt_getprop(fdt, node, "riscv,isa-extensions", &len);
> +	if (prop && fdt_stringlist_contains(prop, len, ext_name))
> +		return true;
> +
> +	prop = fdt_getprop(fdt, node, "riscv,isa", &len);
> +	if (prop && isa_string_contains(prop, ext_name))
> +		return true;
> +
> +	return false;
> +}
> +
> +/**
> + *  fdt_early_match_extension_isa - check if all cpu nodes have an extension
> + *
> + * @fdt: pointer to the device tree blob
> + * @ext_name: the extension to search for
> + *
> + *  Returns true if the all available the cpu nodes have the extension,
> + *  false otherwise
> + */
> +bool fdt_early_match_extension_isa(const void *fdt, const char *ext_name)
> +{
> +	int node, parent;
> +	bool ret = false;
> +
> +	parent = fdt_path_offset(fdt, "/cpus");
> +	if (parent < 0)
> +		return false;
> +
> +	fdt_for_each_subnode(node, fdt, parent) {
> +		if (!fdt_node_name_eq(fdt, node, "cpu"))
> +			continue;
> +
> +		if (!fdt_device_is_available(fdt, node))
> +			continue;
> +
> +		if (!early_cpu_isa_ext_available(fdt, node, ext_name))
> +			return false;
> +
> +		ret = true;
> +	}
> +
> +	return ret;
> +}
> diff --git a/arch/riscv/kernel/pi/pi.h b/arch/riscv/kernel/pi/pi.h
> index 493c8cb7c0e6..21141d84fea6 100644
> --- a/arch/riscv/kernel/pi/pi.h
> +++ b/arch/riscv/kernel/pi/pi.h
> @@ -11,7 +11,10 @@
>   */
>  
>  u64 get_kaslr_seed(uintptr_t dtb_pa);
> +u64 get_kaslr_seed_zkr(const uintptr_t dtb_pa);
>  bool set_nokaslr_from_cmdline(uintptr_t dtb_pa);
>  u64 set_satp_mode_from_cmdline(uintptr_t dtb_pa);
>  
> +bool fdt_early_match_extension_isa(const void *fdt, const char *ext_name);
> +
>  #endif /* _RISCV_PI_H_ */
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 9940171c79f0..bfb068dc4a64 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -1025,6 +1025,7 @@ static void __init pt_ops_set_late(void)
>  #ifdef CONFIG_RANDOMIZE_BASE
>  extern bool __init __pi_set_nokaslr_from_cmdline(uintptr_t dtb_pa);
>  extern u64 __init __pi_get_kaslr_seed(uintptr_t dtb_pa);
> +extern u64 __init __pi_get_kaslr_seed_zkr(const uintptr_t dtb_pa);
>  
>  static int __init print_nokaslr(char *p)
>  {
> @@ -1045,10 +1046,12 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>  
>  #ifdef CONFIG_RANDOMIZE_BASE
>  	if (!__pi_set_nokaslr_from_cmdline(dtb_pa)) {
> -		u64 kaslr_seed = __pi_get_kaslr_seed(dtb_pa);
> +		u64 kaslr_seed = __pi_get_kaslr_seed_zkr(dtb_pa);
>  		u32 kernel_size = (uintptr_t)(&_end) - (uintptr_t)(&_start);
>  		u32 nr_pos;
>  
> +		if (kaslr_seed == 0)
> +			kaslr_seed = __pi_get_kaslr_seed(dtb_pa);
>  		/*
>  		 * Compute the number of positions available: we are limited
>  		 * by the early page table that only has one PUD and we must
> -- 
> 2.45.2
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v3 4/4] RISC-V: Use Zkr to seed KASLR base address

[Alexandre Ghiti]

Hi Jesse,

On Tue, Jul 2, 2024 at 6:30 AM Charlie Jenkins <charlie@rivosinc.com> wrote:
>
> On Mon, Jul 01, 2024 at 02:51:32PM -0400, Jesse Taube wrote:
> > Parse the device tree for Zkr in the isa string.
> > If Zkr is present, use it to seed the kernel base address.
> >
> > On an ACPI system, as of this commit, there is no easy way to check if
> > Zkr is present. Blindly running the instruction isn't an option as;
> > we have to be able to trust the firmware.
> >
> > Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> > ---
> > V1 -> V2:
> >  - Almost entire rewrite
> > V2 -> V3:
> >  - Dont parse iscv,isa-base
> >  - Move fdt_early_match_extension_isa in pi.h under comment
> >  - Only check enabled cpus
> >  - Rename early_isa_str to fdt_early_match_extension_isa
> >  - Rename get_ext_named to early_cpu_isa_ext_available
> >  - Rewrite isa_string_contains
> >  - Update commit description
> >  - Use fdt_stringlist_contains for riscv,isa-extensions
> > ---
> >  arch/riscv/kernel/pi/Makefile           |   2 +-
> >  arch/riscv/kernel/pi/archrandom_early.c |  30 +++++
> >  arch/riscv/kernel/pi/fdt_early.c        | 160 ++++++++++++++++++++++++
> >  arch/riscv/kernel/pi/pi.h               |   3 +
> >  arch/riscv/mm/init.c                    |   5 +-
> >  5 files changed, 198 insertions(+), 2 deletions(-)
> >  create mode 100644 arch/riscv/kernel/pi/archrandom_early.c
> >
> > diff --git a/arch/riscv/kernel/pi/Makefile b/arch/riscv/kernel/pi/Makefile
> > index 1ef7584be0c3..dba902f2a538 100644
> > --- a/arch/riscv/kernel/pi/Makefile
> > +++ b/arch/riscv/kernel/pi/Makefile
> > @@ -33,5 +33,5 @@ $(obj)/string.o: $(srctree)/lib/string.c FORCE
> >  $(obj)/ctype.o: $(srctree)/lib/ctype.c FORCE
> >       $(call if_changed_rule,cc_o_c)
> >
> > -obj-y                := cmdline_early.pi.o fdt_early.pi.o string.pi.o ctype.pi.o lib-fdt.pi.o lib-fdt_ro.pi.o
> > +obj-y                := cmdline_early.pi.o fdt_early.pi.o string.pi.o ctype.pi.o lib-fdt.pi.o lib-fdt_ro.pi.o archrandom_early.pi.o
>
> When CONFIG_FORTIFY_SOURCE is enabled, the string functions try to
> reference `__fortify_panic` which gets aliased to `__pi___fortify_panic`
> and causes lld to fail:
>
> ld.lld: error: undefined hidden symbol: __pi___fortify_panic
> >>> referenced by fdt_early.c:24 (arch/riscv/kernel/pi/fdt_early.c:24)
> >>>               arch/riscv/kernel/pi/fdt_early.pi.o:(__pi_fdt_early_match_extension_isa) in archive vmlinux.a
>
> This early in boot __fortify_panic isn't supported. Can you add
> `CFLAGS_fdt_early.o += -D__NO_FORTIFY` next to the same addition for
> cmdline_early and fdt_early?

With this ^ fixed, you can add:

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex

>
> The rest of this looks go to me (just send a new version with this simple fix).
>
> Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
>
> >  extra-y              := $(patsubst %.pi.o,%.o,$(obj-y))
> > diff --git a/arch/riscv/kernel/pi/archrandom_early.c b/arch/riscv/kernel/pi/archrandom_early.c
> > new file mode 100644
> > index 000000000000..3f05d3cf3b7b
> > --- /dev/null
> > +++ b/arch/riscv/kernel/pi/archrandom_early.c
> > @@ -0,0 +1,30 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#include <asm/csr.h>
> > +#include <linux/processor.h>
> > +
> > +#include "pi.h"
> > +
> > +/*
> > + * To avoid rewriting code include asm/archrandom.h and create macros
> > + * for the functions that won't be included.
> > + */
> > +#undef riscv_has_extension_unlikely
> > +#define riscv_has_extension_likely(...) false
> > +#undef pr_err_once
> > +#define pr_err_once(...)
> > +
> > +#include <asm/archrandom.h>
> > +
> > +u64 get_kaslr_seed_zkr(const uintptr_t dtb_pa)
> > +{
> > +     unsigned long seed = 0;
> > +
> > +     if (!fdt_early_match_extension_isa((const void *)dtb_pa, "zkr"))
> > +             return 0;
> > +
> > +     if (!csr_seed_long(&seed))
> > +             return 0;
> > +
> > +     return seed;
> > +}
> > diff --git a/arch/riscv/kernel/pi/fdt_early.c b/arch/riscv/kernel/pi/fdt_early.c
> > index 40ee299702bf..49ff5360bf87 100644
> > --- a/arch/riscv/kernel/pi/fdt_early.c
> > +++ b/arch/riscv/kernel/pi/fdt_early.c
> > @@ -2,6 +2,7 @@
> >  #include <linux/types.h>
> >  #include <linux/init.h>
> >  #include <linux/libfdt.h>
> > +#include <linux/ctype.h>
> >
> >  #include "pi.h"
> >
> > @@ -23,3 +24,162 @@ u64 get_kaslr_seed(uintptr_t dtb_pa)
> >       *prop = 0;
> >       return ret;
> >  }
> > +
> > +/**
> > + *  fdt_device_is_available - check if a device is available for use
> > + *
> > + * @fdt: pointer to the device tree blob
> > + * @node: offset of the node whose property to find
> > + *
> > + *  Returns true if the status property is absent or set to "okay" or "ok",
> > + *  false otherwise
> > + */
> > +static bool fdt_device_is_available(const void *fdt, int node)
> > +{
> > +     const char *status;
> > +     int statlen;
> > +
> > +     status = fdt_getprop(fdt, node, "status", &statlen);
> > +     if (!status)
> > +             return true;
> > +
> > +     if (statlen > 0) {
> > +             if (!strcmp(status, "okay") || !strcmp(status, "ok"))
> > +                     return true;
> > +     }
> > +
> > +     return false;
> > +}
> > +
> > +/* Copy of fdt_nodename_eq_ */
> > +static int fdt_node_name_eq(const void *fdt, int offset,
> > +                         const char *s)
> > +{
> > +     int olen;
> > +     int len = strlen(s);
> > +     const char *p = fdt_get_name(fdt, offset, &olen);
> > +
> > +     if (!p || olen < len)
> > +             /* short match */
> > +             return 0;
> > +
> > +     if (memcmp(p, s, len) != 0)
> > +             return 0;
> > +
> > +     if (p[len] == '\0')
> > +             return 1;
> > +     else if (!memchr(s, '@', len) && (p[len] == '@'))
> > +             return 1;
> > +     else
> > +             return 0;
> > +}
> > +
> > +/**
> > + *  isa_string_contains - check if isa string contains an extension
> > + *
> > + * @isa_str: isa string to search
> > + * @ext_name: the extension to search for
> > + *
> > + *  Returns true if the extension is in the given isa string,
> > + *  false otherwise
> > + */
> > +static bool isa_string_contains(const char *isa_str, const char *ext_name)
> > +{
> > +     size_t i, single_end, len = strlen(ext_name);
> > +     char ext_end;
> > +
> > +     /* Error must contain rv32/64 */
> > +     if (strlen(isa_str) < 4)
> > +             return false;
> > +
> > +     if (len == 1) {
> > +             single_end = strcspn(isa_str, "sSxXzZ");
> > +             /* Search for single chars between rv32/64 and multi-letter extensions */
> > +             for (i = 4; i < single_end; i++) {
> > +                     if (tolower(isa_str[i]) == ext_name[0])
> > +                             return true;
> > +             }
> > +             return false;
> > +     }
> > +
> > +     /* Skip to start of multi-letter extensions */
> > +     isa_str = strpbrk(isa_str, "sSxXzZ");
> > +     while (isa_str) {
> > +             if (strncasecmp(isa_str, ext_name, len) == 0) {
> > +                     ext_end = isa_str[len];
> > +                     /* Check if matches the whole extension excluding version. */
> > +                     if (ext_end == '\0' || ext_end == '_' || isdigit(ext_end))
> > +                             return true;
> > +             }
> > +             /* Multi-letter extensions must be split from other multi-letter
> > +              * extensions with an "_", the end of a multi-letter extension will
> > +              * either be the null character or the "_" at the start of the next
> > +              * multi-letter extension.
> > +              */
> > +             isa_str = strchr(isa_str, '_');
> > +             if (isa_str)
> > +                     isa_str++;
> > +     }
> > +
> > +     return false;
> > +}
> > +
> > +/**
> > + *  early_cpu_isa_ext_available - check if cpu node has an extension
> > + *
> > + * @fdt: pointer to the device tree blob
> > + * @node: offset of the cpu node
> > + * @ext_name: the extension to search for
> > + *
> > + *  Returns true if the cpu node has the extension,
> > + *  false otherwise
> > + */
> > +static bool early_cpu_isa_ext_available(const void *fdt, int node, const char *ext_name)
> > +{
> > +     const void *prop;
> > +     int len;
> > +
> > +     prop = fdt_getprop(fdt, node, "riscv,isa-extensions", &len);
> > +     if (prop && fdt_stringlist_contains(prop, len, ext_name))
> > +             return true;
> > +
> > +     prop = fdt_getprop(fdt, node, "riscv,isa", &len);
> > +     if (prop && isa_string_contains(prop, ext_name))
> > +             return true;
> > +
> > +     return false;
> > +}
> > +
> > +/**
> > + *  fdt_early_match_extension_isa - check if all cpu nodes have an extension
> > + *
> > + * @fdt: pointer to the device tree blob
> > + * @ext_name: the extension to search for
> > + *
> > + *  Returns true if the all available the cpu nodes have the extension,
> > + *  false otherwise
> > + */
> > +bool fdt_early_match_extension_isa(const void *fdt, const char *ext_name)
> > +{
> > +     int node, parent;
> > +     bool ret = false;
> > +
> > +     parent = fdt_path_offset(fdt, "/cpus");
> > +     if (parent < 0)
> > +             return false;
> > +
> > +     fdt_for_each_subnode(node, fdt, parent) {
> > +             if (!fdt_node_name_eq(fdt, node, "cpu"))
> > +                     continue;
> > +
> > +             if (!fdt_device_is_available(fdt, node))
> > +                     continue;
> > +
> > +             if (!early_cpu_isa_ext_available(fdt, node, ext_name))
> > +                     return false;
> > +
> > +             ret = true;
> > +     }
> > +
> > +     return ret;
> > +}
> > diff --git a/arch/riscv/kernel/pi/pi.h b/arch/riscv/kernel/pi/pi.h
> > index 493c8cb7c0e6..21141d84fea6 100644
> > --- a/arch/riscv/kernel/pi/pi.h
> > +++ b/arch/riscv/kernel/pi/pi.h
> > @@ -11,7 +11,10 @@
> >   */
> >
> >  u64 get_kaslr_seed(uintptr_t dtb_pa);
> > +u64 get_kaslr_seed_zkr(const uintptr_t dtb_pa);
> >  bool set_nokaslr_from_cmdline(uintptr_t dtb_pa);
> >  u64 set_satp_mode_from_cmdline(uintptr_t dtb_pa);
> >
> > +bool fdt_early_match_extension_isa(const void *fdt, const char *ext_name);
> > +
> >  #endif /* _RISCV_PI_H_ */
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 9940171c79f0..bfb068dc4a64 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -1025,6 +1025,7 @@ static void __init pt_ops_set_late(void)
> >  #ifdef CONFIG_RANDOMIZE_BASE
> >  extern bool __init __pi_set_nokaslr_from_cmdline(uintptr_t dtb_pa);
> >  extern u64 __init __pi_get_kaslr_seed(uintptr_t dtb_pa);
> > +extern u64 __init __pi_get_kaslr_seed_zkr(const uintptr_t dtb_pa);
> >
> >  static int __init print_nokaslr(char *p)
> >  {
> > @@ -1045,10 +1046,12 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> >
> >  #ifdef CONFIG_RANDOMIZE_BASE
> >       if (!__pi_set_nokaslr_from_cmdline(dtb_pa)) {
> > -             u64 kaslr_seed = __pi_get_kaslr_seed(dtb_pa);
> > +             u64 kaslr_seed = __pi_get_kaslr_seed_zkr(dtb_pa);
> >               u32 kernel_size = (uintptr_t)(&_end) - (uintptr_t)(&_start);
> >               u32 nr_pos;
> >
> > +             if (kaslr_seed == 0)
> > +                     kaslr_seed = __pi_get_kaslr_seed(dtb_pa);
> >               /*
> >                * Compute the number of positions available: we are limited
> >                * by the early page table that only has one PUD and we must
> > --
> > 2.45.2
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v3 2/4] RISC-V: lib: Add pi aliases for string functions

[Alexandre Ghiti]

On Tue, Jul 2, 2024 at 5:02 AM Charlie Jenkins <charlie@rivosinc.com> wrote:
>
> On Mon, Jul 01, 2024 at 02:51:30PM -0400, Jesse Taube wrote:
> > memset, strcmp, and strncmp are all used in the the __pi_ section.
> > add SYM_FUNC_ALIAS for them.
> >
> > When KASAN is enabled in <asm/string.h> __pi___memset is also needed.
> >
> > Suggested-by: Charlie Jenkins <charlie@rivosinc.com>
> > Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> > ---
> > V2 -> V3:
> >  - New patch
> > ---
> >  arch/riscv/lib/memset.S  | 2 ++
> >  arch/riscv/lib/strcmp.S  | 1 +
> >  arch/riscv/lib/strncmp.S | 1 +
> >  3 files changed, 4 insertions(+)
> >
> > diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
> > index 35f358e70bdb..da23b8347e2d 100644
> > --- a/arch/riscv/lib/memset.S
> > +++ b/arch/riscv/lib/memset.S
> > @@ -111,3 +111,5 @@ SYM_FUNC_START(__memset)
> >       ret
> >  SYM_FUNC_END(__memset)
> >  SYM_FUNC_ALIAS_WEAK(memset, __memset)
> > +SYM_FUNC_ALIAS(__pi_memset, __memset)
> > +SYM_FUNC_ALIAS(__pi___memset, __memset)
> > diff --git a/arch/riscv/lib/strcmp.S b/arch/riscv/lib/strcmp.S
> > index 687b2bea5c43..bc73325b2fd1 100644
> > --- a/arch/riscv/lib/strcmp.S
> > +++ b/arch/riscv/lib/strcmp.S
> > @@ -120,3 +120,4 @@ strcmp_zbb:
> >  .option pop
> >  #endif
> >  SYM_FUNC_END(strcmp)
> > +SYM_FUNC_ALIAS(__pi_strcmp, strcmp)
> > diff --git a/arch/riscv/lib/strncmp.S b/arch/riscv/lib/strncmp.S
> > index aba5b3148621..b36325a57f6a 100644
> > --- a/arch/riscv/lib/strncmp.S
> > +++ b/arch/riscv/lib/strncmp.S
> > @@ -136,3 +136,4 @@ strncmp_zbb:
> >  .option pop
> >  #endif
> >  SYM_FUNC_END(strncmp)
> > +SYM_FUNC_ALIAS(__pi_strncmp, strncmp)
> > --
> > 2.45.2
> >
>
> Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
>

You can add:

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex

Re: [PATCH v3 4/4] RISC-V: Use Zkr to seed KASLR base address

[Conor Dooley]

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

On Mon, Jul 01, 2024 at 02:51:32PM -0400, Jesse Taube wrote:

> +/**
> + *  isa_string_contains - check if isa string contains an extension
> + *
> + * @isa_str: isa string to search
> + * @ext_name: the extension to search for
> + *
> + *  Returns true if the extension is in the given isa string,
> + *  false otherwise
> + */
> +static bool isa_string_contains(const char *isa_str, const char *ext_name)
> +{
> +	size_t i, single_end, len = strlen(ext_name);
> +	char ext_end;
> +
> +	/* Error must contain rv32/64 */
> +	if (strlen(isa_str) < 4)
> +		return false;
> +
> +	if (len == 1) {
> +		single_end = strcspn(isa_str, "sSxXzZ");
> +		/* Search for single chars between rv32/64 and multi-letter extensions */
> +		for (i = 4; i < single_end; i++) {
> +			if (tolower(isa_str[i]) == ext_name[0])
> +				return true;
> +		}
> +		return false;
> +	}
> +
> +	/* Skip to start of multi-letter extensions */
> +	isa_str = strpbrk(isa_str, "sSxXzZ");

Technically this could break with the old QEMUs that had "su" in the
single letter part, but at this point I think enough time has passed
that it does not matter.

> +	while (isa_str) {
> +		if (strncasecmp(isa_str, ext_name, len) == 0) {
> +			ext_end = isa_str[len];
> +			/* Check if matches the whole extension excluding version. */
> +			if (ext_end == '\0' || ext_end == '_' || isdigit(ext_end))

I'm also not entirely sure about the final clause here. If you have an
extension "foo" and "foo32b", you'd match on the latter, right? I don't
think any extensions like that at the minute (foo32b type stuff does,
but the "root" alphabetical part for a foo32b doesn't), but I also don't
wanna have to chase down bugs in a parser this early in the future! The
devicetree binding doesn't actually allow anyone to put version
information in the isa string, so maybe the thing to do is just drop the
isdigit() check, given we only support devicetree right now for this early
probing?

> +				return true;
> +		}
> +		/* Multi-letter extensions must be split from other multi-letter
> +		 * extensions with an "_", the end of a multi-letter extension will
> +		 * either be the null character or the "_" at the start of the next
> +		 * multi-letter extension.
> +		 */
> +		isa_str = strchr(isa_str, '_');
> +		if (isa_str)
> +			isa_str++;
> +	}
> +
> +	return false;
> +}

Otherwise, I think this is fine now. Thanks for the updates.

Cheers,
Conor.

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

Re: [PATCH v3 4/4] RISC-V: Use Zkr to seed KASLR base address

[kernel test robot]

Hi Jesse,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on linus/master v6.10-rc6 next-20240702]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jesse-Taube/RISC-V-pi-Force-hidden-visibility-for-all-symbol-references/20240702-025515
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20240701185132.319995-5-jesse%40rivosinc.com
patch subject: [PATCH v3 4/4] RISC-V: Use Zkr to seed KASLR base address
config: riscv-randconfig-001-20240703 (https://download.01.org/0day-ci/archive/20240703/202407030941.LFCgSw4G-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 326ba38a991250a8587a399a260b0f7af2c9166a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240703/202407030941.LFCgSw4G-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407030941.LFCgSw4G-lkp@intel.com/

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined hidden symbol: __pi___fortify_panic
   >>> referenced by fdt_early.c:24 (arch/riscv/kernel/pi/fdt_early.c:24)
   >>>               arch/riscv/kernel/pi/fdt_early.pi.o:(__pi_fdt_early_match_extension_isa) in archive vmlinux.a

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3 4/4] RISC-V: Use Zkr to seed KASLR base address

[Zong Li]

On Tue, Jul 2, 2024 at 12:31 PM Charlie Jenkins <charlie@rivosinc.com> wrote:
>
> On Mon, Jul 01, 2024 at 02:51:32PM -0400, Jesse Taube wrote:
> > Parse the device tree for Zkr in the isa string.
> > If Zkr is present, use it to seed the kernel base address.
> >
> > On an ACPI system, as of this commit, there is no easy way to check if
> > Zkr is present. Blindly running the instruction isn't an option as;
> > we have to be able to trust the firmware.
> >
> > Signed-off-by: Jesse Taube <jesse@rivosinc.com>

I have tried this patch, it works to me.

Tested-by: Zong Li <zong.li@sifive.com>

> > ---
> > V1 -> V2:
> >  - Almost entire rewrite
> > V2 -> V3:
> >  - Dont parse iscv,isa-base
> >  - Move fdt_early_match_extension_isa in pi.h under comment
> >  - Only check enabled cpus
> >  - Rename early_isa_str to fdt_early_match_extension_isa
> >  - Rename get_ext_named to early_cpu_isa_ext_available
> >  - Rewrite isa_string_contains
> >  - Update commit description
> >  - Use fdt_stringlist_contains for riscv,isa-extensions
> > ---
> >  arch/riscv/kernel/pi/Makefile           |   2 +-
> >  arch/riscv/kernel/pi/archrandom_early.c |  30 +++++
> >  arch/riscv/kernel/pi/fdt_early.c        | 160 ++++++++++++++++++++++++
> >  arch/riscv/kernel/pi/pi.h               |   3 +
> >  arch/riscv/mm/init.c                    |   5 +-
> >  5 files changed, 198 insertions(+), 2 deletions(-)
> >  create mode 100644 arch/riscv/kernel/pi/archrandom_early.c
> >
> > diff --git a/arch/riscv/kernel/pi/Makefile b/arch/riscv/kernel/pi/Makefile
> > index 1ef7584be0c3..dba902f2a538 100644
> > --- a/arch/riscv/kernel/pi/Makefile
> > +++ b/arch/riscv/kernel/pi/Makefile
> > @@ -33,5 +33,5 @@ $(obj)/string.o: $(srctree)/lib/string.c FORCE
> >  $(obj)/ctype.o: $(srctree)/lib/ctype.c FORCE
> >       $(call if_changed_rule,cc_o_c)
> >
> > -obj-y                := cmdline_early.pi.o fdt_early.pi.o string.pi.o ctype.pi.o lib-fdt.pi.o lib-fdt_ro.pi.o
> > +obj-y                := cmdline_early.pi.o fdt_early.pi.o string.pi.o ctype.pi.o lib-fdt.pi.o lib-fdt_ro.pi.o archrandom_early.pi.o
>
> When CONFIG_FORTIFY_SOURCE is enabled, the string functions try to
> reference `__fortify_panic` which gets aliased to `__pi___fortify_panic`
> and causes lld to fail:
>
> ld.lld: error: undefined hidden symbol: __pi___fortify_panic
> >>> referenced by fdt_early.c:24 (arch/riscv/kernel/pi/fdt_early.c:24)
> >>>               arch/riscv/kernel/pi/fdt_early.pi.o:(__pi_fdt_early_match_extension_isa) in archive vmlinux.a
>
> This early in boot __fortify_panic isn't supported. Can you add
> `CFLAGS_fdt_early.o += -D__NO_FORTIFY` next to the same addition for
> cmdline_early and fdt_early?
>
> The rest of this looks go to me (just send a new version with this simple fix).
>
> Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
>
> >  extra-y              := $(patsubst %.pi.o,%.o,$(obj-y))
> > diff --git a/arch/riscv/kernel/pi/archrandom_early.c b/arch/riscv/kernel/pi/archrandom_early.c
> > new file mode 100644
> > index 000000000000..3f05d3cf3b7b
> > --- /dev/null
> > +++ b/arch/riscv/kernel/pi/archrandom_early.c
> > @@ -0,0 +1,30 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#include <asm/csr.h>
> > +#include <linux/processor.h>
> > +
> > +#include "pi.h"
> > +
> > +/*
> > + * To avoid rewriting code include asm/archrandom.h and create macros
> > + * for the functions that won't be included.
> > + */
> > +#undef riscv_has_extension_unlikely
> > +#define riscv_has_extension_likely(...) false
> > +#undef pr_err_once
> > +#define pr_err_once(...)
> > +
> > +#include <asm/archrandom.h>
> > +
> > +u64 get_kaslr_seed_zkr(const uintptr_t dtb_pa)
> > +{
> > +     unsigned long seed = 0;
> > +
> > +     if (!fdt_early_match_extension_isa((const void *)dtb_pa, "zkr"))
> > +             return 0;
> > +
> > +     if (!csr_seed_long(&seed))
> > +             return 0;
> > +
> > +     return seed;
> > +}
> > diff --git a/arch/riscv/kernel/pi/fdt_early.c b/arch/riscv/kernel/pi/fdt_early.c
> > index 40ee299702bf..49ff5360bf87 100644
> > --- a/arch/riscv/kernel/pi/fdt_early.c
> > +++ b/arch/riscv/kernel/pi/fdt_early.c
> > @@ -2,6 +2,7 @@
> >  #include <linux/types.h>
> >  #include <linux/init.h>
> >  #include <linux/libfdt.h>
> > +#include <linux/ctype.h>
> >
> >  #include "pi.h"
> >
> > @@ -23,3 +24,162 @@ u64 get_kaslr_seed(uintptr_t dtb_pa)
> >       *prop = 0;
> >       return ret;
> >  }
> > +
> > +/**
> > + *  fdt_device_is_available - check if a device is available for use
> > + *
> > + * @fdt: pointer to the device tree blob
> > + * @node: offset of the node whose property to find
> > + *
> > + *  Returns true if the status property is absent or set to "okay" or "ok",
> > + *  false otherwise
> > + */
> > +static bool fdt_device_is_available(const void *fdt, int node)
> > +{
> > +     const char *status;
> > +     int statlen;
> > +
> > +     status = fdt_getprop(fdt, node, "status", &statlen);
> > +     if (!status)
> > +             return true;
> > +
> > +     if (statlen > 0) {
> > +             if (!strcmp(status, "okay") || !strcmp(status, "ok"))
> > +                     return true;
> > +     }
> > +
> > +     return false;
> > +}
> > +
> > +/* Copy of fdt_nodename_eq_ */
> > +static int fdt_node_name_eq(const void *fdt, int offset,
> > +                         const char *s)
> > +{
> > +     int olen;
> > +     int len = strlen(s);
> > +     const char *p = fdt_get_name(fdt, offset, &olen);
> > +
> > +     if (!p || olen < len)
> > +             /* short match */
> > +             return 0;
> > +
> > +     if (memcmp(p, s, len) != 0)
> > +             return 0;
> > +
> > +     if (p[len] == '\0')
> > +             return 1;
> > +     else if (!memchr(s, '@', len) && (p[len] == '@'))
> > +             return 1;
> > +     else
> > +             return 0;
> > +}
> > +
> > +/**
> > + *  isa_string_contains - check if isa string contains an extension
> > + *
> > + * @isa_str: isa string to search
> > + * @ext_name: the extension to search for
> > + *
> > + *  Returns true if the extension is in the given isa string,
> > + *  false otherwise
> > + */
> > +static bool isa_string_contains(const char *isa_str, const char *ext_name)
> > +{
> > +     size_t i, single_end, len = strlen(ext_name);
> > +     char ext_end;
> > +
> > +     /* Error must contain rv32/64 */
> > +     if (strlen(isa_str) < 4)
> > +             return false;
> > +
> > +     if (len == 1) {
> > +             single_end = strcspn(isa_str, "sSxXzZ");
> > +             /* Search for single chars between rv32/64 and multi-letter extensions */
> > +             for (i = 4; i < single_end; i++) {
> > +                     if (tolower(isa_str[i]) == ext_name[0])
> > +                             return true;
> > +             }
> > +             return false;
> > +     }
> > +
> > +     /* Skip to start of multi-letter extensions */
> > +     isa_str = strpbrk(isa_str, "sSxXzZ");
> > +     while (isa_str) {
> > +             if (strncasecmp(isa_str, ext_name, len) == 0) {
> > +                     ext_end = isa_str[len];
> > +                     /* Check if matches the whole extension excluding version. */
> > +                     if (ext_end == '\0' || ext_end == '_' || isdigit(ext_end))
> > +                             return true;
> > +             }
> > +             /* Multi-letter extensions must be split from other multi-letter
> > +              * extensions with an "_", the end of a multi-letter extension will
> > +              * either be the null character or the "_" at the start of the next
> > +              * multi-letter extension.
> > +              */
> > +             isa_str = strchr(isa_str, '_');
> > +             if (isa_str)
> > +                     isa_str++;
> > +     }
> > +
> > +     return false;
> > +}
> > +
> > +/**
> > + *  early_cpu_isa_ext_available - check if cpu node has an extension
> > + *
> > + * @fdt: pointer to the device tree blob
> > + * @node: offset of the cpu node
> > + * @ext_name: the extension to search for
> > + *
> > + *  Returns true if the cpu node has the extension,
> > + *  false otherwise
> > + */
> > +static bool early_cpu_isa_ext_available(const void *fdt, int node, const char *ext_name)
> > +{
> > +     const void *prop;
> > +     int len;
> > +
> > +     prop = fdt_getprop(fdt, node, "riscv,isa-extensions", &len);
> > +     if (prop && fdt_stringlist_contains(prop, len, ext_name))
> > +             return true;
> > +
> > +     prop = fdt_getprop(fdt, node, "riscv,isa", &len);
> > +     if (prop && isa_string_contains(prop, ext_name))
> > +             return true;
> > +
> > +     return false;
> > +}
> > +
> > +/**
> > + *  fdt_early_match_extension_isa - check if all cpu nodes have an extension
> > + *
> > + * @fdt: pointer to the device tree blob
> > + * @ext_name: the extension to search for
> > + *
> > + *  Returns true if the all available the cpu nodes have the extension,
> > + *  false otherwise
> > + */
> > +bool fdt_early_match_extension_isa(const void *fdt, const char *ext_name)
> > +{
> > +     int node, parent;
> > +     bool ret = false;
> > +
> > +     parent = fdt_path_offset(fdt, "/cpus");
> > +     if (parent < 0)
> > +             return false;
> > +
> > +     fdt_for_each_subnode(node, fdt, parent) {
> > +             if (!fdt_node_name_eq(fdt, node, "cpu"))
> > +                     continue;
> > +
> > +             if (!fdt_device_is_available(fdt, node))
> > +                     continue;
> > +
> > +             if (!early_cpu_isa_ext_available(fdt, node, ext_name))
> > +                     return false;
> > +
> > +             ret = true;
> > +     }
> > +
> > +     return ret;
> > +}
> > diff --git a/arch/riscv/kernel/pi/pi.h b/arch/riscv/kernel/pi/pi.h
> > index 493c8cb7c0e6..21141d84fea6 100644
> > --- a/arch/riscv/kernel/pi/pi.h
> > +++ b/arch/riscv/kernel/pi/pi.h
> > @@ -11,7 +11,10 @@
> >   */
> >
> >  u64 get_kaslr_seed(uintptr_t dtb_pa);
> > +u64 get_kaslr_seed_zkr(const uintptr_t dtb_pa);
> >  bool set_nokaslr_from_cmdline(uintptr_t dtb_pa);
> >  u64 set_satp_mode_from_cmdline(uintptr_t dtb_pa);
> >
> > +bool fdt_early_match_extension_isa(const void *fdt, const char *ext_name);
> > +
> >  #endif /* _RISCV_PI_H_ */
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 9940171c79f0..bfb068dc4a64 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -1025,6 +1025,7 @@ static void __init pt_ops_set_late(void)
> >  #ifdef CONFIG_RANDOMIZE_BASE
> >  extern bool __init __pi_set_nokaslr_from_cmdline(uintptr_t dtb_pa);
> >  extern u64 __init __pi_get_kaslr_seed(uintptr_t dtb_pa);
> > +extern u64 __init __pi_get_kaslr_seed_zkr(const uintptr_t dtb_pa);
> >
> >  static int __init print_nokaslr(char *p)
> >  {
> > @@ -1045,10 +1046,12 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> >
> >  #ifdef CONFIG_RANDOMIZE_BASE
> >       if (!__pi_set_nokaslr_from_cmdline(dtb_pa)) {
> > -             u64 kaslr_seed = __pi_get_kaslr_seed(dtb_pa);
> > +             u64 kaslr_seed = __pi_get_kaslr_seed_zkr(dtb_pa);
> >               u32 kernel_size = (uintptr_t)(&_end) - (uintptr_t)(&_start);
> >               u32 nr_pos;
> >
> > +             if (kaslr_seed == 0)
> > +                     kaslr_seed = __pi_get_kaslr_seed(dtb_pa);
> >               /*
> >                * Compute the number of positions available: we are limited
> >                * by the early page table that only has one PUD and we must
> > --
> > 2.45.2
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv