[LTP] [PATCH 1/1] keyctl30: Check module is presented before loading

[LTP] [PATCH 1/1] keyctl30: Check module is presented before loading

[Petr Vorel]

This fixes test on the system without module:

    modprobe: FATAL: Module pkcs8_key_parser not found in directory /usr/lib/modules/6.12.0-160100.5-azure
    tst_cmd.c:112: TBROK: 'modprobe' exited with a non-zero code 1 at tst_cmd.c:112

With the fix it TCONF as expected:

    tst_module.c:72: TCONF: Failed to find module 'pkcs8_key_parser'

Fixes: 1bb448ed9e ("keyctl30: Test KEYCTL_PKEY_QUERY on private key")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
FYI Cyril suggested to add into struct config_module_map:

 	{"CONFIG_NET_SCH_INGRESS", "sch_ingress"},
 	{"CONFIG_NET_CLS_MATCHALL", "cls_matchall"},
 	{"CONFIG_NET_ACT_PEDIT", "act_pedit"},
+	{"CONFIG_PKCS8_PRIVATE_KEY_PARSER", "pcks8_key_parser"},

Although it does not harm this is IMHO just wrong function being used.

 testcases/kernel/syscalls/keyctl/keyctl30.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/keyctl/keyctl30.c b/testcases/kernel/syscalls/keyctl/keyctl30.c
index ef51bba338..5f98d2a7d4 100644
--- a/testcases/kernel/syscalls/keyctl/keyctl30.c
+++ b/testcases/kernel/syscalls/keyctl/keyctl30.c
@@ -29,7 +29,7 @@ static void setup(void)
 {
 	SAFE_KEYCTL(KEYCTL_JOIN_SESSION_KEYRING, 0, 0, 0, 0);
 
-	tst_modprobe("pkcs8_key_parser", NULL);
+	tst_module_load("pkcs8_key_parser", NULL);
 
 	priv_key = add_asymmetric_key_or_tconf("priv", rsa2048_pkcs8,
 					       sizeof(rsa2048_pkcs8),
-- 
2.55.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] keyctl30: Check module is presented before loading

[Petr Vorel]

Hm, we have more tests using this (not only keyctl*.c but also cve-2026-46331.c.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] keyctl30: Check module is presented before loading

[Cyril Hrubis]

Hi!
> This fixes test on the system without module:
> 
>     modprobe: FATAL: Module pkcs8_key_parser not found in directory /usr/lib/modules/6.12.0-160100.5-azure
>     tst_cmd.c:112: TBROK: 'modprobe' exited with a non-zero code 1 at tst_cmd.c:112
> 
> With the fix it TCONF as expected:
> 
>     tst_module.c:72: TCONF: Failed to find module 'pkcs8_key_parser'
> 
> Fixes: 1bb448ed9e ("keyctl30: Test KEYCTL_PKEY_QUERY on private key")
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> FYI Cyril suggested to add into struct config_module_map:
> 
>  	{"CONFIG_NET_SCH_INGRESS", "sch_ingress"},
>  	{"CONFIG_NET_CLS_MATCHALL", "cls_matchall"},
>  	{"CONFIG_NET_ACT_PEDIT", "act_pedit"},
> +	{"CONFIG_PKCS8_PRIVATE_KEY_PARSER", "pcks8_key_parser"},

I think that this is still cleaner, this way we skip the test even
before we start running it. Maybe we should attempt to auto-generate the
table for all CONFIG_* variables used in LTP.

Also keyctl23.c and keyctl29.c have tst_modprobe("x509_key_parser",
NULL); So we need fix/mappping for x509_key_parser too.

> Although it does not harm this is IMHO just wrong function being used.

That depends, if we decide to have the checks in the tst_kconfig.c then
tst_modprobe() is the only function that should be used.

Also looiks like the primary user of the tst_module_load() are tests
that build modules in LTP tree, where it does make sense, since these
may fail to build and be missing and there are no CONFIG_ variables for
these. The rest of the tests seems to be using tst_modprobe().

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] keyctl30: Check module is presented before loading

