Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] The cleanup loop iterates over the TEST_MODS array in forward order, violating dependency requirements and causing rmmod to fail. - [Medium] Functions invoked during cleanup call die() on failure, prematurely aborting the EXIT trap and skipping pop_config(). --
commit aafb48b2a54fe2209742378f31fb96583598557a Author: Marcos Paulo de Souza <[email protected]> selftests: livepatch: Remove leftover modules when a testcase fails This commit introduces a cleanup mechanism for livepatch selftests that tracks loaded modules and attempts to remove them if a test fails or exits prematurely. It uses a TEST_MODS array to keep track of dependencies and unloads the remaining modules during an EXIT trap. > diff --git a/tools/testing/selftests/livepatch/functions.sh > b/tools/testing/selftests/livepatch/functions.sh > index 3ec0b7962fc5c..25f1370038654 100644 > --- a/tools/testing/selftests/livepatch/functions.sh > +++ b/tools/testing/selftests/livepatch/functions.sh [ ... ] > @@ -125,6 +127,14 @@ function set_ftrace_enabled() { > } > > function cleanup() { > + # Remove leftover modules in reverse order to handle dependencies > + for mod_item in "${TEST_MODS[@]}"; do [Severity: Medium] Does this loop actually iterate in reverse order? The comment states the modules will be removed in reverse order to handle dependencies, but ${TEST_MODS[@]} expands the array in ascending index order. Because dependent modules (like livepatches) are loaded after their base modules, iterating in forward order will attempt to unload the base modules first. This would likely cause rmmod to fail with a module in use error. > + if is_livepatch_mod "$mod_item"; then > + disable_lp "$mod_item" > + fi > + _remove_mod "$mod_item" [Severity: Medium] Can this call prematurely abort the cleanup process? The cleanup() function is executed within an EXIT trap. If _remove_mod() fails to unload a module, it calls die() which executes an exit command: tools/testing/selftests/livepatch/functions.sh:_remove_mod() { ... die "failed to unload module $mod (/sys/module)" } Executing exit inside a bash EXIT trap immediately halts the execution of the trap itself. This would abruptly skip the remaining iterations of the module cleanup loop and skip the subsequent pop_config() call. Failing to execute pop_config() leaves the global kernel debug and tracing configurations modified, which could pollute the state of the test system. > + done > + > pop_config > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
