[PATCH 6.12.y 0/2] Kunit to check the longest symbol length

[PATCH 6.12.y 0/2] Kunit to check the longest symbol length

[Sergio González Collado]

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

[PATCH 6.12.y 1/2] Kunit to check the longest symbol length

[Sergio González Collado]

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");
-- 
2.39.2

Re: [PATCH 6.12.y 1/2] Kunit to check the longest symbol length

[Miguel Ojeda]

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

Re: [PATCH 6.12.y 1/2] Kunit to check the longest symbol length

[Sergio González Collado]

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

Re: [PATCH 6.12.y 1/2] Kunit to check the longest symbol length

[Greg KH]

On Sun, Jun 08, 2025 at 06:10:13PM +0200, Sergio González Collado wrote:
> 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")'

No need to run checkpatch on stuff you are backporting to stable, as it
should follow the same style as what ever is in Linus's tree.

We need/want the full hash here.

thanks,

greg k-h

Re: [PATCH 6.12.y 1/2] Kunit to check the longest symbol length

[Sergio González Collado]

Hello,

 Should I resend the patch v2 with the full hash? or is it ok as
Miguel already mentioned it?

 commit c104c16073b7fdb3e4eae18f66f4009f6b073d6f upstream.

 Either way would be ok for me.

Sergio

On Tue, 17 Jun 2025 at 16:00, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sun, Jun 08, 2025 at 06:10:13PM +0200, Sergio González Collado wrote:
> > 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")'
>
> No need to run checkpatch on stuff you are backporting to stable, as it
> should follow the same style as what ever is in Linus's tree.
>
> We need/want the full hash here.
>
> thanks,
>
> greg k-h

Re: [PATCH 6.12.y 1/2] Kunit to check the longest symbol length

[Miguel Ojeda]

On Tue, Jun 17, 2025 at 5:51 PM Sergio González Collado
<sergio.collado@gmail.com> wrote:
>
>  Should I resend the patch v2 with the full hash? or is it ok as
> Miguel already mentioned it?
>
>  commit c104c16073b7fdb3e4eae18f66f4009f6b073d6f upstream.

Since you said you didn't modify it, then if the patch applies cleanly
against 6.12.y (which it seems so currently, from a quick test), then
Option 2 in the instructions should be enough.

Thanks!

Cheers,
Miguel

Re: [PATCH 6.12.y 1/2] Kunit to check the longest symbol length

[Sasha Levin]

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

[PATCH 6.12.y 2/2] x86/tools: Drop duplicate unlikely() definition in insn_decoder_test.c

[Sergio González Collado]

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-74c84a7bf05b@kernel.org
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>
-- 
2.39.2

Re: [PATCH 6.12.y 2/2] x86/tools: Drop duplicate unlikely() definition in insn_decoder_test.c

[Miguel Ojeda]

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

Re: [PATCH 6.12.y 2/2] x86/tools: Drop duplicate unlikely() definition in insn_decoder_test.c

[Sasha Levin]

[ 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 Chancellor<nathan@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-74c84a7bf05b@kernel.org
    +    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   |

Re: [PATCH 6.12.y 0/2] Kunit to check the longest symbol length

[Miguel Ojeda]

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