[Petr Vorel]

> Hi!
> > This fixes test on the system without module:

> >     modprobe: FATAL: Module pkcs8_key_parser not found in directory /usr/lib/modules/6.12.0-160100.5-azure
> >     tst_cmd.c:112: TBROK: 'modprobe' exited with a non-zero code 1 at tst_cmd.c:112

> > With the fix it TCONF as expected:

> >     tst_module.c:72: TCONF: Failed to find module 'pkcs8_key_parser'

> > Fixes: 1bb448ed9e ("keyctl30: Test KEYCTL_PKEY_QUERY on private key")
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > FYI Cyril suggested to add into struct config_module_map:

> >  	{"CONFIG_NET_SCH_INGRESS", "sch_ingress"},
> >  	{"CONFIG_NET_CLS_MATCHALL", "cls_matchall"},
> >  	{"CONFIG_NET_ACT_PEDIT", "act_pedit"},
> > +	{"CONFIG_PKCS8_PRIVATE_KEY_PARSER", "pcks8_key_parser"},

> I think that this is still cleaner, this way we skip the test even
> before we start running it.

Yeah, exit early + having it in the metadata is better - feel free to post the
above as a patch and we refuse this patch.

After release thinking:
Thinking it twice having a separate metadata for "direct" module loading was
good (the functionality we replaced by kconfig). Why? Now we need to
1) load module manually in the test
2) manual module loading is hidden in metadata behind kernel config (tester see
that in the test output, but it's not obvious in the metadata).

Sure, we don't want to revert that whole big patchset. But maybe we could
consider to
1) call tst_module_load() based on config_module_map() instead doing it
in the test
2) print this mapping in the metadata (that would require struct config_module_map
being accessible also by metadata/metaparse.c.

> Maybe we should attempt to auto-generate the
> table for all CONFIG_* variables used in LTP.

Sure, that's a good idea. Useful would be some script similar to updating
sycalls numbers. With the only exception: don't delete old modules (modules can
be deleted, but still used by supported kernels).

> Also keyctl23.c and keyctl29.c have tst_modprobe("x509_key_parser",
> NULL); So we need fix/mappping for x509_key_parser too.

Yeah, I also noted that + cve-2026-46331.c (all Andrea's agent based code,
we should teach agent about the differences).

> > Although it does not harm this is IMHO just wrong function being used.

> That depends, if we decide to have the checks in the tst_kconfig.c then
> tst_modprobe() is the only function that should be used.

Sure, for distro modules. tst_module_load() is still needed for LTP specific
out-of-tree modules (again, we should teach the agent).

> Also looiks like the primary user of the tst_module_load() are tests
> that build modules in LTP tree, where it does make sense, since these
> may fail to build and be missing and there are no CONFIG_ variables for
> these. The rest of the tests seems to be using tst_modprobe().

Exactly.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] keyctl30: Check module is presented before loading

[Cyril Hrubis]

Hi!
> > >  	{"CONFIG_NET_SCH_INGRESS", "sch_ingress"},
> > >  	{"CONFIG_NET_CLS_MATCHALL", "cls_matchall"},
> > >  	{"CONFIG_NET_ACT_PEDIT", "act_pedit"},
> > > +	{"CONFIG_PKCS8_PRIVATE_KEY_PARSER", "pcks8_key_parser"},
> 
> > I think that this is still cleaner, this way we skip the test even
> > before we start running it.
> 
> Yeah, exit early + having it in the metadata is better - feel free to post the
> above as a patch and we refuse this patch.
> 
> After release thinking:
> Thinking it twice having a separate metadata for "direct" module loading was
> good (the functionality we replaced by kconfig). Why? Now we need to
> 1) load module manually in the test
> 2) manual module loading is hidden in metadata behind kernel config (tester see
> that in the test output, but it's not obvious in the metadata).

It's more subtle than this, in most cases kernel actually loads the
module automatically when the functionality is used (e.g. you create a
socket and kernel loads the module with the particular socket support).
The cases where the module needs to be loaded are quite rare. It's never
unloaded though, so maybe we should attempt that too (if it wasn't
loaded and was loaded because of the test).

> Sure, we don't want to revert that whole big patchset. But maybe we could
> consider to
> 1) call tst_module_load() based on config_module_map() instead doing it
> in the test

