[PATCH] tracepoint: Allow trace events in modules with TAINT_TEST

[PATCH] tracepoint: Allow trace events in modules with TAINT_TEST

[alison.schofield]

From: Alison Schofield <alison.schofield@intel.com>

Commit 2852ca7fba9f ("panic: Taint kernel if tests are run")
introduced a new taint type, TAINT_TEST, to signal that an
in-kernel test has been run.

TAINT_TEST taint type defaults into a 'bad_taint' list for
kernel tracing and blocks the creation of trace events. This
causes a problem for CXL testing where loading the cxl_test
module makes all CXL modules out-of-tree, blocking any trace
events.

Trace events are in development for CXL at the moment and this
issue was found in test with v6.0-rc1.

Reported-by: Ira Weiny <ira.weiny@intel.com>
Suggested-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 kernel/tracepoint.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 64ea283f2f86..5f17378c9dc2 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -571,7 +571,8 @@ static void for_each_tracepoint_range(
 bool trace_module_has_bad_taint(struct module *mod)
 {
 	return mod->taints & ~((1 << TAINT_OOT_MODULE) | (1 << TAINT_CRAP) |
-			       (1 << TAINT_UNSIGNED_MODULE));
+			       (1 << TAINT_UNSIGNED_MODULE) |
+			       (1 << TAINT_TEST));
 }
 
 static BLOCKING_NOTIFIER_HEAD(tracepoint_notify_list);

base-commit: 4c612826bec1441214816827979b62f84a097e91
-- 
2.31.1

Re: [PATCH] tracepoint: Allow trace events in modules with TAINT_TEST

[Dave Jiang]

On 8/26/2022 3:35 PM, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
>
> Commit 2852ca7fba9f ("panic: Taint kernel if tests are run")
> introduced a new taint type, TAINT_TEST, to signal that an
> in-kernel test has been run.

has been loaded?

>
> TAINT_TEST taint type defaults into a 'bad_taint' list for
> kernel tracing and blocks the creation of trace events. This
> causes a problem for CXL testing where loading the cxl_test
> module makes all CXL modules out-of-tree, blocking any trace
> events.
>
> Trace events are in development for CXL at the moment and this
> issue was found in test with v6.0-rc1.
>
> Reported-by: Ira Weiny <ira.weiny@intel.com>
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>   kernel/tracepoint.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 64ea283f2f86..5f17378c9dc2 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -571,7 +571,8 @@ static void for_each_tracepoint_range(
>   bool trace_module_has_bad_taint(struct module *mod)
>   {
>   	return mod->taints & ~((1 << TAINT_OOT_MODULE) | (1 << TAINT_CRAP) |
> -			       (1 << TAINT_UNSIGNED_MODULE));
> +			       (1 << TAINT_UNSIGNED_MODULE) |
> +			       (1 << TAINT_TEST));
>   }
>   
>   static BLOCKING_NOTIFIER_HEAD(tracepoint_notify_list);
>
> base-commit: 4c612826bec1441214816827979b62f84a097e91

Re: [PATCH] tracepoint: Allow trace events in modules with TAINT_TEST

[Alison Schofield]

On Fri, Aug 26, 2022 at 03:48:42PM -0700, Dave Jiang wrote:
> 
> On 8/26/2022 3:35 PM, alison.schofield@intel.com wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > Commit 2852ca7fba9f ("panic: Taint kernel if tests are run")
> > introduced a new taint type, TAINT_TEST, to signal that an
> > in-kernel test has been run.
> 
> has been loaded?
> 

I took that explanation directly from the mentioned commit, where
it seems that 'run' was the intended word when referring to KUnit 
tests. Reviewer feedback led the submitter to make the taint per
module to handle 'longer lasting' modules.

