Kunit to check the longest symbol length

PATCH Superseded Sergio González Collado 2 participants 4 messages Feb 2, 2025 – Feb 19, 2025

Subsystems (4)

x86 Kselftest Library code Rust

lore

Share & tools

lore

Sergio González Collado

author

2025-02-02 19:24 UTC #0 lore raw

PATCH v8 1/1 Kunit to check the longest symbol length

The longest length of a symbol (KSYM_NAME_LEN) was increased to 512 in the reference [1]. This patch adds kunit test suite to check the longest symbol length. These tests verify that the longest symbol length defined is supported.

This test can also help other efforts for longer symbol length, like [2].

The test suite defines one symbol with the longest possible length.

The first test verify that functions with names of the created symbol, can be called or not.

The second test, verify that the symbols are created (or not) in the kernel symbol table.

[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/

Tested-by Martin Rodriguez Reboredo <[email protected]>

Reviewed-by Shuah Khan <[email protected]>

Reviewed-by Rae Moar <[email protected]>

Signed-off-by Sergio González Collado <[email protected]>

Link https://github.com/Rust-for-Linux/linux/issues/504

---
V7 -> V8: typo fixed & rebased
---
V6 -> V7: rebased
---
V5 -> V6: remove tests with symbols of length KSYM_NAME_LEN+1
---
V4 -> V5: fixed typo, added improved description
---
V3 -> V4: add x86 mantainers, add new reference.
---
V2 -> V3: updated base and added MODULE_DESCRIPTION() and MODULE_AUTHOR()
---
V1 -> V2: corrected CI tests. Added fix proposed at [3]

[3] https://lore.kernel.org/lkml/Y9ES4UKl%[email protected]/T/#m3ef0e12bb834d01ed1ebdcae12ef5f2add342077

The test execution should result in something like:

```
[20:04:35] =============== longest-symbol (4 subtests) ================
[20:04:35] [PASSED] test_longest_symbol
[20:04:35] [PASSED] test_longest_symbol_kallsyms
[20:04:35] ================= [PASSED] longest-symbol ==================
[20:04:35] ============================================================
[20:04:35] Testing complete. Ran 4 tests: passed: 4
```
---
 arch/x86/tools/insn_decoder_test.c |  3 +-
 lib/Kconfig.debug                  |  9 ++++
 lib/Makefile                       |  2 +
 lib/longest_symbol_kunit.c         | 84 ++++++++++++++++++++++++++++++
 4 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100644 lib/longest_symbol_kunit.c

arch/x86/tools/insn_decoder_test.c +2 -1

index 472540aeabc2..6c2986d2ad11 100644
@@ -10,6 +10,7 @@
 #include <assert.h>
 #include <unistd.h>
 #include <stdarg.h>
+#include <linux/kallsyms.h>
 
 #define unlikely(cond) (cond)

@@ -106,7 +107,7 @@ static void parse_args(int argc, char **argv)
 	}
 }
 
-#define BUFSIZE 256
+#define BUFSIZE (256 + KSYM_NAME_LEN)
 
 int main(int argc, char **argv)
 {

lib/Kconfig.debug +9 -0

index 1af972a92d06..62d43aa9e8f0 100644
@@ -2838,6 +2838,15 @@ config FORTIFY_KUNIT_TEST
 	  by the str*() and mem*() family of functions. For testing runtime
 	  traps of FORTIFY_SOURCE, see LKDTM's "FORTIFY_*" tests.
 
+config LONGEST_SYM_KUNIT_TEST
+	tristate "Test the longest symbol possible" if !KUNIT_ALL_TESTS
+	depends on KUNIT && KPROBES
+	default KUNIT_ALL_TESTS
+	help
+	  Tests the longest symbol possible
+
+	  If unsure, say N.
+
 config HW_BREAKPOINT_KUNIT_TEST
 	bool "Test hw_breakpoint constraints accounting" if !KUNIT_ALL_TESTS
 	depends on HAVE_HW_BREAKPOINT

lib/Makefile +2 -0

index d5cfc7afbbb8..e8fec9defec2 100644
@@ -393,6 +393,8 @@ obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
 obj-$(CONFIG_CRC_KUNIT_TEST) += crc_kunit.o
 obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
 obj-$(CONFIG_USERCOPY_KUNIT_TEST) += usercopy_kunit.o
+obj-$(CONFIG_LONGEST_SYM_KUNIT_TEST) += longest_symbol_kunit.o
+CFLAGS_longest_symbol_kunit.o += $(call cc-disable-warning, missing-prototypes)
 
 obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o

lib/longest_symbol_kunit.c new file +84 -0

84 lines not on this page · raw message

base-commit: a86bf2283d2c9769205407e2b54777c03d012939

Signature · 1 line

2.39.2

Rae Moar

2025-02-07 21:54 UTC #1 lore raw

Quoted 17 lines · Sergio González Collado

The longest length of a symbol (KSYM_NAME_LEN) was increased to 512 in the reference [1]. This patch adds kunit test suite to check the longest symbol length. These tests verify that the longest symbol length defined is supported.
This test can also help other efforts for longer symbol length, like [2].
The test suite defines one symbol with the longest possible length.
The first test verify that functions with names of the created symbol, can be called or not.
The second test, verify that the symbols are created (or not) in the kernel symbol table.

Quoted 8 lines

[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/
Tested-by Martin Rodriguez Reboredo <[email protected]>
Reviewed-by Shuah Khan <[email protected]>
Reviewed-by Rae Moar <[email protected]>
Signed-off-by Sergio González Collado <[email protected]>
Link https://github.com/Rust-for-Linux/linux/issues/504

Hello!

Thanks for fixing the typo and this new version! This patch still does not apply cleanly in the Makefile for me. Have you rebased it on the kselftest/kunit branch? I also have a few more questions that I just noticed.

Thanks!
-Rae

Quoted 47 lines

---
V7 -> V8: typo fixed & rebased
---
V6 -> V7: rebased
---
V5 -> V6: remove tests with symbols of length KSYM_NAME_LEN+1
---
V4 -> V5: fixed typo, added improved description
---
V3 -> V4: add x86 mantainers, add new reference.
---
V2 -> V3: updated base and added MODULE_DESCRIPTION() and MODULE_AUTHOR()
---
V1 -> V2: corrected CI tests. Added fix proposed at [3]
[3] https://lore.kernel.org/lkml/Y9ES4UKl%[email protected]/T/#m3ef0e12bb834d01ed1ebdcae12ef5f2add342077
The test execution should result in something like:
```
[20:04:35] =============== longest-symbol (4 subtests) ================
[20:04:35] [PASSED] test_longest_symbol
[20:04:35] [PASSED] test_longest_symbol_kallsyms
[20:04:35] ================= [PASSED] longest-symbol ==================
[20:04:35] ============================================================
[20:04:35] Testing complete. Ran 4 tests: passed: 4
```
---
 arch/x86/tools/insn_decoder_test.c |  3 +-
 lib/Kconfig.debug                  |  9 ++++
 lib/Makefile                       |  2 +
 lib/longest_symbol_kunit.c         | 84 ++++++++++++++++++++++++++++++
 4 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100644 lib/longest_symbol_kunit.c
arch/x86/tools/insn_decoder_test.c +1 -0
index 472540aeabc2..6c2986d2ad11 100644
@@ -10,6 +10,7 @@
 #include <assert.h>
 #include <unistd.h>
 #include <stdarg.h>
+#include <linux/kallsyms.h>
 
 #define unlikely(cond) (cond)
 

arch/x86/tools/insn_decoder_test.c static void parse_args(int argc, char **argv) · line 110

Quoted 5 lines

arch/x86/tools/insn_decoder_test.c +1 -1

@@ -106,7 +107,7 @@ static void parse_args(int argc, char **argv)
        }
 }
 
-#define BUFSIZE 256
+#define BUFSIZE (256 + KSYM_NAME_LEN)

I'm not too familiar with this test. I believe this would potentially make a symbol with a length that exceeds the KSYM_NAME_LEN. What is the intention for this line?

Quoted 28 lines