We can do that, but I would only do so if it's actually needed, so that
the module autoload is tested as well. For that we would probably need
to add a needs_load to the config to module mapping table.

> 2) print this mapping in the metadata (that would require struct config_module_map
> being accessible also by metadata/metaparse.c.
>
> > Maybe we should attempt to auto-generate the
> > table for all CONFIG_* variables used in LTP.
> 
> Sure, that's a good idea. Useful would be some script similar to updating
> sycalls numbers. With the only exception: don't delete old modules (modules can
> be deleted, but still used by supported kernels).

As long as we have complete list of CONFIG_* variables from the tests,
which can be easily extracted from the test, we know which modules we
need mapping for. No need to keep anything that isn't in that set.

> > Also keyctl23.c and keyctl29.c have tst_modprobe("x509_key_parser",
> > NULL); So we need fix/mappping for x509_key_parser too.
> 
> Yeah, I also noted that + cve-2026-46331.c (all Andrea's agent based code,
> we should teach agent about the differences).

There are also tests that do tst_cmd("modprobe", ...) these should be
fixed as well (git grep '"modprobe"').

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] keyctl30: Check module is presented before loading

[Petr Vorel]

> Hi!
> > > >  	{"CONFIG_NET_SCH_INGRESS", "sch_ingress"},
> > > >  	{"CONFIG_NET_CLS_MATCHALL", "cls_matchall"},
> > > >  	{"CONFIG_NET_ACT_PEDIT", "act_pedit"},
> > > > +	{"CONFIG_PKCS8_PRIVATE_KEY_PARSER", "pcks8_key_parser"},

> > > I think that this is still cleaner, this way we skip the test even
> > > before we start running it.

> > Yeah, exit early + having it in the metadata is better - feel free to post the
> > above as a patch and we refuse this patch.

> > After release thinking:
> > Thinking it twice having a separate metadata for "direct" module loading was
> > good (the functionality we replaced by kconfig). Why? Now we need to
> > 1) load module manually in the test
> > 2) manual module loading is hidden in metadata behind kernel config (tester see
> > that in the test output, but it's not obvious in the metadata).

> It's more subtle than this, in most cases kernel actually loads the
> module automatically when the functionality is used (e.g. you create a
> socket and kernel loads the module with the particular socket support).
> The cases where the module needs to be loaded are quite rare. It's never
> unloaded though, so maybe we should attempt that too (if it wasn't
> loaded and was loaded because of the test).

kconfig is more than modules (e.g. module or even kernel core functionality
enabled or disabled). I wonder if it's useful for users to know some module
needs to be presented to be loaded (if not built-in) => bool vs. tristate
kernel config. Maybe not.

Other thing is these few modules which don't support autoloading. (Mostly not
bugs, at least the latest vmw_vsock_virtio_transport not supporting it was meant
to be for testing/debugging [1].). Also the old .needs_drivers documented via
metadata both cases - the modules needed to be loaded manually (e.g. "sctp" in
testcases/cve/sctphantom.c) and autoloaded modules (e.g. "dummy" in
testcases/cve/tcindex01.c or "tun" in testcases/kernel/uevents/uevent02.c or
"quota_v2" in testcases/kernel/syscalls/quotactl/quotactl0*.c).

[1] https://lore.kernel.org/virtualization/acqLo6-IQVBzY3UW@sgarzare-redhat/

> > Sure, we don't want to revert that whole big patchset. But maybe we could
> > consider to
> > 1) call tst_module_load() based on config_module_map() instead doing it
> > in the test

