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

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

 



Very interesting that behaviour. I will inspect what is happening there.
Thanks for pointing that out.

On Thu, 21 Nov 2024 at 22:45, Rae Moar <rmoar@xxxxxxxxxx> wrote:
>
> 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