arch/x86/tools/insn_decoder_test.c +0 -0

 int main(int argc, char **argv)
 {

lib/Kconfig.debug +9 -0

index 1af972a92d06..62d43aa9e8f0 100644
@@ -2838,6 +2838,15 @@ config FORTIFY_KUNIT_TEST
          by the str*() and mem*() family of functions. For testing runtime
          traps of FORTIFY_SOURCE, see LKDTM's "FORTIFY_*" tests.
 
+config LONGEST_SYM_KUNIT_TEST
+       tristate "Test the longest symbol possible" if !KUNIT_ALL_TESTS
+       depends on KUNIT && KPROBES
+       default KUNIT_ALL_TESTS
+       help
+         Tests the longest symbol possible
+
+         If unsure, say N.
+
 config HW_BREAKPOINT_KUNIT_TEST
        bool "Test hw_breakpoint constraints accounting" if !KUNIT_ALL_TESTS
        depends on HAVE_HW_BREAKPOINT

lib/Makefile +0 -0

index d5cfc7afbbb8..e8fec9defec2 100644

lib/Makefile obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o · line 399

Quoted 7 lines

lib/Makefile +2 -0

@@ -393,6 +393,8 @@ obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
 obj-$(CONFIG_CRC_KUNIT_TEST) += crc_kunit.o
 obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
 obj-$(CONFIG_USERCOPY_KUNIT_TEST) += usercopy_kunit.o
+obj-$(CONFIG_LONGEST_SYM_KUNIT_TEST) += longest_symbol_kunit.o
+CFLAGS_longest_symbol_kunit.o += $(call cc-disable-warning, missing-prototypes)
 
 obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o

These are the lines that are causing the patch to not apply cleanly. The change list that applies cleanly for me is:

obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o obj-$(CONFIG_USERCOPY_KUNIT_TEST) += usercopy_kunit.o obj-$(CONFIG_CRC16_KUNIT_TEST) += crc16_kunit.o
+obj-$(CONFIG_LONGEST_SYM_KUNIT_TEST) += longest_symbol_kunit.o +CFLAGS_longest_symbol_kunit.o += $(call cc-disable-warning, missing-prototypes)

obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o

Quoted 7 lines

lib/longest_symbol_kunit.c new file +0 -0
new file mode 100644
index 000000000000..2a2dd1151097

lib/longest_symbol_kunit.c line 10

Quoted 10 lines

lib/longest_symbol_kunit.c new file +10 -0

@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test the longest symbol length. Execute with:
+ *  ./tools/testing/kunit/kunit.py run longest-symbol
+ *  --arch=x86_64 --kconfig_add CONFIG_KPROBES=y --kconfig_add CONFIG_MODULES=y
+ *  --kconfig_add CONFIG_RETPOLINE=n --kconfig_add CONFIG_CFI_CLANG=n
+ *  --kconfig_add CONFIG_MITIGATION_RETPOLINE=n
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

I don't believe you use this macro. Could probably be deleted.

lib/longest_symbol_kunit.c line 23

Quoted 13 lines

lib/longest_symbol_kunit.c new file +13 -0

+
+#include <kunit/test.h>
+#include <linux/stringify.h>
+#include <linux/kprobes.h>
+#include <linux/kallsyms.h>
+
+#define DI(name) s##name##name
+#define DDI(name) DI(n##name##name)
+#define DDDI(name) DDI(n##name##name)
+#define DDDDI(name) DDDI(n##name##name)
+#define DDDDDI(name) DDDDI(n##name##name)
+
+#define PLUS1(name) __PASTE(name, e)

I don't think you use this anymore with the new changes. Can probably be deleted.

Quoted 6 lines

lib/longest_symbol_kunit.c new file +6 -0

+
+/*Generate a symbol whose name length is 511 */
+#define LONGEST_SYM_NAME  DDDDDI(g1h2i3j4k5l6m7n)
+
+#define RETURN_LONGEST_SYM 0xAAAAA
+

lib/longest_symbol_kunit.c line 39

Quoted 10 lines

lib/longest_symbol_kunit.c new file +10 -0

+noinline int LONGEST_SYM_NAME(void);
+noinline int LONGEST_SYM_NAME(void)
+{
+       return RETURN_LONGEST_SYM;
+}
+
+_Static_assert(sizeof(__stringify(LONGEST_SYM_NAME)) == KSYM_NAME_LEN,
+"Incorrect symbol length found. Expected KSYM_NAME_LEN: "
+__stringify(KSYM_NAME) ", but found: "
+__stringify(sizeof(LONGEST_SYM_NAME)));

Should this error return __stringify(KSYM_NAME_LEN) instead of __stringify(KSYM_NAME) to give the maximum length?

Also, I get an error because the length of LONGEST_SYM_NAME is 512. The error is produced by this code:
https://elixir.bootlin.com/linux/v6.13.1/source/scripts/kallsyms.c#L152
and alerts if the symbol length is >= to KSYM_NAME_LEN. That is fine as long as that is the intention of this test to produce a warning. Or should this warning change to be "> KSYM_NAME_LEN" if usage of symbols that are the maximum length is allowed?

Quoted 50 lines

lib/longest_symbol_kunit.c new file +45 -0

+
+static void test_longest_symbol(struct kunit *test)
+{
+       KUNIT_EXPECT_EQ(test, RETURN_LONGEST_SYM, LONGEST_SYM_NAME());
+};
+
+static void test_longest_symbol_kallsyms(struct kunit *test)
+{
+       unsigned long (*kallsyms_lookup_name)(const char *name);
+       static int (*longest_sym)(void);
+
+       struct kprobe kp = {
+               .symbol_name = "kallsyms_lookup_name",
+       };
+
+       if (register_kprobe(&kp) < 0) {
+               pr_info("%s: kprobe not registered\n", __func__);
+               KUNIT_FAIL(test, "test_longest_symbol kallsyms: kprobe not registered\n");
+               return;
+       }
+
+       kunit_warn(test, "test_longest_symbol kallsyms: kprobe registered\n");
+       kallsyms_lookup_name = (unsigned long (*)(const char *name))kp.addr;
+       unregister_kprobe(&kp);
+
+       longest_sym =
+               (void *) kallsyms_lookup_name(__stringify(LONGEST_SYM_NAME));
+       KUNIT_EXPECT_EQ(test, RETURN_LONGEST_SYM, longest_sym());
+};
+
+static struct kunit_case longest_symbol_test_cases[] = {
+       KUNIT_CASE(test_longest_symbol),
+       KUNIT_CASE(test_longest_symbol_kallsyms),
+       {}
+};
+
+static struct kunit_suite longest_symbol_test_suite = {
+       .name = "longest-symbol",
+       .test_cases = longest_symbol_test_cases,
+};
+kunit_test_suite(longest_symbol_test_suite);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Test the longest symbol length");
+MODULE_AUTHOR("Sergio González Collado");

base-commit: a86bf2283d2c9769205407e2b54777c03d012939 --
2.39.2

Sergio González Collado

author

2025-02-17 20:30 UTC #2 lore raw

Hello,

Quoted 89 lines · Rae Moar

Quoted 17 lines · Sergio González Collado
The longest length of a symbol (KSYM_NAME_LEN) was increased to 512 in the reference [1]. This patch adds kunit test suite to check the longest symbol length. These tests verify that the longest symbol length defined is supported.
This test can also help other efforts for longer symbol length, like [2].
The test suite defines one symbol with the longest possible length.
The first test verify that functions with names of the created symbol, can be called or not.
The second test, verify that the symbols are created (or not) in the kernel symbol table.
Quoted 8 lines
[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/
Tested-by Martin Rodriguez Reboredo <[email protected]>
Reviewed-by Shuah Khan <[email protected]>
Reviewed-by Rae Moar <[email protected]>
Signed-off-by Sergio González Collado <[email protected]>
Link https://github.com/Rust-for-Linux/linux/issues/504
Hello!
Thanks for fixing the typo and this new version! This patch still does not apply cleanly in the Makefile for me. Have you rebased it on the kselftest/kunit branch? I also have a few more questions that I just noticed.
Thanks!
-Rae
Quoted 50 lines
---
V7 -> V8: typo fixed & rebased
---
V6 -> V7: rebased
---
V5 -> V6: remove tests with symbols of length KSYM_NAME_LEN+1
---
V4 -> V5: fixed typo, added improved description
---
V3 -> V4: add x86 mantainers, add new reference.
---
V2 -> V3: updated base and added MODULE_DESCRIPTION() and MODULE_AUTHOR()
---
V1 -> V2: corrected CI tests. Added fix proposed at [3]
[3] https://lore.kernel.org/lkml/Y9ES4UKl%[email protected]/T/#m3ef0e12bb834d01ed1ebdcae12ef5f2add342077
The test execution should result in something like:
```
[20:04:35] =============== longest-symbol (4 subtests) ================
[20:04:35] [PASSED] test_longest_symbol
[20:04:35] [PASSED] test_longest_symbol_kallsyms
[20:04:35] ================= [PASSED] longest-symbol ==================
[20:04:35] ============================================================
[20:04:35] Testing complete. Ran 4 tests: passed: 4
```
---
 arch/x86/tools/insn_decoder_test.c |  3 +-
 lib/Kconfig.debug                  |  9 ++++
 lib/Makefile                       |  2 +
 lib/longest_symbol_kunit.c         | 84 ++++++++++++++++++++++++++++++
 4 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100644 lib/longest_symbol_kunit.c
arch/x86/tools/insn_decoder_test.c +1 -0
index 472540aeabc2..6c2986d2ad11 100644
@@ -10,6 +10,7 @@
 #include <assert.h>
 #include <unistd.h>
 #include <stdarg.h>
+#include <linux/kallsyms.h>
 
 #define unlikely(cond) (cond)
 
@@ -106,7 +107,7 @@ static void parse_args(int argc, char **argv)
        }
 }
 

