Re: [PATCH 2/4] selftests: livepatch: Remove leftover modules when a testcase fails

[sashiko-bot@kernel.org]

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 <mpdesouza@suse.com>

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/20260524-livepatch-unload-on-fail-v1-0-7465de7f741d@suse.com?part=2