> We can do that, but I would only do so if it's actually needed, so that
> the module autoload is tested as well. For that we would probably need
> to add a needs_load to the config to module mapping table.

I'd also load only these which need it (without autoloading support). And yes,
we would have to mark them because it's not obvious from "struct config_module_map"
which ones needs to be loaded manually.

> > 2) print this mapping in the metadata (that would require struct config_module_map
> > being accessible also by metadata/metaparse.c.

> > > Maybe we should attempt to auto-generate the
> > > table for all CONFIG_* variables used in LTP.

> > Sure, that's a good idea. Useful would be some script similar to updating
> > sycalls numbers. With the only exception: don't delete old modules (modules can
> > be deleted, but still used by supported kernels).

> As long as we have complete list of CONFIG_* variables from the tests,
> which can be easily extracted from the test, we know which modules we
> need mapping for. No need to keep anything that isn't in that set.

Again, not all kconfigs means tristate (module), some bool configs are required
to make sure certain functionality is enabled. Unfortunately we need to check
them one by one with kernel sources to see if they are modules (tristate) or bools.

> > > Also keyctl23.c and keyctl29.c have tst_modprobe("x509_key_parser",
> > > NULL); So we need fix/mappping for x509_key_parser too.

> > Yeah, I also noted that + cve-2026-46331.c (all Andrea's agent based code,
> > we should teach agent about the differences).

> There are also tests that do tst_cmd("modprobe", ...) these should be
> fixed as well (git grep '"modprobe"').

$ git grep  '"modprobe"' testcases/
testcases/kernel/device-drivers/zram/zram03.c:  const char *const cmd_modprobe[] = {"modprobe", "zram", NULL};
testcases/kernel/device-drivers/zram/zram03.c:          {.cmd = "modprobe"},
testcases/kernel/syscalls/keyctl/keyctl05.c:    tst_cmd((const char*[]){"modprobe", MODULE, NULL}, NULL, NULL, 0);
testcases/kernel/syscalls/madvise/madvise11.c:  const char *const cmd_modprobe[] = {"modprobe", HW_MODULE, NULL};
testcases/kernel/syscalls/madvise/madvise11.c:          {.cmd = "modprobe"},
testcases/network/can/cve/can_bcm01.c:  tst_cmd((const char*[]){"modprobe", "vcan", NULL}, NULL, NULL, 0);
testcases/network/can/filter-tests/can_common.h:        CAN_CMD("modprobe", "-r", "vcan");
testcases/network/can/filter-tests/can_common.h:        CAN_CMD("modprobe", "vcan", "echo=1");
testcases/network/can/filter-tests/can_common.h:        CAN_CMD("modprobe", "-r", "vcan");
testcases/network/netstress/netstress.c:                const char * const argv[] = {"modprobe", "dccp_ipv6", NULL};

I remember these. I wanted in the past to add support to tst_modprobe() to pass
module parameters, but it turn out only few tests used them. From this grep it
looks like can_common.h uses module parameters (and kvm tests not listed here,
which use tst_module_reload(), but that's different). But I did not noticed that
other uses could use tst_modprobe() directly.  I can send a patch but it should
be probably merged after the release.

=> back to fixes needed for a release.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] keyctl30: Check module is presented before loading

[Cyril Hrubis]

Hi!
> > It's more subtle than this, in most cases kernel actually loads the
> > module automatically when the functionality is used (e.g. you create a
> > socket and kernel loads the module with the particular socket support).
> > The cases where the module needs to be loaded are quite rare. It's never
> > unloaded though, so maybe we should attempt that too (if it wasn't
> > loaded and was loaded because of the test).
> 
> kconfig is more than modules (e.g. module or even kernel core functionality
> enabled or disabled). I wonder if it's useful for users to know some module
> needs to be presented to be loaded (if not built-in) => bool vs. tristate
> kernel config. Maybe not.

We actually check and only do the module presence if the CONFIG_FOO=m,
if the code is build in the branch that checks for the module existence
never runs.