Quoted 6 lines

arch/x86/tools/insn_decoder_test.c static void parse_args(int argc, char **argv) · line 110
Quoted 2 lines
arch/x86/tools/insn_decoder_test.c +1 -1
-#define BUFSIZE 256
+#define BUFSIZE (256 + KSYM_NAME_LEN)
I'm not too familiar with this test. I believe this would potentially make a symbol with a length that exceeds the KSYM_NAME_LEN. What is the intention for this line?

That will define how much space to write a symbol. I'm also not familiar with that test, but I know had to be fixed:
https://lore.kernel.org/lkml/Y9ES4UKl%[email protected]/T/#m3ef0e12bb834d01ed1ebdcae12ef5f2add342077

Quoted 45 lines

Quoted 29 lines

arch/x86/tools/insn_decoder_test.c +0 -0

 int main(int argc, char **argv)
 {

lib/Kconfig.debug +9 -0

index 1af972a92d06..62d43aa9e8f0 100644
@@ -2838,6 +2838,15 @@ config FORTIFY_KUNIT_TEST
          by the str*() and mem*() family of functions. For testing runtime
          traps of FORTIFY_SOURCE, see LKDTM's "FORTIFY_*" tests.
 
+config LONGEST_SYM_KUNIT_TEST
+       tristate "Test the longest symbol possible" if !KUNIT_ALL_TESTS
+       depends on KUNIT && KPROBES
+       default KUNIT_ALL_TESTS
+       help
+         Tests the longest symbol possible
+
+         If unsure, say N.
+
 config HW_BREAKPOINT_KUNIT_TEST
        bool "Test hw_breakpoint constraints accounting" if !KUNIT_ALL_TESTS
        depends on HAVE_HW_BREAKPOINT

lib/Makefile +0 -0

index d5cfc7afbbb8..e8fec9defec2 100644

lib/Makefile obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o · line 399

Quoted 7 lines

lib/Makefile +2 -0

@@ -393,6 +393,8 @@ obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
 obj-$(CONFIG_CRC_KUNIT_TEST) += crc_kunit.o
 obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
 obj-$(CONFIG_USERCOPY_KUNIT_TEST) += usercopy_kunit.o
+obj-$(CONFIG_LONGEST_SYM_KUNIT_TEST) += longest_symbol_kunit.o
+CFLAGS_longest_symbol_kunit.o += $(call cc-disable-warning, missing-prototypes)
 
 obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o

These are the lines that are causing the patch to not apply cleanly. The change list that applies cleanly for me is:

lib/Makefile obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o · line 399

Quoted 4 lines

lib/Makefile +2 -0

+obj-$(CONFIG_LONGEST_SYM_KUNIT_TEST) += longest_symbol_kunit.o
+CFLAGS_longest_symbol_kunit.o += $(call cc-disable-warning, missing-prototypes)
 
 obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o

I will create a new rebased version of the patch.

Quoted 11 lines

lib/longest_symbol_kunit.c new file +4 -0

new file mode 100644
index 000000000000..2a2dd1151097
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test the longest symbol length. Execute with:
+ *  ./tools/testing/kunit/kunit.py run longest-symbol

Quoted 9 lines

lib/longest_symbol_kunit.c line 10
Quoted 6 lines
lib/longest_symbol_kunit.c new file +6 -0
+ *  --arch=x86_64 --kconfig_add CONFIG_KPROBES=y --kconfig_add CONFIG_MODULES=y
+ *  --kconfig_add CONFIG_RETPOLINE=n --kconfig_add CONFIG_CFI_CLANG=n
+ *  --kconfig_add CONFIG_MITIGATION_RETPOLINE=n
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
I don't believe you use this macro. Could probably be deleted.

This macro should work out of the box by means of the pr_*() macros:
https://docs.kernel.org/core-api/printk-basics.html#c.pr_fmt

Quoted 18 lines

Quoted 4 lines
lib/longest_symbol_kunit.c new file +3 -0
+
+#include <kunit/test.h>
+#include <linux/stringify.h>
lib/longest_symbol_kunit.c line 23
Quoted 10 lines
lib/longest_symbol_kunit.c new file +10 -0
+#include <linux/kprobes.h>
+#include <linux/kallsyms.h>
+
+#define DI(name) s##name##name
+#define DDI(name) DI(n##name##name)
+#define DDDI(name) DDI(n##name##name)
+#define DDDDI(name) DDDI(n##name##name)
+#define DDDDDI(name) DDDDI(n##name##name)
+
+#define PLUS1(name) __PASTE(name, e)
I don't think you use this anymore with the new changes. Can probably be deleted.

Indeed, I totally overlooked that.

Quoted 12 lines

lib/longest_symbol_kunit.c new file +11 -0

+
+/*Generate a symbol whose name length is 511 */
+#define LONGEST_SYM_NAME  DDDDDI(g1h2i3j4k5l6m7n)
+
+#define RETURN_LONGEST_SYM 0xAAAAA
+
+noinline int LONGEST_SYM_NAME(void);
+noinline int LONGEST_SYM_NAME(void)
+{
+       return RETURN_LONGEST_SYM;
+}

Quoted 9 lines

lib/longest_symbol_kunit.c line 39
Quoted 5 lines
lib/longest_symbol_kunit.c new file +5 -0
+
+_Static_assert(sizeof(__stringify(LONGEST_SYM_NAME)) == KSYM_NAME_LEN,
+"Incorrect symbol length found. Expected KSYM_NAME_LEN: "
+__stringify(KSYM_NAME) ", but found: "
+__stringify(sizeof(LONGEST_SYM_NAME)));
Should this error return __stringify(KSYM_NAME_LEN) instead of __stringify(KSYM_NAME) to give the maximum length?

It is done on purpose to show what the actual symbol is. Under some conditions the names can incorporate
prefix symbols, for example:
https://lore.kernel.org/lkml/[email protected]/,
if this happens it will be easier to spot what is going on.

Quoted 9 lines

Also, I get an error because the length of LONGEST_SYM_NAME is 512. The error is produced by this code:
https://elixir.bootlin.com/linux/v6.13.1/source/scripts/kallsyms.c#L152
and alerts if the symbol length is >= to KSYM_NAME_LEN. That is fine as long as that is the intention of this test to produce a warning. Or should this warning change to be "> KSYM_NAME_LEN" if usage of symbols that are the maximum length is allowed?

Indeed that is expected, as the limit is reached. I think it may not be really needed to update the condition to "> KSYM_NAME_LEN".

Thanks a lot for your remarks.
Cheers!

Rae Moar

2025-02-19 23:10 UTC #3 lore raw

Quoted 174 lines · Sergio González Collado

Hello,
Quoted 89 lines · Rae Moar
Quoted 17 lines · Sergio González Collado
The longest length of a symbol (KSYM_NAME_LEN) was increased to 512 in the reference [1]. This patch adds kunit test suite to check the longest symbol length. These tests verify that the longest symbol length defined is supported.
This test can also help other efforts for longer symbol length, like [2].
The test suite defines one symbol with the longest possible length.
The first test verify that functions with names of the created symbol, can be called or not.
The second test, verify that the symbols are created (or not) in the kernel symbol table.
Quoted 8 lines
[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/
Tested-by Martin Rodriguez Reboredo <[email protected]>
Reviewed-by Shuah Khan <[email protected]>
Reviewed-by Rae Moar <[email protected]>
Signed-off-by Sergio González Collado <[email protected]>
Link https://github.com/Rust-for-Linux/linux/issues/504
Hello!
Thanks for fixing the typo and this new version! This patch still does not apply cleanly in the Makefile for me. Have you rebased it on the kselftest/kunit branch? I also have a few more questions that I just noticed.
Thanks!
-Rae
Quoted 50 lines
---
V7 -> V8: typo fixed & rebased
---
V6 -> V7: rebased
---
V5 -> V6: remove tests with symbols of length KSYM_NAME_LEN+1
---
V4 -> V5: fixed typo, added improved description
---
V3 -> V4: add x86 mantainers, add new reference.
---
V2 -> V3: updated base and added MODULE_DESCRIPTION() and MODULE_AUTHOR()
---
V1 -> V2: corrected CI tests. Added fix proposed at [3]
[3] https://lore.kernel.org/lkml/Y9ES4UKl%[email protected]/T/#m3ef0e12bb834d01ed1ebdcae12ef5f2add342077
The test execution should result in something like:
```
[20:04:35] =============== longest-symbol (4 subtests) ================
[20:04:35] [PASSED] test_longest_symbol
[20:04:35] [PASSED] test_longest_symbol_kallsyms
[20:04:35] ================= [PASSED] longest-symbol ==================
[20:04:35] ============================================================
[20:04:35] Testing complete. Ran 4 tests: passed: 4
```
---
 arch/x86/tools/insn_decoder_test.c |  3 +-
 lib/Kconfig.debug                  |  9 ++++
 lib/Makefile                       |  2 +
 lib/longest_symbol_kunit.c         | 84 ++++++++++++++++++++++++++++++
 4 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100644 lib/longest_symbol_kunit.c
arch/x86/tools/insn_decoder_test.c +1 -0
index 472540aeabc2..6c2986d2ad11 100644
@@ -10,6 +10,7 @@
 #include <assert.h>
 #include <unistd.h>
 #include <stdarg.h>
+#include <linux/kallsyms.h>
 
 #define unlikely(cond) (cond)
 
@@ -106,7 +107,7 @@ static void parse_args(int argc, char **argv)
        }
 }
 
