| Message ID | 20220701084744.3002019-2-davidgow@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4,1/4] panic: Taint kernel if tests are run | expand |
On Fri, Jul 01, 2022 at 04:47:42PM +0800, David Gow wrote: > Taint the kernel with TAINT_TEST whenever a test module loads, by adding > a new "TEST" module property, and setting it for all modules in the > tools/testing directory. This property can also be set manually, for > tests which live outside the tools/testing directory with: > MODULE_INFO(test, "Y"); > > Signed-off-by: David Gow <davidgow@google.com> > --- > > This patch is new in v4 of this series. > > --- > kernel/module/main.c | 8 ++++++++ > scripts/mod/modpost.c | 3 +++ > 2 files changed, 11 insertions(+) > > diff --git a/kernel/module/main.c b/kernel/module/main.c > index fed58d30725d..f2ca0a3ee5e6 100644 > --- a/kernel/module/main.c > +++ b/kernel/module/main.c > @@ -1988,6 +1988,14 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags) > /* Set up license info based on the info section */ > set_license(mod, get_modinfo(info, "license")); > > + if (!get_modinfo(info, "test")) { > + if (!test_taint(TAINT_TEST)) > + pr_warn("%s: loading test module taints kernel.\n", > + mod->name); > + add_taint_module(mod, TAINT_TEST, LOCKDEP_STILL_OK); > + } > + > + Why 2 blank lines? thanks, greg k-h
On Fri, Jul 1, 2022 at 4:55 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Fri, Jul 01, 2022 at 04:47:42PM +0800, David Gow wrote: > > Taint the kernel with TAINT_TEST whenever a test module loads, by adding > > a new "TEST" module property, and setting it for all modules in the > > tools/testing directory. This property can also be set manually, for > > tests which live outside the tools/testing directory with: > > MODULE_INFO(test, "Y"); > > > > Signed-off-by: David Gow <davidgow@google.com> > > --- > > > > This patch is new in v4 of this series. > > > > --- > > kernel/module/main.c | 8 ++++++++ > > scripts/mod/modpost.c | 3 +++ > > 2 files changed, 11 insertions(+) > > > > diff --git a/kernel/module/main.c b/kernel/module/main.c > > index fed58d30725d..f2ca0a3ee5e6 100644 > > --- a/kernel/module/main.c > > +++ b/kernel/module/main.c > > @@ -1988,6 +1988,14 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags) > > /* Set up license info based on the info section */ > > set_license(mod, get_modinfo(info, "license")); > > > > + if (!get_modinfo(info, "test")) { > > + if (!test_taint(TAINT_TEST)) > > + pr_warn("%s: loading test module taints kernel.\n", > > + mod->name); > > + add_taint_module(mod, TAINT_TEST, LOCKDEP_STILL_OK); > > + } > > + > > + > > Why 2 blank lines? > > thanks, > > greg k-h Whoops: not sure where those came from. I've removed the extra newline locally: it'll be gone in the next revision. Cheers, -- David
On Fri, Jul 01, 2022 at 04:47:42PM +0800, David Gow wrote: > Taint the kernel with TAINT_TEST whenever a test module loads, by adding > a new "TEST" module property, and setting it for all modules in the > tools/testing directory. This property can also be set manually, for > tests which live outside the tools/testing directory with: > MODULE_INFO(test, "Y"); > > Signed-off-by: David Gow <davidgow@google.com> > --- > > This patch is new in v4 of this series. > > --- > kernel/module/main.c | 8 ++++++++ > scripts/mod/modpost.c | 3 +++ > 2 files changed, 11 insertions(+) > > diff --git a/kernel/module/main.c b/kernel/module/main.c > index fed58d30725d..f2ca0a3ee5e6 100644 > --- a/kernel/module/main.c > +++ b/kernel/module/main.c > @@ -1988,6 +1988,14 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags) > /* Set up license info based on the info section */ > set_license(mod, get_modinfo(info, "license")); > > + if (!get_modinfo(info, "test")) { > + if (!test_taint(TAINT_TEST)) > + pr_warn("%s: loading test module taints kernel.\n", > + mod->name); That seems pretty chatty, maybe just pr_warn_once() and make indicate which is the first one? For kernel builds where their goal is to just loop testing this will grow the kernel log without not much need. Luis > + add_taint_module(mod, TAINT_TEST, LOCKDEP_STILL_OK); > + } > + > + > return 0; > } > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index 29d5a841e215..5937212b4433 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -2191,6 +2191,9 @@ static void add_header(struct buffer *b, struct module *mod) > > if (strstarts(mod->name, "drivers/staging")) > buf_printf(b, "\nMODULE_INFO(staging, \"Y\");\n"); > + > + if (strstarts(mod->name, "tools/testing")) > + buf_printf(b, "\nMODULE_INFO(test, \"Y\");\n"); Otherwise looks good, thanks for doing this! Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> Luis
On Sat, Jul 2, 2022 at 6:30 AM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Fri, Jul 01, 2022 at 04:47:42PM +0800, David Gow wrote: > > Taint the kernel with TAINT_TEST whenever a test module loads, by adding > > a new "TEST" module property, and setting it for all modules in the > > tools/testing directory. This property can also be set manually, for > > tests which live outside the tools/testing directory with: > > MODULE_INFO(test, "Y"); > > > > Signed-off-by: David Gow <davidgow@google.com> > > --- > > > > This patch is new in v4 of this series. > > > > --- > > kernel/module/main.c | 8 ++++++++ > > scripts/mod/modpost.c | 3 +++ > > 2 files changed, 11 insertions(+) > > > > diff --git a/kernel/module/main.c b/kernel/module/main.c > > index fed58d30725d..f2ca0a3ee5e6 100644 > > --- a/kernel/module/main.c > > +++ b/kernel/module/main.c > > @@ -1988,6 +1988,14 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags) > > /* Set up license info based on the info section */ > > set_license(mod, get_modinfo(info, "license")); > > > > + if (!get_modinfo(info, "test")) { > > + if (!test_taint(TAINT_TEST)) > > + pr_warn("%s: loading test module taints kernel.\n", > > + mod->name); > > That seems pretty chatty, maybe just pr_warn_once() and make indicate > which is the first one? For kernel builds where their goal is to just > loop testing this will grow the kernel log without not much need. Fair enough: while the other taints (intree, staging) do warn on every module load, it is less likely people will be loading lots of them (and then possibly looking at the log for test results). I'll change this in v5. -- David
diff --git a/kernel/module/main.c b/kernel/module/main.c index fed58d30725d..f2ca0a3ee5e6 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1988,6 +1988,14 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags) /* Set up license info based on the info section */ set_license(mod, get_modinfo(info, "license")); + if (!get_modinfo(info, "test")) { + if (!test_taint(TAINT_TEST)) + pr_warn("%s: loading test module taints kernel.\n", + mod->name); + add_taint_module(mod, TAINT_TEST, LOCKDEP_STILL_OK); + } + + return 0; } diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 29d5a841e215..5937212b4433 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -2191,6 +2191,9 @@ static void add_header(struct buffer *b, struct module *mod) if (strstarts(mod->name, "drivers/staging")) buf_printf(b, "\nMODULE_INFO(staging, \"Y\");\n"); + + if (strstarts(mod->name, "tools/testing")) + buf_printf(b, "\nMODULE_INFO(test, \"Y\");\n"); } static void add_exported_symbols(struct buffer *buf, struct module *mod)
Taint the kernel with TAINT_TEST whenever a test module loads, by adding a new "TEST" module property, and setting it for all modules in the tools/testing directory. This property can also be set manually, for tests which live outside the tools/testing directory with: MODULE_INFO(test, "Y"); Signed-off-by: David Gow <davidgow@google.com> --- This patch is new in v4 of this series. --- kernel/module/main.c | 8 ++++++++ scripts/mod/modpost.c | 3 +++ 2 files changed, 11 insertions(+)