Message ID | 20220701084744.3002019-4-davidgow@google.com |
---|---|
State | New |
Headers | show |
Series | [v4,1/4] panic: Taint kernel if tests are run | expand |
Hi David, I love your patch! Yet something to improve: [auto build test ERROR on masahiroy-kbuild/for-next] [also build test ERROR on shuah-kselftest/next linus/master v5.19-rc4 next-20220701] [cannot apply to mcgrof/modules-next] [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] url: https://github.com/intel-lab-lkp/linux/commits/David-Gow/panic-Taint-kernel-if-tests-are-run/20220701-164843 base: https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next config: alpha-randconfig-r006-20220629 (https://download.01.org/0day-ci/archive/20220702/202207020132.SKDpQP9D-lkp@intel.com/config) compiler: alpha-linux-gcc (GCC) 11.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/42b6461d6cca4baeeeed474b1400e203057c2b9b git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review David-Gow/panic-Taint-kernel-if-tests-are-run/20220701-164843 git checkout 42b6461d6cca4baeeeed474b1400e203057c2b9b # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=alpha SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): In file included from lib/test_printf.c:27: lib/test_printf.c: In function 'test_printf_init': >> lib/../tools/testing/selftests/kselftest_module.h:45:19: error: 'TAINT_KUNIT' undeclared (first use in this function) 45 | add_taint(TAINT_KUNIT, LOCKDEP_STILL_OK); \ | ^~~~~~~~~~~ lib/test_printf.c:801:1: note: in expansion of macro 'KSTM_MODULE_LOADERS' 801 | KSTM_MODULE_LOADERS(test_printf); | ^~~~~~~~~~~~~~~~~~~ lib/../tools/testing/selftests/kselftest_module.h:45:19: note: each undeclared identifier is reported only once for each function it appears in 45 | add_taint(TAINT_KUNIT, LOCKDEP_STILL_OK); \ | ^~~~~~~~~~~ lib/test_printf.c:801:1: note: in expansion of macro 'KSTM_MODULE_LOADERS' 801 | KSTM_MODULE_LOADERS(test_printf); | ^~~~~~~~~~~~~~~~~~~ vim +/TAINT_KUNIT +45 lib/../tools/testing/selftests/kselftest_module.h 40 41 #define KSTM_MODULE_LOADERS(__module) \ 42 static int __init __module##_init(void) \ 43 { \ 44 pr_info("loaded.\n"); \ > 45 add_taint(TAINT_KUNIT, LOCKDEP_STILL_OK); \ 46 selftest(); \ 47 return kstm_report(total_tests, failed_tests, skipped_tests); \ 48 } \ 49 static void __exit __module##_exit(void) \ 50 { \ 51 pr_info("unloaded.\n"); \ 52 } \ 53 module_init(__module##_init); \ 54 module_exit(__module##_exit) 55
On Fri, Jul 01, 2022 at 04:47:44PM +0800, David Gow wrote: > Make any kselftest test module (using the kselftest_module framework) > taint the kernel with TAINT_TEST on module load. > > Note that several selftests use kernel modules which are not based on > the kselftest_module framework, and so will not automatically taint the > kernel. > > This can be done in two ways: > - Moving the module to the tools/testing directory. All modules under > this directory will taint the kernel. > - Adding the 'test' module property with: > MODULE_INFO(test, "Y") This just needs to be documented somewhere other than a commit log. Otherwise I am not sure how we can be sure it will catch on. > Similarly, selftests which do not load modules into the kernel generally > should not taint the kernel (or possibly should only do so on failure), > as it's assumed that testing from user-space should be safe. Regardless, > they can write to /proc/sys/kernel/tainted if required. > > Signed-off-by: David Gow <davidgow@google.com> Looks good otherwise! Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> Do we want this to go through selftest / kunit / modules tree? Happy for it to through any. I can't predict a conflict. Luis
On Sat, Jul 2, 2022 at 6:33 AM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Fri, Jul 01, 2022 at 04:47:44PM +0800, David Gow wrote: > > Make any kselftest test module (using the kselftest_module framework) > > taint the kernel with TAINT_TEST on module load. > > > > Note that several selftests use kernel modules which are not based on > > the kselftest_module framework, and so will not automatically taint the > > kernel. > > > > This can be done in two ways: > > - Moving the module to the tools/testing directory. All modules under > > this directory will taint the kernel. > > - Adding the 'test' module property with: > > MODULE_INFO(test, "Y") > > This just needs to be documented somewhere other than a commit log. > Otherwise I am not sure how we can be sure it will catch on. I've updated the kselftest documentation for v5. > > Similarly, selftests which do not load modules into the kernel generally > > should not taint the kernel (or possibly should only do so on failure), > > as it's assumed that testing from user-space should be safe. Regardless, > > they can write to /proc/sys/kernel/tainted if required. > > > > Signed-off-by: David Gow <davidgow@google.com> > > Looks good otherwise! > > Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> > > Do we want this to go through selftest / kunit / modules tree? > Happy for it to through any. I can't predict a conflict. I don't mind which tree it goes through either -- I'm not aware of anything which would depend on it. I do have it on the list of things pending for the KUnit tree, but it's much less KUnit-specific now compared to v1. Regardless, I'll leave in the KUnit to-do list, and we'll pick it up if no-one else particularly wants to. Cheers, -- David
On Sat, Jul 2, 2022 at 12:06 PM David Gow <davidgow@google.com> wrote: > > On Sat, Jul 2, 2022 at 6:33 AM Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > On Fri, Jul 01, 2022 at 04:47:44PM +0800, David Gow wrote: > > > Make any kselftest test module (using the kselftest_module framework) > > > taint the kernel with TAINT_TEST on module load. > > > > > > Note that several selftests use kernel modules which are not based on > > > the kselftest_module framework, and so will not automatically taint the > > > kernel. > > > > > > This can be done in two ways: > > > - Moving the module to the tools/testing directory. All modules under > > > this directory will taint the kernel. > > > - Adding the 'test' module property with: > > > MODULE_INFO(test, "Y") > > > > This just needs to be documented somewhere other than a commit log. > > Otherwise I am not sure how we can be sure it will catch on. > > I've updated the kselftest documentation for v5. > > > > Similarly, selftests which do not load modules into the kernel generally > > > should not taint the kernel (or possibly should only do so on failure), > > > as it's assumed that testing from user-space should be safe. Regardless, > > > they can write to /proc/sys/kernel/tainted if required. > > > > > > Signed-off-by: David Gow <davidgow@google.com> > > > > Looks good otherwise! > > > > Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> > > > > Do we want this to go through selftest / kunit / modules tree? > > Happy for it to through any. I can't predict a conflict. > > I don't mind which tree it goes through either -- I'm not aware of > anything which would depend on it. I do have it on the list of things > pending for the KUnit tree, but it's much less KUnit-specific now > compared to v1. Regardless, I'll leave in the KUnit to-do list, and > we'll pick it up if no-one else particularly wants to. > FYI: It looks like patches 1 & 3 are already in the kunit tree, so it makes sense to take the rest of them, too: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/log/?h=kunit Cheers, -- David
diff --git a/tools/testing/selftests/kselftest_module.h b/tools/testing/selftests/kselftest_module.h index e2ea41de3f35..226e616b82e0 100644 --- a/tools/testing/selftests/kselftest_module.h +++ b/tools/testing/selftests/kselftest_module.h @@ -3,6 +3,7 @@ #define __KSELFTEST_MODULE_H #include <linux/module.h> +#include <linux/panic.h> /* * Test framework for writing test modules to be loaded by kselftest. @@ -41,6 +42,7 @@ static inline int kstm_report(unsigned int total_tests, unsigned int failed_test static int __init __module##_init(void) \ { \ pr_info("loaded.\n"); \ + add_taint(TAINT_KUNIT, LOCKDEP_STILL_OK); \ selftest(); \ return kstm_report(total_tests, failed_tests, skipped_tests); \ } \
Make any kselftest test module (using the kselftest_module framework) taint the kernel with TAINT_TEST on module load. Note that several selftests use kernel modules which are not based on the kselftest_module framework, and so will not automatically taint the kernel. This can be done in two ways: - Moving the module to the tools/testing directory. All modules under this directory will taint the kernel. - Adding the 'test' module property with: MODULE_INFO(test, "Y") Similarly, selftests which do not load modules into the kernel generally should not taint the kernel (or possibly should only do so on failure), as it's assumed that testing from user-space should be safe. Regardless, they can write to /proc/sys/kernel/tainted if required. Signed-off-by: David Gow <davidgow@google.com> --- This still only covers a subset of selftest modules, but combined with the modpost check for the tools/testing path, it should catch many future tests. Others can be moved, adapted to use this framework, or have MODULE_INFO(test, "Y") added. (Alas, I don't have the time to hunt down all of the tests which don't do this at the moment. No changes since v3: https://lore.kernel.org/lkml/20220513083212.3537869-3-davidgow@google.com/ --- tools/testing/selftests/kselftest_module.h | 2 ++ 1 file changed, 2 insertions(+)