> Other thing is these few modules which don't support autoloading. (Mostly not
> bugs, at least the latest vmw_vsock_virtio_transport not supporting it was meant
> to be for testing/debugging [1].). Also the old .needs_drivers documented via
> metadata both cases - the modules needed to be loaded manually (e.g. "sctp" in
> testcases/cve/sctphantom.c) and autoloaded modules (e.g. "dummy" in
> testcases/cve/tcindex01.c or "tun" in testcases/kernel/uevents/uevent02.c or
> "quota_v2" in testcases/kernel/syscalls/quotactl/quotactl0*.c).

I do not see the problem here, autoloading is as simple as:

diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
index 46eeb8ec0..5d4e4d248 100644
--- a/lib/tst_kconfig.c
+++ b/lib/tst_kconfig.c
@@ -147,6 +147,7 @@ static void kconfig_runtime_check(struct tst_kconfig_var *var)
 static struct config_module_map {
        const char *config;
        const char *module_name;
+       int autoload;
 } config_module_maps[] = {
        {"CONFIG_KVM", "kvm"},
        {"CONFIG_ZRAM", "zram"},
@@ -165,8 +166,8 @@ static struct config_module_map {
        {"CONFIG_NET_SCH_INGRESS", "sch_ingress"},
        {"CONFIG_NET_CLS_MATCHALL", "cls_matchall"},
        {"CONFIG_NET_ACT_PEDIT", "act_pedit"},
-       {"CONFIG_PKCS8_PRIVATE_KEY_PARSER", "pcks8_key_parser"},
-       {"CONFIG_X509_CERTIFICATE_PARSER", "x509_key_parser"},
+       {"CONFIG_PKCS8_PRIVATE_KEY_PARSER", "pcks8_key_parser", 1},
+       {"CONFIG_X509_CERTIFICATE_PARSER", "x509_key_parser", 1},
        {}
 };

@@ -186,6 +187,11 @@ static void kconfig_module_check(struct tst_kconfig_var *var)
                        var->choice = 'n';
                        return;
                }
+
+               if (config_module_maps[i].autoload) {
+                       tst_res(TINFO, "Autoloading '%s'", config_module_maps[i].module_name);
+                       tst_modprobe(config_module_maps[i].module_name);
+               }
        }
 }

If we want that information to be present in metadata and documentation,
we can render that table from a (generated) json that could be parsed by
the documentation code as well.

I think it's better to have that information the the test library once,
rather than to keep a copy in each test.

> > As long as we have complete list of CONFIG_* variables from the tests,
> > which can be easily extracted from the test, we know which modules we
> > need mapping for. No need to keep anything that isn't in that set.
> 
> Again, not all kconfigs means tristate (module), some bool configs are required
> to make sure certain functionality is enabled. Unfortunately we need to check
> them one by one with kernel sources to see if they are modules (tristate) or bools.

There should be include/config/tristate.conf in the kernel sources once
make defconfig is done in linux kernel sources. The harder part is the
CONFIG_ to module name mapping, for that we would have to grep kernel
Makefiles.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] keyctl30: Check module is presented before loading

[Petr Vorel]

> Hi!
> > > It's more subtle than this, in most cases kernel actually loads the
> > > module automatically when the functionality is used (e.g. you create a
> > > socket and kernel loads the module with the particular socket support).
> > > The cases where the module needs to be loaded are quite rare. It's never
> > > unloaded though, so maybe we should attempt that too (if it wasn't
> > > loaded and was loaded because of the test).

> > kconfig is more than modules (e.g. module or even kernel core functionality
> > enabled or disabled). I wonder if it's useful for users to know some module
> > needs to be presented to be loaded (if not built-in) => bool vs. tristate
> > kernel config. Maybe not.

> We actually check and only do the module presence if the CONFIG_FOO=m,
> if the code is build in the branch that checks for the module existence
> never runs.

Thanks for info, makes sense.