> > 
> > TAINT_TEST taint type defaults into a 'bad_taint' list for
> > kernel tracing and blocks the creation of trace events. This
> > causes a problem for CXL testing where loading the cxl_test
> > module makes all CXL modules out-of-tree, blocking any trace
> > events.
> > 
> > Trace events are in development for CXL at the moment and this
> > issue was found in test with v6.0-rc1.
> > 
> > Reported-by: Ira Weiny <ira.weiny@intel.com>
> > Suggested-by: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >   kernel/tracepoint.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> > index 64ea283f2f86..5f17378c9dc2 100644
> > --- a/kernel/tracepoint.c
> > +++ b/kernel/tracepoint.c
> > @@ -571,7 +571,8 @@ static void for_each_tracepoint_range(
> >   bool trace_module_has_bad_taint(struct module *mod)
> >   {
> >   	return mod->taints & ~((1 << TAINT_OOT_MODULE) | (1 << TAINT_CRAP) |
> > -			       (1 << TAINT_UNSIGNED_MODULE));
> > +			       (1 << TAINT_UNSIGNED_MODULE) |
> > +			       (1 << TAINT_TEST));
> >   }
> >   static BLOCKING_NOTIFIER_HEAD(tracepoint_notify_list);
> > 
> > base-commit: 4c612826bec1441214816827979b62f84a097e91

Re: [PATCH] tracepoint: Allow trace events in modules with TAINT_TEST

[Alison Schofield]

+ David Gow - Submitter of the new taint type

On Fri, Aug 26, 2022 at 03:35:11PM -0700, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> Commit 2852ca7fba9f ("panic: Taint kernel if tests are run")
> introduced a new taint type, TAINT_TEST, to signal that an
> in-kernel test has been run.
> 
> TAINT_TEST taint type defaults into a 'bad_taint' list for
> kernel tracing and blocks the creation of trace events. This
> causes a problem for CXL testing where loading the cxl_test
> module makes all CXL modules out-of-tree, blocking any trace
> events.
> 
> Trace events are in development for CXL at the moment and this
> issue was found in test with v6.0-rc1.
> 
> Reported-by: Ira Weiny <ira.weiny@intel.com>
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  kernel/tracepoint.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 64ea283f2f86..5f17378c9dc2 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -571,7 +571,8 @@ static void for_each_tracepoint_range(
>  bool trace_module_has_bad_taint(struct module *mod)
>  {
>  	return mod->taints & ~((1 << TAINT_OOT_MODULE) | (1 << TAINT_CRAP) |
> -			       (1 << TAINT_UNSIGNED_MODULE));
> +			       (1 << TAINT_UNSIGNED_MODULE) |
> +			       (1 << TAINT_TEST));
>  }
>  
>  static BLOCKING_NOTIFIER_HEAD(tracepoint_notify_list);
> 
> base-commit: 4c612826bec1441214816827979b62f84a097e91
> -- 
> 2.31.1
> 

Re: [PATCH] tracepoint: Allow trace events in modules with TAINT_TEST

[David Gow]

On Sat, Aug 27, 2022 at 8:02 AM Alison Schofield
<alison.schofield@intel.com> wrote:
>
> + David Gow - Submitter of the new taint type
>
> On Fri, Aug 26, 2022 at 03:35:11PM -0700, alison.schofield@intel.com wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> >
> > Commit 2852ca7fba9f ("panic: Taint kernel if tests are run")
> > introduced a new taint type, TAINT_TEST, to signal that an
> > in-kernel test has been run.
> >
> > TAINT_TEST taint type defaults into a 'bad_taint' list for
> > kernel tracing and blocks the creation of trace events. This
> > causes a problem for CXL testing where loading the cxl_test
> > module makes all CXL modules out-of-tree, blocking any trace
> > events.
> >
> > Trace events are in development for CXL at the moment and this
> > issue was found in test with v6.0-rc1.
> >
> > Reported-by: Ira Weiny <ira.weiny@intel.com>
> > Suggested-by: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---

This seems fine to me: thanks for catching it. Test modules should be
safe enough to load for this.

Does the comment in tracepoint_module_coming() need updating as well?
https://elixir.bootlin.com/linux/v6.0-rc2/source/kernel/tracepoint.c#L650

