The longest length of a symbol (KSYM_NAME_LEN) was increased to 512 in the reference [1]. Because in Rust symbols can become quite long due to namespacing introduced by modules, types, traits, generics, etc.
This patch series presents two commits that implement a test to verify that a symbol with KSYM_NAME_LEN of 512 can be read.
The first commit: To check that symbol length was valid, the commit implements a kunit test that verifies that a symbol of 512 length can be read.
The second commit: There was a warning when building with clang because there was a definition of unlikely from compiler.h in tools/include/linux, which conflicted with the one in the instruction decoder selftest.
[1] https://lore.kernel.org/lkml/20220802015052.10452-6-ojeda@kernel.org/
--- Nathan Chancellor (1): x86/tools: Drop duplicate unlikely() definition in insn_decoder_test.c
Sergio González Collado (1): Kunit to check the longest symbol length
arch/x86/tools/insn_decoder_test.c | 5 +- lib/Kconfig.debug | 9 ++++ lib/Makefile | 2 + lib/longest_symbol_kunit.c | 82 ++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 lib/longest_symbol_kunit.c
base-commit: ba9210b8c96355a16b78e1b890dce78f284d6f31
commit c104c16073b7 ("Kunit to check the longest symbol length") upstream
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/20220802015052.10452-6-ojeda@kernel.org/ [2] https://lore.kernel.org/lkml/20240605032120.3179157-1-song@kernel.org/
Link: https://lore.kernel.org/r/20250302221518.76874-1-sergio.collado@gmail.com Tested-by: Martin Rodriguez Reboredo yakoyoku@gmail.com Reviewed-by: Shuah Khan skhan@linuxfoundation.org Reviewed-by: Rae Moar rmoar@google.com Signed-off-by: Sergio González Collado sergio.collado@gmail.com Link: https://github.com/Rust-for-Linux/linux/issues/504 Acked-by: David Gow davidgow@google.com Signed-off-by: Shuah Khan shuah@kernel.org --- arch/x86/tools/insn_decoder_test.c | 3 +- lib/Kconfig.debug | 9 ++++ lib/Makefile | 2 + lib/longest_symbol_kunit.c | 82 ++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 lib/longest_symbol_kunit.c
diff --git a/arch/x86/tools/insn_decoder_test.c b/arch/x86/tools/insn_decoder_test.c index 472540aeabc2..6c2986d2ad11 100644 --- a/arch/x86/tools/insn_decoder_test.c +++ b/arch/x86/tools/insn_decoder_test.c @@ -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) { diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index e48375fe5a50..b1d7c427bbe3 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2807,6 +2807,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 diff --git a/lib/Makefile b/lib/Makefile index 773adf88af41..fc878e716825 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -389,6 +389,8 @@ CFLAGS_fortify_kunit.o += $(DISABLE_STRUCTLEAK_PLUGIN) 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_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
diff --git a/lib/longest_symbol_kunit.c b/lib/longest_symbol_kunit.c new file mode 100644 index 000000000000..e3c28ff1807f --- /dev/null +++ b/lib/longest_symbol_kunit.c @@ -0,0 +1,82 @@ +// 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 + +#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) + +/*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; +} + +_Static_assert(sizeof(__stringify(LONGEST_SYM_NAME)) == KSYM_NAME_LEN, +"Incorrect symbol length found. Expected KSYM_NAME_LEN: " +__stringify(KSYM_NAME_LEN) ", but found: " +__stringify(sizeof(LONGEST_SYM_NAME))); + +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", __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");
On Sun, Jun 8, 2025 at 4:55 PM Sergio González Collado sergio.collado@gmail.com wrote:
commit c104c16073b7 ("Kunit to check the longest symbol length") upstream
I think this may need to be the full hash, and a period at the end:
commit c104c16073b7fdb3e4eae18f66f4009f6b073d6f upstream.
But like in the other patch, maybe the stable team's tooling still picks it up or maybe they fix it manually.
However, more importantly, is there any difference w.r.t. the original mainline commit?
If yes, what the difference is should be mentioned.
If not, this probably could just be Option 2, since at least if I take the hash I can directly apply it (auto-merged). Same for the other patch.
Thanks!
Cheers, Miguel
Hello,
Thanks for the remarks.
The commit is exactly the same as in the mainline commit.
The upstreamed commit, is mentioned in that way, because when I used the full hash, I was getting this error from scripts/checkpatch.pl:
ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 0123456789ab ("commit description")' #10:
Regards, Sergio
On Sun, 8 Jun 2025 at 17:59, Miguel Ojeda miguel.ojeda.sandonis@gmail.com wrote:
On Sun, Jun 8, 2025 at 4:55 PM Sergio González Collado sergio.collado@gmail.com wrote:
commit c104c16073b7 ("Kunit to check the longest symbol length") upstream
I think this may need to be the full hash, and a period at the end:
commit c104c16073b7fdb3e4eae18f66f4009f6b073d6f upstream.
But like in the other patch, maybe the stable team's tooling still picks it up or maybe they fix it manually.
However, more importantly, is there any difference w.r.t. the original mainline commit?
If yes, what the difference is should be mentioned.
If not, this probably could just be Option 2, since at least if I take the hash I can directly apply it (auto-merged). Same for the other patch.
Thanks!
Cheers, Miguel
[ Sasha's backport helper bot ]
Hi,
Summary of potential issues: ⚠️ Found matching upstream commit but patch is missing proper reference to it ⚠️ Found follow-up fixes in mainline
Found matching upstream commit: c104c16073b7fdb3e4eae18f66f4009f6b073d6f
Status in newer kernel trees: 6.15.y | Present (exact SHA1) 6.14.y | Not found
Found fixes commits: f710202b2a45 x86/tools: Drop duplicate unlikely() definition in insn_decoder_test.c
Note: The patch differs from the upstream commit: --- 1: c104c16073b7f ! 1: 39ce5e9e3c965 Kunit to check the longest symbol length @@ Metadata ## Commit message ## Kunit to check the longest symbol length
+ commit c104c16073b7 ("Kunit to check the longest symbol length") upstream + 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 @@ Commit message Reviewed-by: Rae Moar rmoar@google.com Signed-off-by: Sergio González Collado sergio.collado@gmail.com Link: https://github.com/Rust-for-Linux/linux/issues/504 - Reviewed-by: Rae Moar rmoar@google.com Acked-by: David Gow davidgow@google.com Signed-off-by: Shuah Khan shuah@kernel.org
@@ lib/Kconfig.debug: config FORTIFY_KUNIT_TEST depends on HAVE_HW_BREAKPOINT
## lib/Makefile ## -@@ lib/Makefile: obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o - obj-$(CONFIG_CRC_KUNIT_TEST) += crc_kunit.o +@@ lib/Makefile: CFLAGS_fortify_kunit.o += $(DISABLE_STRUCTLEAK_PLUGIN) + 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_LONGEST_SYM_KUNIT_TEST) += longest_symbol_kunit.o ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.12.y | Success | Success |
From: Nathan Chancellor nathan@kernel.org
From Nathan Chancellor nathan@kernel.org
[Upstream f710202b2a45addea3dcdcd862770ecbaf6597ef]
After commit c104c16073b7 ("Kunit to check the longest symbol length"), there is a warning when building with clang because there is now a definition of unlikely from compiler.h in tools/include/linux, which conflicts with the one in the instruction decoder selftest:
arch/x86/tools/insn_decoder_test.c:15:9: warning: 'unlikely' macro redefined [-Wmacro-redefined]
Remove the second unlikely() definition, as it is no longer necessary, clearing up the warning.
Fixes: c104c16073b7 ("Kunit to check the longest symbol length") Signed-off-by: Nathan Chancellor nathan@kernel.org Signed-off-by: Ingo Molnar mingo@kernel.org Acked-by: Shuah Khan skhan@linuxfoundation.org Link: https://lore.kernel.org/r/20250318-x86-decoder-test-fix-unlikely-redef-v1-1-... Signed-off-by: Sergio González Collado sergio.collado@gmail.com --- arch/x86/tools/insn_decoder_test.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/arch/x86/tools/insn_decoder_test.c b/arch/x86/tools/insn_decoder_test.c index 6c2986d2ad11..08cd913cbd4e 100644 --- a/arch/x86/tools/insn_decoder_test.c +++ b/arch/x86/tools/insn_decoder_test.c @@ -12,8 +12,6 @@ #include <stdarg.h> #include <linux/kallsyms.h>
-#define unlikely(cond) (cond) - #include <asm/insn.h> #include <inat.c> #include <insn.c>
On Sun, Jun 8, 2025 at 4:55 PM Sergio González Collado sergio.collado@gmail.com wrote:
From: Nathan Chancellor nathan@kernel.org
From Nathan Chancellor nathan@kernel.org
I think the second one does not need to be here.
[Upstream f710202b2a45addea3dcdcd862770ecbaf6597ef]
If you use the second format, I think this would need to be:
[ Upstream commit f710202b2a45addea3dcdcd862770ecbaf6597ef ]
But maybe the stable team's tooling still recognizes it or they fix it manually.
#include <stdarg.h> #include <linux/kallsyms.h>
-#define unlikely(cond) (cond)
#include <asm/insn.h> #include <inat.c> #include <insn.c>
The contents seem indeed equivalent to the original commit, so it looks fine to me if Option 3 is used:
Acked-by: Miguel Ojeda ojeda@kernel.org
Thanks!
Cheers, Miguel
[ Sasha's backport helper bot ]
Hi,
Summary of potential issues: ℹ️ This is part 2/2 of a series ⚠️ Found matching upstream commit but patch is missing proper reference to it
Found matching upstream commit: f710202b2a45addea3dcdcd862770ecbaf6597ef
WARNING: Author mismatch between patch and found commit: Backport author: sergio.collado@gmail.com Commit author: Nathan Chancellornathan@kernel.org
Status in newer kernel trees: 6.15.y | Present (exact SHA1) 6.14.y | Not found
Note: The patch differs from the upstream commit: --- 1: f710202b2a45a ! 1: aea5c55c49b88 x86/tools: Drop duplicate unlikely() definition in insn_decoder_test.c @@ Metadata ## Commit message ## x86/tools: Drop duplicate unlikely() definition in insn_decoder_test.c
+ From Nathan Chancellor nathan@kernel.org + + [Upstream f710202b2a45addea3dcdcd862770ecbaf6597ef] + After commit c104c16073b7 ("Kunit to check the longest symbol length"), there is a warning when building with clang because there is now a definition of unlikely from compiler.h in tools/include/linux, which @@ Commit message Signed-off-by: Ingo Molnar mingo@kernel.org Acked-by: Shuah Khan skhan@linuxfoundation.org Link: https://lore.kernel.org/r/20250318-x86-decoder-test-fix-unlikely-redef-v1-1-... + Signed-off-by: Sergio González Collado sergio.collado@gmail.com
## arch/x86/tools/insn_decoder_test.c ## @@ ---
NOTE: These results are for this patch alone. Full series testing will be performed when all parts are received.
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.14.y | Success | Success |
On Sun, Jun 8, 2025 at 4:55 PM Sergio González Collado sergio.collado@gmail.com wrote:
This patch series presents two commits that implement a test to verify that a symbol with KSYM_NAME_LEN of 512 can be read.
The cover letter for a backport should mainly justify why the commits need to be backported to a particular stable kernel, i.e. it is good to explain the commits, but that should be already explain in the upstream ones.
As far as I understood, this solved an actual error error in some configs, which is the key part for the backport -- copy-pasting the string here for future searches:
arch/x86/tools/insn_decoder_test: error: malformed line
Thanks!
Cheers, Miguel
linux-stable-mirror@lists.linaro.org