kunit: run test suites only after module initialization completes
Commit2810c1e998
("kunit: Fix wild-memory-access bug in kunit_free_suite_set()") fixed a wild-memory-access bug that could have happened during the loading phase of test suites built and executed as loadable modules. However, it also introduced a problematic side effect that causes test suites modules to crash when they attempt to register fake devices. When a module is loaded, it traverses the MODULE_STATE_UNFORMED and MODULE_STATE_COMING states before reaching the normal operating state MODULE_STATE_LIVE. Finally, when the module is removed, it moves to MODULE_STATE_GOING before being released. However, if the loading function load_module() fails between complete_formation() and do_init_module(), the module goes directly from MODULE_STATE_COMING to MODULE_STATE_GOING without passing through MODULE_STATE_LIVE. This behavior was causing kunit_module_exit() to be called without having first executed kunit_module_init(). Since kunit_module_exit() is responsible for freeing the memory allocated by kunit_module_init() through kunit_filter_suites(), this behavior was resulting in a wild-memory-access bug. Commit2810c1e998
("kunit: Fix wild-memory-access bug in kunit_free_suite_set()") fixed this issue by running the tests when the module is still in MODULE_STATE_COMING. However, modules in that state are not fully initialized, lacking sysfs kobjects. Therefore, if a test module attempts to register a fake device, it will inevitably crash. This patch proposes a different approach to fix the original wild-memory-access bug while restoring the normal module execution flow by making kunit_module_exit() able to detect if kunit_module_init() has previously initialized the tests suite set. In this way, test modules can once again register fake devices without crashing. This behavior is achieved by checking whether mod->kunit_suites is a virtual or direct mapping address. If it is a virtual address, then kunit_module_init() has allocated the suite_set in kunit_filter_suites() using kmalloc_array(). On the contrary, if mod->kunit_suites is still pointing to the original address that was set when looking up the .kunit_test_suites section of the module, then the loading phase has failed and there's no memory to be freed. v4: - rebased on 6.8 - noted that kunit_filter_suites() must return a virtual address v3: - add a comment to clarify why the start address is checked v2: - add include <linux/mm.h> Fixes:2810c1e998
("kunit: Fix wild-memory-access bug in kunit_free_suite_set()") Reviewed-by: David Gow <davidgow@google.com> Tested-by: Rae Moar <rmoar@google.com> Tested-by: Richard Fitzgerald <rf@opensource.cirrus.com> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> Signed-off-by: Marco Pagani <marpagan@redhat.com> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
This commit is contained in:
parent
57e39086fb
commit
a1af6a2bfa
@ -146,6 +146,10 @@ void kunit_free_suite_set(struct kunit_suite_set suite_set)
|
||||
kfree(suite_set.start);
|
||||
}
|
||||
|
||||
/*
|
||||
* Filter and reallocate test suites. Must return the filtered test suites set
|
||||
* allocated at a valid virtual address or NULL in case of error.
|
||||
*/
|
||||
struct kunit_suite_set
|
||||
kunit_filter_suites(const struct kunit_suite_set *suite_set,
|
||||
const char *filter_glob,
|
||||
|
@ -17,6 +17,7 @@
|
||||
#include <linux/panic.h>
|
||||
#include <linux/sched/debug.h>
|
||||
#include <linux/sched.h>
|
||||
#include <linux/mm.h>
|
||||
|
||||
#include "debugfs.h"
|
||||
#include "device-impl.h"
|
||||
@ -801,12 +802,19 @@ static void kunit_module_exit(struct module *mod)
|
||||
};
|
||||
const char *action = kunit_action();
|
||||
|
||||
/*
|
||||
* Check if the start address is a valid virtual address to detect
|
||||
* if the module load sequence has failed and the suite set has not
|
||||
* been initialized and filtered.
|
||||
*/
|
||||
if (!suite_set.start || !virt_addr_valid(suite_set.start))
|
||||
return;
|
||||
|
||||
if (!action)
|
||||
__kunit_test_suites_exit(mod->kunit_suites,
|
||||
mod->num_kunit_suites);
|
||||
|
||||
if (suite_set.start)
|
||||
kunit_free_suite_set(suite_set);
|
||||
kunit_free_suite_set(suite_set);
|
||||
}
|
||||
|
||||
static int kunit_module_notify(struct notifier_block *nb, unsigned long val,
|
||||
@ -816,12 +824,12 @@ static int kunit_module_notify(struct notifier_block *nb, unsigned long val,
|
||||
|
||||
switch (val) {
|
||||
case MODULE_STATE_LIVE:
|
||||
kunit_module_init(mod);
|
||||
break;
|
||||
case MODULE_STATE_GOING:
|
||||
kunit_module_exit(mod);
|
||||
break;
|
||||
case MODULE_STATE_COMING:
|
||||
kunit_module_init(mod);
|
||||
break;
|
||||
case MODULE_STATE_UNFORMED:
|
||||
break;
|
||||
|
Loading…
Reference in New Issue
Block a user