* Re: [PATCH] kunit: run test suites only after module initialization completes
2023-10-16 20:35 [PATCH] kunit: run test suites only after module initialization completes Marco Pagani
@ 2023-10-17 1:28 ` Jinjie Ruan
2023-10-17 14:45 ` Marco Pagani
2023-10-18 7:06 ` kernel test robot
2023-10-18 21:14 ` kernel test robot
2 siblings, 1 reply; 5+ messages in thread
From: Jinjie Ruan @ 2023-10-17 1:28 UTC (permalink / raw)
To: Marco Pagani, Brendan Higgins, David Gow, Rae Moar, Shuah Khan
Cc: linux-kselftest, kunit-dev, linux-kernel
On 2023/10/17 4:35, Marco Pagani wrote:
> Commit 2810c1e99867 ("kunit: Fix wild-memory-access bug in
> kunit_free_suite_set()") is causing all test suites to run (when
> built as modules) while still in MODULE_STATE_COMING. In that state,
> test modules are not fully initialized and lack sysfs kobjects.
> This behavior can cause a crash if the test module tries to register
> fake devices.
>
> This patch restores the normal execution flow, waiting for the module
> initialization to complete before running the test suites.
> The issue reported in the commit mentioned above is addressed using
> virt_addr_valid() to detect if the module loading has failed
> and mod->kunit_suites has not been allocated using kmalloc_array().
>
> Fixes: 2810c1e99867 ("kunit: Fix wild-memory-access bug in kunit_free_suite_set()")
> Signed-off-by: Marco Pagani <marpagan@redhat.com>
> ---
> lib/kunit/test.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 421f13981412..1a49569186fc 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -769,12 +769,14 @@ static void kunit_module_exit(struct module *mod)
> };
> const char *action = kunit_action();
>
> + 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 the module state is from MODULE_STATE_LIVE to MODULE_STATE_GOING, in
kunit_module_init() the kunit_exec_run_tests() is executed when action
is NULL whether kunit_filter_suites() succeeds or not. But in
kunit_module_exit() __kunit_test_suites_exit() will not be executed when
action is NULL if kunit_filter_suites() fails.
>
> - 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,
> @@ -784,12 +786,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;
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] kunit: run test suites only after module initialization completes
2023-10-17 1:28 ` Jinjie Ruan
@ 2023-10-17 14:45 ` Marco Pagani
0 siblings, 0 replies; 5+ messages in thread
From: Marco Pagani @ 2023-10-17 14:45 UTC (permalink / raw)
To: Jinjie Ruan
Cc: Brendan Higgins, David Gow, Rae Moar, Shuah Khan, linux-kselftest,
kunit-dev, linux-kernel
On 2023-10-17 03:28, Jinjie Ruan wrote:
>
>
> On 2023/10/17 4:35, Marco Pagani wrote:
>> Commit 2810c1e99867 ("kunit: Fix wild-memory-access bug in
>> kunit_free_suite_set()") is causing all test suites to run (when
>> built as modules) while still in MODULE_STATE_COMING. In that state,
>> test modules are not fully initialized and lack sysfs kobjects.
>> This behavior can cause a crash if the test module tries to register
>> fake devices.
>>
>> This patch restores the normal execution flow, waiting for the module
>> initialization to complete before running the test suites.
>> The issue reported in the commit mentioned above is addressed using
>> virt_addr_valid() to detect if the module loading has failed
>> and mod->kunit_suites has not been allocated using kmalloc_array().
>>
>> Fixes: 2810c1e99867 ("kunit: Fix wild-memory-access bug in kunit_free_suite_set()")
>> Signed-off-by: Marco Pagani <marpagan@redhat.com>
>> ---
>> lib/kunit/test.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
>> index 421f13981412..1a49569186fc 100644
>> --- a/lib/kunit/test.c
>> +++ b/lib/kunit/test.c
>> @@ -769,12 +769,14 @@ static void kunit_module_exit(struct module *mod)
>> };
>> const char *action = kunit_action();
>>
>> + 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 the module state is from MODULE_STATE_LIVE to MODULE_STATE_GOING, in
> kunit_module_init() the kunit_exec_run_tests() is executed when action
> is NULL whether kunit_filter_suites() succeeds or not.
If kunit_filter_suites() fails in kunit_module_init(), suite_set is
initialized to {0, 0}. Hence, kunit_exec_run_tests() will not execute
the test suites since num_suites = suite_set->end - suite_set->start
equals 0.
> But in kunit_module_exit() __kunit_test_suites_exit() will not be executed when
> action is NULL if kunit_filter_suites() fails.
If kunit_filter_suites() has previously failed in kunit_module_init(),then
kunit_module_exit() will return before calling __kunit_test_suites_exit()
since suite_set.start has previously been set to 0.
>
>>
>> - 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,
>> @@ -784,12 +786,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;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kunit: run test suites only after module initialization completes
2023-10-16 20:35 [PATCH] kunit: run test suites only after module initialization completes Marco Pagani
2023-10-17 1:28 ` Jinjie Ruan
@ 2023-10-18 7:06 ` kernel test robot
2023-10-18 21:14 ` kernel test robot
2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2023-10-18 7:06 UTC (permalink / raw)
To: Marco Pagani, Brendan Higgins, David Gow, Rae Moar, Shuah Khan,
Jinjie Ruan
Cc: oe-kbuild-all, Marco Pagani, linux-kselftest, kunit-dev,
linux-kernel
Hi Marco,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.6-rc6 next-20231017]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Marco-Pagani/kunit-run-test-suites-only-after-module-initialization-completes/20231017-161611
base: linus/master
patch link: https://lore.kernel.org/r/20231016203548.21993-1-marpagan%40redhat.com
patch subject: [PATCH] kunit: run test suites only after module initialization completes
config: csky-randconfig-002-20231018 (https://download.01.org/0day-ci/archive/20231018/202310181220.us7Bvp16-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231018/202310181220.us7Bvp16-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310181220.us7Bvp16-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from arch/csky/include/asm/thread_info.h:9,
from include/linux/thread_info.h:60,
from include/asm-generic/preempt.h:5,
from ./arch/csky/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:79,
from include/linux/spinlock.h:56,
from include/linux/kref.h:16,
from include/kunit/test.h:22,
from include/kunit/resource.h:12,
from lib/kunit/test.c:9:
lib/kunit/test.c: In function 'kunit_module_exit':
>> arch/csky/include/asm/page.h:38:43: error: 'high_memory' undeclared (first use in this function)
38 | (void *)(kaddr) < high_memory)
| ^~~~~~~~~~~
lib/kunit/test.c:772:34: note: in expansion of macro 'virt_addr_valid'
772 | if (!suite_set.start || !virt_addr_valid(suite_set.start))
| ^~~~~~~~~~~~~~~
arch/csky/include/asm/page.h:38:43: note: each undeclared identifier is reported only once for each function it appears in
38 | (void *)(kaddr) < high_memory)
| ^~~~~~~~~~~
lib/kunit/test.c:772:34: note: in expansion of macro 'virt_addr_valid'
772 | if (!suite_set.start || !virt_addr_valid(suite_set.start))
| ^~~~~~~~~~~~~~~
vim +/high_memory +38 arch/csky/include/asm/page.h
013de2d6671d89 Guo Ren 2018-09-05 36
013de2d6671d89 Guo Ren 2018-09-05 37 #define virt_addr_valid(kaddr) ((void *)(kaddr) >= (void *)PAGE_OFFSET && \
013de2d6671d89 Guo Ren 2018-09-05 @38 (void *)(kaddr) < high_memory)
013de2d6671d89 Guo Ren 2018-09-05 39
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] kunit: run test suites only after module initialization completes
2023-10-16 20:35 [PATCH] kunit: run test suites only after module initialization completes Marco Pagani
2023-10-17 1:28 ` Jinjie Ruan
2023-10-18 7:06 ` kernel test robot
@ 2023-10-18 21:14 ` kernel test robot
2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2023-10-18 21:14 UTC (permalink / raw)
To: Marco Pagani, Brendan Higgins, David Gow, Rae Moar, Shuah Khan,
Jinjie Ruan
Cc: oe-kbuild-all, Marco Pagani, linux-kselftest, kunit-dev,
linux-kernel
Hi Marco,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.6-rc6 next-20231018]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Marco-Pagani/kunit-run-test-suites-only-after-module-initialization-completes/20231017-161611
base: linus/master
patch link: https://lore.kernel.org/r/20231016203548.21993-1-marpagan%40redhat.com
patch subject: [PATCH] kunit: run test suites only after module initialization completes
config: arm-randconfig-004-20231019 (https://download.01.org/0day-ci/archive/20231019/202310190514.uKp3bS5Q-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231019/202310190514.uKp3bS5Q-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310190514.uKp3bS5Q-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from arch/arm/include/asm/page.h:188,
from arch/arm/include/asm/thread_info.h:14,
from include/linux/thread_info.h:60,
from include/asm-generic/preempt.h:5,
from ./arch/arm/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:79,
from include/linux/spinlock.h:56,
from include/linux/kref.h:16,
from include/kunit/test.h:22,
from include/kunit/resource.h:12,
from lib/kunit/test.c:9:
lib/kunit/test.c: In function 'kunit_module_exit':
>> arch/arm/include/asm/memory.h:391:116: error: 'high_memory' undeclared (first use in this function)
391 | #define virt_addr_valid(kaddr) (((unsigned long)(kaddr) >= PAGE_OFFSET && (unsigned long)(kaddr) < (unsigned long)high_memory) \
| ^~~~~~~~~~~
lib/kunit/test.c:772:34: note: in expansion of macro 'virt_addr_valid'
772 | if (!suite_set.start || !virt_addr_valid(suite_set.start))
| ^~~~~~~~~~~~~~~
arch/arm/include/asm/memory.h:391:116: note: each undeclared identifier is reported only once for each function it appears in
391 | #define virt_addr_valid(kaddr) (((unsigned long)(kaddr) >= PAGE_OFFSET && (unsigned long)(kaddr) < (unsigned long)high_memory) \
| ^~~~~~~~~~~
lib/kunit/test.c:772:34: note: in expansion of macro 'virt_addr_valid'
772 | if (!suite_set.start || !virt_addr_valid(suite_set.start))
| ^~~~~~~~~~~~~~~
vim +/high_memory +391 arch/arm/include/asm/memory.h
05944d74bc28ff include/asm-arm/memory.h Russell King 2006-11-30 389
e26a9e00afc482 arch/arm/include/asm/memory.h Russell King 2014-03-25 390 #define virt_to_page(kaddr) pfn_to_page(virt_to_pfn(kaddr))
efea3403d4b7c6 arch/arm/include/asm/memory.h Laura Abbott 2013-12-21 @391 #define virt_addr_valid(kaddr) (((unsigned long)(kaddr) >= PAGE_OFFSET && (unsigned long)(kaddr) < (unsigned long)high_memory) \
e26a9e00afc482 arch/arm/include/asm/memory.h Russell King 2014-03-25 392 && pfn_valid(virt_to_pfn(kaddr)))
^1da177e4c3f41 include/asm-arm/memory.h Linus Torvalds 2005-04-16 393
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 5+ messages in thread