> > Other thing is these few modules which don't support autoloading. (Mostly not
> > bugs, at least the latest vmw_vsock_virtio_transport not supporting it was meant
> > to be for testing/debugging [1].). Also the old .needs_drivers documented via
> > metadata both cases - the modules needed to be loaded manually (e.g. "sctp" in
> > testcases/cve/sctphantom.c) and autoloaded modules (e.g. "dummy" in
> > testcases/cve/tcindex01.c or "tun" in testcases/kernel/uevents/uevent02.c or
> > "quota_v2" in testcases/kernel/syscalls/quotactl/quotactl0*.c).

> I do not see the problem here, autoloading is as simple as:

> diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
> index 46eeb8ec0..5d4e4d248 100644
> --- a/lib/tst_kconfig.c
> +++ b/lib/tst_kconfig.c
> @@ -147,6 +147,7 @@ static void kconfig_runtime_check(struct tst_kconfig_var *var)
>  static struct config_module_map {
>         const char *config;
>         const char *module_name;
> +       int autoload;
>  } config_module_maps[] = {
>         {"CONFIG_KVM", "kvm"},
>         {"CONFIG_ZRAM", "zram"},
> @@ -165,8 +166,8 @@ static struct config_module_map {
>         {"CONFIG_NET_SCH_INGRESS", "sch_ingress"},
>         {"CONFIG_NET_CLS_MATCHALL", "cls_matchall"},
>         {"CONFIG_NET_ACT_PEDIT", "act_pedit"},
> -       {"CONFIG_PKCS8_PRIVATE_KEY_PARSER", "pcks8_key_parser"},
> -       {"CONFIG_X509_CERTIFICATE_PARSER", "x509_key_parser"},
> +       {"CONFIG_PKCS8_PRIVATE_KEY_PARSER", "pcks8_key_parser", 1},
> +       {"CONFIG_X509_CERTIFICATE_PARSER", "x509_key_parser", 1},
>         {}
>  };

> @@ -186,6 +187,11 @@ static void kconfig_module_check(struct tst_kconfig_var *var)
>                         var->choice = 'n';
>                         return;
>                 }
> +
> +               if (config_module_maps[i].autoload) {
> +                       tst_res(TINFO, "Autoloading '%s'", config_module_maps[i].module_name);
> +                       tst_modprobe(config_module_maps[i].module_name);
> +               }
>         }
>  }

+1 (unless autoload is really module specific, not test specific).


will you send a patch after the release? Feel free to ask me to finish it
(find which ones are needed).

> If we want that information to be present in metadata and documentation,
> we can render that table from a (generated) json that could be parsed by
> the documentation code as well.

+1

> I think it's better to have that information the the test library once,
> rather than to keep a copy in each test.

+1, I really hope that this is not test specific (2 tests somehow using a single
module, one with a way that autoload works on other use case not). It would be
very rare case, let's ignore it now (again that vmw_vsock_virtio_transport which
is loaded by vsock only [1] shows that autoloading can fail).

[1] https://github.com/bluca/systemd/commit/7b435741ba7667cc14aa25d22309da703e0cbf33

> > > As long as we have complete list of CONFIG_* variables from the tests,
> > > which can be easily extracted from the test, we know which modules we
> > > need mapping for. No need to keep anything that isn't in that set.

> > Again, not all kconfigs means tristate (module), some bool configs are required
> > to make sure certain functionality is enabled. Unfortunately we need to check
> > them one by one with kernel sources to see if they are modules (tristate) or bools.

> There should be include/config/tristate.conf in the kernel sources once
> make defconfig is done in linux kernel sources.

New thing to me, but already removed 6 years ago in v5.6-rc1. I indeed had
include/config/tristate.conf with old kernel (leftover of debugging 4.4 based
kernel). And Masahiro Yamada mentioned about false positives in it :).

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8b41fc4454e36fbfdbb23f940d023d4dece2de29

> The harder part is the
> CONFIG_ to module name mapping, for that we would have to grep kernel
> Makefiles.

Exactly.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp