Re: [LTP] [PATCH 0/4] Add .modprobe (loading modules in C API)

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Petr Vorel <pvorel@suse.cz> writes:

> Hi,
>
> when I started this patch it looked to me that quite a few C tests executes
> modprobe to load kernel module. In the end, so far only can_bcm01.c and
> madvise11.c uses it. But maybe more could be used.

Just to be clear this is a great idea in principle.

What concerns me is that module detection and loading will be
mandatory. Then tests which can work without root, even run in a sandbox,
will require real root.

I want to be able to see if a bug is reachable from inside a container,
embedded system or nsjail without adding /lib/modules to the
environment or recompiling the test.

In particular the module detection is a problem because it stops the
test from running when the module is present, but some file is not. If
it is acceptable for Android to disable this check then it should be
acceptable for any system.

If we make the check "best efforts", then we don't need a special define
for Android either.

>
> If I add support for module parameters (it would be easy to add), it could be
> used also in can_common.h testcases/network/can/filter-tests/.
>
> It could also be used in the old API C tests in testcases/kernel/input (which
> use input_helper.c), of course after we rewrite them to the new API.
>
> Tests which use modprobe but using .modprobe is not usable:
> * kvm_pagefault01.c (more complicated use - it requires checks).
> * zram03.c, zram_lib.sh (too complicated check due /sys/class/zram-control
>   introduced in v4.2-rc1 vs. the old API, but maybe this could be simplified)
> * netstress.c (used only when testing dccp, which is determined by getopts)
>
> But if we move code I put into tst_test.c into separate function, we could be
> unified interface which would be usable for those tests as well.
>
> I haven't added support for parameters (it would be easy to add), atm only
> kvm_pagefault01.c and can_common.h use them.
>
> If is .modprobe (as TST_MODPROBE) supported in shell API, then these could use it:
> * new API shell tests: binfmt_misc_lib.sh, rcu_torture.sh, ip_tests.sh (if we
>   don't delete this test), mpls01.sh, tests which use mpls_lib.sh, tests which
>   use tcp_cc_lib.sh, tc01.sh
> * fou01.sh (new API) needs to load module after getopts, but it could use some
>   unified interface.
> * old API shell tests (after they are rewritten): lock_torture.sh
>
> Few notes on modprobe:
> * Do we even need to remove modules?

IDK, but it could be useful for triggering a double free or counter
underflow etc.

> * should we use "modprobe -r" instead of "rmmod" on cleanup? rmmod is a simple
> program which removes (unloads) a module from the Linux kernel. "modprobe -r"
> handles a dependencies (if more modules loded, they are also removed).

These should probably be configurable. There are different
implementations of these tools (e.g. busybox, toybox etc.). In Toybox
modprobe is in "pending", meanwhile lsmod, insmod and rmmod are complete.

>
> * Network tests use -s on remote (log errors into syslog), I guess we probably
> prefer for general use error on stderr.
>
> Petr Vorel (4):
>   tst_kernel: Add safe_check_driver()
>   lib: Add .modprobe
>   madvise11: Replace .needs_drivers with .modprobe
>   can_bcm01: Move vcan to .modprobe
>
>  doc/C-Test-API.asciidoc                       |  5 ++
>  doc/Test-Writing-Guidelines.asciidoc          |  1 +
>  include/tst_kernel.h                          |  9 +++
>  include/tst_test.h                            |  5 +-
>  lib/tst_kernel.c                              |  6 ++
>  lib/tst_test.c                                | 56 ++++++++++++++++++-
>  testcases/kernel/syscalls/madvise/madvise11.c | 36 +-----------
>  testcases/network/can/cve/can_bcm01.c         | 19 ++++---
>  8 files changed, 90 insertions(+), 47 deletions(-)
>
> -- 
> 2.42.0


-- 
Thank you,
Richard.

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