Regardless, this is
Reviewed-by: David Gow <davidgow@google.com>

Cheers,
-- David

> >  kernel/tracepoint.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> > index 64ea283f2f86..5f17378c9dc2 100644
> > --- a/kernel/tracepoint.c
> > +++ b/kernel/tracepoint.c
> > @@ -571,7 +571,8 @@ static void for_each_tracepoint_range(
> >  bool trace_module_has_bad_taint(struct module *mod)
> >  {
> >       return mod->taints & ~((1 << TAINT_OOT_MODULE) | (1 << TAINT_CRAP) |
> > -                            (1 << TAINT_UNSIGNED_MODULE));
> > +                            (1 << TAINT_UNSIGNED_MODULE) |
> > +                            (1 << TAINT_TEST));
> >  }
> >
> >  static BLOCKING_NOTIFIER_HEAD(tracepoint_notify_list);
> >
> > base-commit: 4c612826bec1441214816827979b62f84a097e91
> > --
> > 2.31.1
> >

Re: [PATCH] tracepoint: Allow trace events in modules with TAINT_TEST

[Alison Schofield]

On Sat, Aug 27, 2022 at 09:09:54AM +0800, David Gow wrote:
> On Sat, Aug 27, 2022 at 8:02 AM Alison Schofield
> <alison.schofield@intel.com> wrote:
> >
> > + David Gow - Submitter of the new taint type
> >
> > On Fri, Aug 26, 2022 at 03:35:11PM -0700, alison.schofield@intel.com wrote:
> > > From: Alison Schofield <alison.schofield@intel.com>
> > >
> > > Commit 2852ca7fba9f ("panic: Taint kernel if tests are run")
> > > introduced a new taint type, TAINT_TEST, to signal that an
> > > in-kernel test has been run.
> > >
> > > TAINT_TEST taint type defaults into a 'bad_taint' list for
> > > kernel tracing and blocks the creation of trace events. This
> > > causes a problem for CXL testing where loading the cxl_test
> > > module makes all CXL modules out-of-tree, blocking any trace
> > > events.
> > >
> > > Trace events are in development for CXL at the moment and this
> > > issue was found in test with v6.0-rc1.
> > >
> > > Reported-by: Ira Weiny <ira.weiny@intel.com>
> > > Suggested-by: Dan Williams <dan.j.williams@intel.com>
> > > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > > ---
> 
> This seems fine to me: thanks for catching it. Test modules should be
> safe enough to load for this.
> 
> Does the comment in tracepoint_module_coming() need updating as well?
> https://elixir.bootlin.com/linux/v6.0-rc2/source/kernel/tracepoint.c#L650

Yes, ready in a v2:
-	 * Staging, out-of-tree, and unsigned GPL modules are fine.
+	 * Staging, out-of-tree, unsigned GPL, and test modules are fine.

Thanks!
Alison

> 
> Regardless, this is
> Reviewed-by: David Gow <davidgow@google.com>
> 
> Cheers,
> -- David
> 
> > >  kernel/tracepoint.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> > > index 64ea283f2f86..5f17378c9dc2 100644
> > > --- a/kernel/tracepoint.c
> > > +++ b/kernel/tracepoint.c
> > > @@ -571,7 +571,8 @@ static void for_each_tracepoint_range(
> > >  bool trace_module_has_bad_taint(struct module *mod)
> > >  {
> > >       return mod->taints & ~((1 << TAINT_OOT_MODULE) | (1 << TAINT_CRAP) |
> > > -                            (1 << TAINT_UNSIGNED_MODULE));
> > > +                            (1 << TAINT_UNSIGNED_MODULE) |
> > > +                            (1 << TAINT_TEST));
> > >  }
> > >
> > >  static BLOCKING_NOTIFIER_HEAD(tracepoint_notify_list);
> > >
> > > base-commit: 4c612826bec1441214816827979b62f84a097e91
> > > --
> > > 2.31.1
> > >