Quoted 6 lines
arch/x86/tools/insn_decoder_test.c static void parse_args(int argc, char **argv) · line 110
Quoted 2 lines
arch/x86/tools/insn_decoder_test.c +1 -1
-#define BUFSIZE 256
+#define BUFSIZE (256 + KSYM_NAME_LEN)
I'm not too familiar with this test. I believe this would potentially make a symbol with a length that exceeds the KSYM_NAME_LEN. What is the intention for this line?
That will define how much space to write a symbol. I'm also not familiar with that test, but I know had to be fixed:
https://lore.kernel.org/lkml/Y9ES4UKl%[email protected]/T/#m3ef0e12bb834d01ed1ebdcae12ef5f2add342077
Quoted 45 lines
Quoted 29 lines
arch/x86/tools/insn_decoder_test.c +0 -0
 int main(int argc, char **argv)
 {
lib/Kconfig.debug +9 -0
index 1af972a92d06..62d43aa9e8f0 100644
@@ -2838,6 +2838,15 @@ config FORTIFY_KUNIT_TEST
          by the str*() and mem*() family of functions. For testing runtime
          traps of FORTIFY_SOURCE, see LKDTM's "FORTIFY_*" tests.
 
+config LONGEST_SYM_KUNIT_TEST
+       tristate "Test the longest symbol possible" if !KUNIT_ALL_TESTS
+       depends on KUNIT && KPROBES
+       default KUNIT_ALL_TESTS
+       help
+         Tests the longest symbol possible
+
+         If unsure, say N.
+
 config HW_BREAKPOINT_KUNIT_TEST
        bool "Test hw_breakpoint constraints accounting" if !KUNIT_ALL_TESTS
        depends on HAVE_HW_BREAKPOINT
lib/Makefile +0 -0
index d5cfc7afbbb8..e8fec9defec2 100644
lib/Makefile obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o · line 399
Quoted 7 lines
lib/Makefile +2 -0
@@ -393,6 +393,8 @@ obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
 obj-$(CONFIG_CRC_KUNIT_TEST) += crc_kunit.o
 obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
 obj-$(CONFIG_USERCOPY_KUNIT_TEST) += usercopy_kunit.o
+obj-$(CONFIG_LONGEST_SYM_KUNIT_TEST) += longest_symbol_kunit.o
+CFLAGS_longest_symbol_kunit.o += $(call cc-disable-warning, missing-prototypes)
 
 obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
These are the lines that are causing the patch to not apply cleanly. The change list that applies cleanly for me is:
obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o obj-$(CONFIG_USERCOPY_KUNIT_TEST) += usercopy_kunit.o obj-$(CONFIG_CRC16_KUNIT_TEST) += crc16_kunit.o
lib/Makefile obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o · line 399
Quoted 4 lines
lib/Makefile +2 -0
+obj-$(CONFIG_LONGEST_SYM_KUNIT_TEST) += longest_symbol_kunit.o
+CFLAGS_longest_symbol_kunit.o += $(call cc-disable-warning, missing-prototypes)
 
 obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
I will create a new rebased version of the patch.
Quoted 11 lines
Quoted 11 lines
lib/longest_symbol_kunit.c new file +4 -0
new file mode 100644
index 000000000000..2a2dd1151097
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test the longest symbol length. Execute with:
+ *  ./tools/testing/kunit/kunit.py run longest-symbol
Quoted 9 lines
lib/longest_symbol_kunit.c line 10
Quoted 6 lines
lib/longest_symbol_kunit.c new file +6 -0
+ *  --arch=x86_64 --kconfig_add CONFIG_KPROBES=y --kconfig_add CONFIG_MODULES=y
+ *  --kconfig_add CONFIG_RETPOLINE=n --kconfig_add CONFIG_CFI_CLANG=n
+ *  --kconfig_add CONFIG_MITIGATION_RETPOLINE=n
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
I don't believe you use this macro. Could probably be deleted.

Quoted 2 lines

This macro should work out of the box by means of the pr_*() macros:
https://docs.kernel.org/core-api/printk-basics.html#c.pr_fmt

Hi! Thanks for your responses.

This seems good to me. Thanks for the explanation.

Quoted 42 lines

Quoted 19 lines
Quoted 5 lines
lib/longest_symbol_kunit.c new file +3 -0
+
+#include <kunit/test.h>
+#include <linux/stringify.h>
lib/longest_symbol_kunit.c line 23
Quoted 10 lines
lib/longest_symbol_kunit.c new file +10 -0
+#include <linux/kprobes.h>
+#include <linux/kallsyms.h>
+
+#define DI(name) s##name##name
+#define DDI(name) DI(n##name##name)
+#define DDDI(name) DDI(n##name##name)
+#define DDDDI(name) DDDI(n##name##name)
+#define DDDDDI(name) DDDDI(n##name##name)
+
+#define PLUS1(name) __PASTE(name, e)
I don't think you use this anymore with the new changes. Can probably be deleted.
Indeed, I totally overlooked that.
Quoted 12 lines
Quoted 12 lines
lib/longest_symbol_kunit.c new file +11 -0
+
+/*Generate a symbol whose name length is 511 */
+#define LONGEST_SYM_NAME  DDDDDI(g1h2i3j4k5l6m7n)
+
+#define RETURN_LONGEST_SYM 0xAAAAA
+
+noinline int LONGEST_SYM_NAME(void);
+noinline int LONGEST_SYM_NAME(void)
+{
+       return RETURN_LONGEST_SYM;
+}
Quoted 9 lines
lib/longest_symbol_kunit.c line 39
Quoted 5 lines
lib/longest_symbol_kunit.c new file +5 -0
+
+_Static_assert(sizeof(__stringify(LONGEST_SYM_NAME)) == KSYM_NAME_LEN,
+"Incorrect symbol length found. Expected KSYM_NAME_LEN: "
+__stringify(KSYM_NAME) ", but found: "
+__stringify(sizeof(LONGEST_SYM_NAME)));
Should this error return __stringify(KSYM_NAME_LEN) instead of __stringify(KSYM_NAME) to give the maximum length?

Quoted 5 lines

It is done on purpose to show what the actual symbol is. Under some conditions the names can incorporate
prefix symbols, for example:
https://lore.kernel.org/lkml/[email protected]/,
if this happens it will be easier to spot what is going on.

Right now this outputs: "Incorrect symbol len. Expected KSYM_NAME_LEN:
KSYM_NAME, but found: sizeof(snn...6m7n)"
This seems incorrect because KSYM_NAME has no value. Instead I would
expect: "Incorrect symbol len. Expected KSYM_NAME_LEN: 512, but
found: sizeof(snn...6m7n)".

Quoted 10 lines

Quoted 10 lines
Also, I get an error because the length of LONGEST_SYM_NAME is 512. The error is produced by this code:
https://elixir.bootlin.com/linux/v6.13.1/source/scripts/kallsyms.c#L152
and alerts if the symbol length is >= to KSYM_NAME_LEN. That is fine as long as that is the intention of this test to produce a warning. Or should this warning change to be "> KSYM_NAME_LEN" if usage of symbols that are the maximum length is allowed?

Quoted 3 lines

Indeed that is expected, as the limit is reached. I think it may not be really needed to update the condition to "> KSYM_NAME_LEN".

That seems good to me.

Thanks!
-Rae

Quoted 3 lines

Thanks a lot for your remarks.
Cheers!