Re: [PATCH v5] Kunit to check the longest symbol length

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Nov 17, 2024 at 2:59 PM Sergio González Collado
<sergio.collado@xxxxxxxxx> wrote:
>
> 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 two symbols, one with the longest length defined,
> and other one whit the longest length defined +1. In the test suite
> there are four tests, three positive and one negative.
>
> The first and third tests, verify that functions with names of the created
> symbols, can be called or not.
>
> The second and fourth tests, verify that the symbols are created (or
> not) in the kernel symbol table.
>
> [1] https://lore.kernel.org/lkml/20220802015052.10452-6-ojeda@xxxxxxxxxx/
> [2] https://lore.kernel.org/lkml/20240605032120.3179157-1-song@xxxxxxxxxx/
>
> Tested-by: Martin Rodriguez Reboredo <yakoyoku@xxxxxxxxx>
> Reviewed-by: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Sergio González Collado <sergio.collado@xxxxxxxxx>
> Link: https://github.com/Rust-for-Linux/linux/issues/504

Hello!

This patch is passing tests when it is built-in but it is failing when
I run it as a module.

Here is the result when run as a module with "modprobe longest_symbol_kunit":

[   30.446570] KTAP version 1
[   30.446826] 1..1
[   30.453560]     KTAP version 1
[   30.453785]     # Subtest: longest-symbol
[   30.454376]     # module: longest_symbol_kunit
[   30.457622]     1..4
[   30.463765]     ok 1 test_longest_symbol
[   30.471805]     # test_longest_symbol_kallsyms: test_longest_symbol
kallsyms: kprobe registered
[   30.478995]     ok 2 test_longest_symbol_kallsyms
[   30.479735]     ok 3 test_longest_symbol_plus1
[   30.596269]     # test_longest_symbol_plus1_kallsyms:
test_longest_symbol_plus1 kallsyms: kprobe registered
[   30.602564]     # test_longest_symbol_plus1_kallsyms: EXPECTATION
FAILED at lib/longest_symbol_kunit.c:106
[   30.602564]     Expected longest_sym_plus1 == ((void *)0), but
[   30.602564]         longest_sym_plus1 == ffffffffc01313e0
[   30.602564]         ((void *)0) == 0000000000000000
[   30.603924]     not ok 4 test_longest_symbol_plus1_kallsyms
[   30.606305] # longest-symbol: pass:3 fail:1 skip:0 total:4
[   30.606669] # Totals: pass:3 fail:1 skip:0 total:4
[   30.608618] not ok 1 longest-symbol

Seems the symbol name is found when run as modules?

Thanks!
-Rae

> ---
> V1 -> V2: corrected CI tests. Added fix proposed at [3]
>
> [3] https://lore.kernel.org/lkml/Y9ES4UKl%2F+DtvAVS@xxxxxxxxx/T/#m3ef0e12bb834d01ed1ebdcae12ef5f2add342077
> ---
> V2 -> V3: updated base and added MODULE_DESCRIPTION() and MODULE_AUTHOR()
> ---
> V3 -> V4: add x86 mantainers, add new reference.
> ---
> V4 -> V5: fixed typo, added improved description
>
> The test execution shoud 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] test_longest_symbol_plus1
> [20:04:35] [PASSED] test_longest_symbol_plus1_kallsyms
> [20:04:35] ================= [PASSED] longest-symbol ==================
> [20:04:35] ============================================================
> [20:04:35] Testing complete. Ran 4 tests: passed: 4
> ```
>
> But also there are two warnings like:
>
> ```
> Symbol snnnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnnnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7ne too long for kallsyms (512 >= 512).
> Please increase KSYM_NAME_LEN both in kernel and kallsyms.c
> ```
>
> Because the tests try to generate simbols of the same length as
> KSYM_NAME_LEN. This is the expected outcome, as defined:
> https://lore.kernel.org/all/20131023130753.GO29695@xxxxxxxxxxxxxxxxxx/
>
> ---
>  arch/x86/tools/insn_decoder_test.c |   3 +-
>  lib/Kconfig.debug                  |   9 +++
>  lib/Makefile                       |   2 +
>  lib/longest_symbol_kunit.c         | 125 +++++++++++++++++++++++++++++
>  4 files changed, 138 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..3bde35ea4188 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/kallsysms.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 7312ae7c3cc5..1f3059176637 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2820,6 +2820,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..ebd26eedbf7b
> --- /dev/null
> +++ b/lib/longest_symbol_kunit.c
> @@ -0,0 +1,125 @@
> +// 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)
> +
> +#define PLUS1(name) __PASTE(name, e)
> +
> +/*Generate a symbol whose name length is 511 */
> +#define LONGEST_SYM_NAME  DDDDDI(g1h2i3j4k5l6m7n)
> +
> +/*Generate a symbol whose name length is 512 */
> +#define LONGEST_SYM_NAME_PLUS1 PLUS1(LONGEST_SYM_NAME)
> +
> +#define RETURN_LONGEST_SYM 0xAAAAA
> +#define RETURN_LONGEST_SYM_PLUS1 0x55555
> +
> +noinline int LONGEST_SYM_NAME(void);
> +noinline int LONGEST_SYM_NAME(void)
> +{
> +       return RETURN_LONGEST_SYM;
> +}
> +
> +noinline int LONGEST_SYM_NAME_PLUS1(void);
> +noinline int LONGEST_SYM_NAME_PLUS1(void)
> +{
> +       return RETURN_LONGEST_SYM_PLUS1;
> +}
> +
> +_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)));
> +
> +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 kallsysms: 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 void test_longest_symbol_plus1(struct kunit *test)
> +{
> +       KUNIT_EXPECT_EQ(test, RETURN_LONGEST_SYM_PLUS1, LONGEST_SYM_NAME_PLUS1());
> +};
> +
> +static void test_longest_symbol_plus1_kallsyms(struct kunit *test)
> +{
> +       unsigned long (*kallsyms_lookup_name)(const char *name);
> +       static int (*longest_sym_plus1)(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 kallsysms: kprobe not registered\n");
> +               return;
> +       }
> +
> +       kunit_warn(test, "test_longest_symbol_plus1 kallsyms: kprobe registered\n");
> +       kallsyms_lookup_name = (unsigned long (*)(const char *name))kp.addr;
> +       unregister_kprobe(&kp);
> +
> +       longest_sym_plus1 =
> +               (void *) kallsyms_lookup_name(__stringify(LONGEST_SYM_NAME_PLUS1));
> +       KUNIT_EXPECT_NULL(test, longest_sym_plus1);
> +};
> +
> +static struct kunit_case longest_symbol_test_cases[] = {
> +       KUNIT_CASE(test_longest_symbol),
> +       KUNIT_CASE(test_longest_symbol_kallsyms),
> +       KUNIT_CASE(test_longest_symbol_plus1),
> +       KUNIT_CASE(test_longest_symbol_plus1_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
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@xxxxxxxxxxxxxxxx.
> To view this discussion visit https://groups.google.com/d/msgid/kunit-dev/20241117195923.222145-1-sergio.collado%40gmail.com.





[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux