Re: [LTP] lib: add module is loaded check

[Li Wang <li.wang@linux.dev>]

linuxtestproject.agent@gmail.com wrote:

> --- [PATCH 1/4] ---
> 
> > diff --git a/include/tst_module.h b/include/tst_module.h
> > +bool tst_module_is_loaded(const char *mod_name);
> 
> Low: Add kernel-doc for this new public API, including mod_name and the
> return value. Document normalization, built-in modules, and the current
> false return when /proc/modules cannot be read.

No need, funtion name is clear enough.

> --- [PATCH 2/4] ---
> 
> > diff --git a/include/tst_kconfig.h b/include/tst_kconfig.h
> > + * Called from do_test_cleanup() so that the modules are released when
> > + * the test finishes, regardless of the test result.
> 
> Low: The actual caller is do_cleanup(), in the library parent process,
> not do_test_cleanup(), which invokes the test's cleanup callback.
> Correct the lifecycle description.

No. We would do the unload in the lib-cleanup but not test_cleanup.

> > diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
> > +#define MAX_LOADED_MODULES 16
> > +	if (loaded_module_count < MAX_LOADED_MODULES)
> > +		loaded_modules[loaded_module_count++] = mod;
> 
> Medium: This commit already has 19 mappings, so a valid requirement list
> with all 19 available as modules passes the config check but queues only
> 16, silently omitting three required loads. Size the storage from the
> mapping table or allocate enough space instead of discarding entries.

True in theory, but rarely happens in practice. In LTP tests, I haven't
seen any test load more than 16 modules.

But to be on safe side, let's use 32.

> >  				kconfig_module_check(&vars[i]);
> > +				kconfig_module_record(vars[i].id);
> 
> Medium: kconfig_module_check() can change choice to 'n' for an uninstalled
> module, but this still queues it. For CONFIG_MMU | CONFIG_CAN_RAW with
> MMU=y and an uninstalled CAN_RAW=m, the expression succeeds, then loading
> can-raw aborts the otherwise runnable test. Queue only configs that
> remain enabled after checking, without making optional branches mandatory.

Good point, let's adding back v1 check:

	kconfig_runtime_check(&vars[i]);
 	kconfig_module_check(&vars[i]);
+	if (vars[i].choice == 'm')
+		kconfig_module_record(vars[i].id);

 
> > +	for (i = 0; i < loaded_module_count; i++) {
> > +		tst_res(TDEBUG, "Loading module '%s'", loaded_modules[i]);
> > +		tst_modprobe(loaded_modules[i], NULL);
> > +	}
> > +
> > +	modules_loaded = true;
> 
> High: If one modprobe succeeds and a later one aborts, modules_loaded is
> still false, so the cleanup helper returns without unloading the earlier
> module. Track each successful load immediately and let abort cleanup
> unload that successful prefix, not unattempted entries.

No. tst_modprobe() never calls tst_brk() internally, so a failed load
doesn't trigger the abort path at all, execution just continues.

> > +		const char *const argv[] = { "rmmod", loaded_modules[i - 1], NULL };
> > +		tst_cmd(argv, "/dev/null", "/dev/null", TST_CMD_PASS_RETVAL);
> 
> Medium: modprobe also inserts dependencies, but this removes only the
> explicitly recorded names. With CONFIG_CAN=m and CONFIG_CAN_RAW=m,
> loading can-raw also loads can; removing can-raw leaves the newly loaded,
> unused can module behind. Track newly loaded dependencies and remove
> only test-owned modules in dependency order.

Ok, maybe we can consider `modprobe -r`.

 
> --- [PATCH 3/4] ---
> 
> > The test also needs root to load the modules and to run as the
> > vsock_loopback transport is not available from an unprivileged user
> > namespace.
> > diff --git a/testcases/cve/cve-2025-21756.c b/testcases/cve/cve-2025-21756.c
> > +	.needs_root = 1,
> 
> Low: Root is needed for module insertion, but loopback VSOCK on
> nonprivileged ports in the initial namespaces does not require root;
> an ordinary nonroot process is not an unprivileged user namespace.
> Correct the commit message and document the loading-only requirement in
> the test description. Drop privileges for the reproducer after the
> framework has loaded the modules.

Ok.

-- 
Regards,
Li Wang

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