[PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Ben Hutchings]

Use of the GPL or a compatible licence doesn't necessarily make the code
any good.  We already consider staging modules to be suspect, and this
should also be true for out-of-tree modules which may receive very
little review.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
Debian has been carrying this for the last few kernel versions.  The
recent thread '[RFC] virtualbox tainting.' and discussions at KS suggest
that this might be more generally useful.

Ben.

 include/linux/kernel.h |    1 +
 kernel/module.c        |    5 +++++
 kernel/panic.c         |    2 ++
 scripts/mod/modpost.c  |    7 +++++++
 4 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 46ac9a5..2c05967 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -369,6 +369,7 @@ extern enum system_states {
 #define TAINT_WARN			9
 #define TAINT_CRAP			10
 #define TAINT_FIRMWARE_WORKAROUND	11
+#define TAINT_OOT_MODULE		12
 
 extern const char hex_asc[];
 #define hex_asc_lo(x)	hex_asc[((x) & 0x0f)]
diff --git a/kernel/module.c b/kernel/module.c
index 04379f92..c0872f1 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2487,6 +2487,9 @@ static int check_modinfo(struct module *mod, struct load_info *info)
 		return -ENOEXEC;
 	}
 
+	if (!get_modinfo(info, "intree"))
+		add_taint_module(mod, TAINT_OOT_MODULE);
+
 	if (get_modinfo(info, "staging")) {
 		add_taint_module(mod, TAINT_CRAP);
 		printk(KERN_WARNING "%s: module is from the staging directory,"
@@ -3257,6 +3260,8 @@ static char *module_flags(struct module *mod, char *buf)
 		buf[bx++] = '(';
 		if (mod->taints & (1 << TAINT_PROPRIETARY_MODULE))
 			buf[bx++] = 'P';
+		else if (mod->taints & (1 << TAINT_OOT_MODULE))
+			buf[bx++] = 'O';
 		if (mod->taints & (1 << TAINT_FORCED_MODULE))
 			buf[bx++] = 'F';
 		if (mod->taints & (1 << TAINT_CRAP))
diff --git a/kernel/panic.c b/kernel/panic.c
index d7bb697..b2659360 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -177,6 +177,7 @@ static const struct tnt tnts[] = {
 	{ TAINT_WARN,			'W', ' ' },
 	{ TAINT_CRAP,			'C', ' ' },
 	{ TAINT_FIRMWARE_WORKAROUND,	'I', ' ' },
+	{ TAINT_OOT_MODULE,		'O', ' ' },
 };
 
 /**
@@ -194,6 +195,7 @@ static const struct tnt tnts[] = {
  *  'W' - Taint on warning.
  *  'C' - modules from drivers/staging are loaded.
  *  'I' - Working around severe firmware bug.
+ *  'O' - Out-of-tree module has been loaded.
  *
  *	The string is overwritten by the next call to print_tainted().
  */
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index a509ff8..2bd594e 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1849,6 +1849,12 @@ static void add_header(struct buffer *b, struct module *mod)
 	buf_printf(b, "};\n");
 }
 
+static void add_intree_flag(struct buffer *b, int is_intree)
+{
+	if (is_intree)
+		buf_printf(b, "\nMODULE_INFO(intree, \"Y\");\n");
+}
+
 static void add_staging_flag(struct buffer *b, const char *name)
 {
 	static const char *staging_dir = "drivers/staging";
@@ -2169,6 +2175,7 @@ int main(int argc, char **argv)
 		buf.pos = 0;
 
 		add_header(&buf, mod);
+		add_intree_flag(&buf, !external_module);
 		add_staging_flag(&buf, mod->name);
 		err |= add_versions(&buf, mod);
 		add_depends(&buf, mod, modules);
-- 
1.7.7

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Dave Jones]

On Mon, Oct 24, 2011 at 03:12:28PM +0200, Ben Hutchings wrote:
 > Use of the GPL or a compatible licence doesn't necessarily make the code
 > any good.  We already consider staging modules to be suspect, and this
 > should also be true for out-of-tree modules which may receive very
 > little review.
 > 
 > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
 > ---
 > Debian has been carrying this for the last few kernel versions.  The
 > recent thread '[RFC] virtualbox tainting.' and discussions at KS suggest
 > that this might be more generally useful.

Looks good to me.

Reviewed-by: Dave Jones <davej@redhat.com>

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Randy Dunlap]

On 10/24/11 06:12, Ben Hutchings wrote:
> Use of the GPL or a compatible licence doesn't necessarily make the code
> any good.  We already consider staging modules to be suspect, and this
> should also be true for out-of-tree modules which may receive very
> little review.
> 
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> Debian has been carrying this for the last few kernel versions.  The
> recent thread '[RFC] virtualbox tainting.' and discussions at KS suggest
> that this might be more generally useful.
> 
> Ben.
> 
>  include/linux/kernel.h |    1 +
>  kernel/module.c        |    5 +++++
>  kernel/panic.c         |    2 ++
>  scripts/mod/modpost.c  |    7 +++++++
>  4 files changed, 15 insertions(+), 0 deletions(-)

Hi,

Please add 'O' to Documentation/oops-tracing.txt.

Thanks.

> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 46ac9a5..2c05967 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -369,6 +369,7 @@ extern enum system_states {
>  #define TAINT_WARN			9
>  #define TAINT_CRAP			10
>  #define TAINT_FIRMWARE_WORKAROUND	11
> +#define TAINT_OOT_MODULE		12
>  
>  extern const char hex_asc[];
>  #define hex_asc_lo(x)	hex_asc[((x) & 0x0f)]
> diff --git a/kernel/module.c b/kernel/module.c
> index 04379f92..c0872f1 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2487,6 +2487,9 @@ static int check_modinfo(struct module *mod, struct load_info *info)
>  		return -ENOEXEC;
>  	}
>  
> +	if (!get_modinfo(info, "intree"))
> +		add_taint_module(mod, TAINT_OOT_MODULE);
> +
>  	if (get_modinfo(info, "staging")) {
>  		add_taint_module(mod, TAINT_CRAP);
>  		printk(KERN_WARNING "%s: module is from the staging directory,"
> @@ -3257,6 +3260,8 @@ static char *module_flags(struct module *mod, char *buf)
>  		buf[bx++] = '(';
>  		if (mod->taints & (1 << TAINT_PROPRIETARY_MODULE))
>  			buf[bx++] = 'P';
> +		else if (mod->taints & (1 << TAINT_OOT_MODULE))
> +			buf[bx++] = 'O';
>  		if (mod->taints & (1 << TAINT_FORCED_MODULE))
>  			buf[bx++] = 'F';
>  		if (mod->taints & (1 << TAINT_CRAP))
> diff --git a/kernel/panic.c b/kernel/panic.c
> index d7bb697..b2659360 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -177,6 +177,7 @@ static const struct tnt tnts[] = {
>  	{ TAINT_WARN,			'W', ' ' },
>  	{ TAINT_CRAP,			'C', ' ' },
>  	{ TAINT_FIRMWARE_WORKAROUND,	'I', ' ' },
> +	{ TAINT_OOT_MODULE,		'O', ' ' },
>  };
>  
>  /**
> @@ -194,6 +195,7 @@ static const struct tnt tnts[] = {
>   *  'W' - Taint on warning.
>   *  'C' - modules from drivers/staging are loaded.
>   *  'I' - Working around severe firmware bug.
> + *  'O' - Out-of-tree module has been loaded.
>   *
>   *	The string is overwritten by the next call to print_tainted().
>   */
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index a509ff8..2bd594e 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1849,6 +1849,12 @@ static void add_header(struct buffer *b, struct module *mod)
>  	buf_printf(b, "};\n");
>  }
>  
> +static void add_intree_flag(struct buffer *b, int is_intree)
> +{
> +	if (is_intree)
> +		buf_printf(b, "\nMODULE_INFO(intree, \"Y\");\n");
> +}
> +
>  static void add_staging_flag(struct buffer *b, const char *name)
>  {
>  	static const char *staging_dir = "drivers/staging";
> @@ -2169,6 +2175,7 @@ int main(int argc, char **argv)
>  		buf.pos = 0;
>  
>  		add_header(&buf, mod);
> +		add_intree_flag(&buf, !external_module);
>  		add_staging_flag(&buf, mod->name);
>  		err |= add_versions(&buf, mod);
>  		add_depends(&buf, mod, modules);


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Greg KH]

On Mon, Oct 24, 2011 at 03:12:28PM +0200, Ben Hutchings wrote:
> Use of the GPL or a compatible licence doesn't necessarily make the code
> any good.  We already consider staging modules to be suspect, and this
> should also be true for out-of-tree modules which may receive very
> little review.
> 
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> Debian has been carrying this for the last few kernel versions.  The
> recent thread '[RFC] virtualbox tainting.' and discussions at KS suggest
> that this might be more generally useful.

Wonderful, thanks for pushing this:

	Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Rusty Russell]

On Mon, 24 Oct 2011 07:57:03 -0700, Randy Dunlap <rdunlap@xenotime.net> wrote:
> On 10/24/11 06:12, Ben Hutchings wrote:
> > Use of the GPL or a compatible licence doesn't necessarily make the code
> > any good.  We already consider staging modules to be suspect, and this
> > should also be true for out-of-tree modules which may receive very
> > little review.
> > 
> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> > ---
> > Debian has been carrying this for the last few kernel versions.  The
> > recent thread '[RFC] virtualbox tainting.' and discussions at KS suggest
> > that this might be more generally useful.
> > 
> > Ben.
> > 
> >  include/linux/kernel.h |    1 +
> >  kernel/module.c        |    5 +++++
> >  kernel/panic.c         |    2 ++
> >  scripts/mod/modpost.c  |    7 +++++++
> >  4 files changed, 15 insertions(+), 0 deletions(-)
> 
> Hi,
> 
> Please add 'O' to Documentation/oops-tracing.txt.

I did that, and applied the patch.  See below.

Thanks,
Rusty.

From: Ben Hutchings <ben@decadent.org.uk>
Subject: module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree
Date: Mon, 24 Oct 2011 15:12:28 +0200

Use of the GPL or a compatible licence doesn't necessarily make the code
any good.  We already consider staging modules to be suspect, and this
should also be true for out-of-tree modules which may receive very
little review.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Reviewed-by: Dave Jones <davej@redhat.com>
Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> (patched oops-tracing.txt)
---
 Documentation/oops-tracing.txt |    2 ++
 include/linux/kernel.h         |    1 +
 kernel/module.c                |    5 +++++
 kernel/panic.c                 |    2 ++
 scripts/mod/modpost.c          |    7 +++++++
 5 files changed, 17 insertions(+)

diff --git a/Documentation/oops-tracing.txt b/Documentation/oops-tracing.txt
--- a/Documentation/oops-tracing.txt
+++ b/Documentation/oops-tracing.txt
@@ -263,6 +263,8 @@ characters, each representing a particul
  12: 'I' if the kernel is working around a severe bug in the platform
      firmware (BIOS or similar).
 
+ 13: 'O' if an externally-built ("out-of-tree") module has been loaded.
+
 The primary reason for the 'Tainted: ' string is to tell kernel
 debuggers if this is a clean kernel or if anything unusual has
 occurred.  Tainting is permanent: even if an offending module is
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -369,6 +369,7 @@ extern enum system_states {
 #define TAINT_WARN			9
 #define TAINT_CRAP			10
 #define TAINT_FIRMWARE_WORKAROUND	11
+#define TAINT_OOT_MODULE		12
 
 extern const char hex_asc[];
 #define hex_asc_lo(x)	hex_asc[((x) & 0x0f)]
diff --git a/kernel/module.c b/kernel/module.c
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2487,6 +2487,9 @@ static int check_modinfo(struct module *
 		return -ENOEXEC;
 	}
 
+	if (!get_modinfo(info, "intree"))
+		add_taint_module(mod, TAINT_OOT_MODULE);
+
 	if (get_modinfo(info, "staging")) {
 		add_taint_module(mod, TAINT_CRAP);
 		printk(KERN_WARNING "%s: module is from the staging directory,"
@@ -3257,6 +3260,8 @@ static char *module_flags(struct module 
 		buf[bx++] = '(';
 		if (mod->taints & (1 << TAINT_PROPRIETARY_MODULE))
 			buf[bx++] = 'P';
+		else if (mod->taints & (1 << TAINT_OOT_MODULE))
+			buf[bx++] = 'O';
 		if (mod->taints & (1 << TAINT_FORCED_MODULE))
 			buf[bx++] = 'F';
 		if (mod->taints & (1 << TAINT_CRAP))
diff --git a/kernel/panic.c b/kernel/panic.c
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -177,6 +177,7 @@ static const struct tnt tnts[] = {
 	{ TAINT_WARN,			'W', ' ' },
 	{ TAINT_CRAP,			'C', ' ' },
 	{ TAINT_FIRMWARE_WORKAROUND,	'I', ' ' },
+	{ TAINT_OOT_MODULE,		'O', ' ' },
 };
 
 /**
@@ -194,6 +195,7 @@ static const struct tnt tnts[] = {
  *  'W' - Taint on warning.
  *  'C' - modules from drivers/staging are loaded.
  *  'I' - Working around severe firmware bug.
+ *  'O' - Out-of-tree module has been loaded.
  *
  *	The string is overwritten by the next call to print_tainted().
  */
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1849,6 +1849,12 @@ static void add_header(struct buffer *b,
 	buf_printf(b, "};\n");
 }
 
+static void add_intree_flag(struct buffer *b, int is_intree)
+{
+	if (is_intree)
+		buf_printf(b, "\nMODULE_INFO(intree, \"Y\");\n");
+}
+
 static void add_staging_flag(struct buffer *b, const char *name)
 {
 	static const char *staging_dir = "drivers/staging";
@@ -2169,6 +2175,7 @@ int main(int argc, char **argv)
 		buf.pos = 0;
 
 		add_header(&buf, mod);
+		add_intree_flag(&buf, !external_module);
 		add_staging_flag(&buf, mod->name);
 		err |= add_versions(&buf, mod);
 		add_depends(&buf, mod, modules);

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1336 bytes --]

On Tue, 2011-10-25 at 14:26 +1030, Rusty Russell wrote:
> On Mon, 24 Oct 2011 07:57:03 -0700, Randy Dunlap <rdunlap@xenotime.net> wrote:
> > On 10/24/11 06:12, Ben Hutchings wrote:
> > > Use of the GPL or a compatible licence doesn't necessarily make the code
> > > any good.  We already consider staging modules to be suspect, and this
> > > should also be true for out-of-tree modules which may receive very
> > > little review.
> > > 
> > > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> > > ---
> > > Debian has been carrying this for the last few kernel versions.  The
> > > recent thread '[RFC] virtualbox tainting.' and discussions at KS suggest
> > > that this might be more generally useful.
> > > 
> > > Ben.
> > > 
> > >  include/linux/kernel.h |    1 +
> > >  kernel/module.c        |    5 +++++
> > >  kernel/panic.c         |    2 ++
> > >  scripts/mod/modpost.c  |    7 +++++++
> > >  4 files changed, 15 insertions(+), 0 deletions(-)
> > 
> > Hi,
> > 
> > Please add 'O' to Documentation/oops-tracing.txt.

Sorry, this is the second time I've missed that now...

> I did that, and applied the patch.  See below.
[...]

Thanks to everyone.

Ben.

-- 
Ben Hutchings
DNRC Motto:  I can please only one person per day.
Today is not your day.  Tomorrow isn't looking good either.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Nick Bowler]

On 2011-10-25 14:26 +1030, Rusty Russell wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> Subject: module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree
> Date: Mon, 24 Oct 2011 15:12:28 +0200
> 
> Use of the GPL or a compatible licence doesn't necessarily make the code
> any good.  We already consider staging modules to be suspect, and this
> should also be true for out-of-tree modules which may receive very
> little review.
> 
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> Reviewed-by: Dave Jones <davej@redhat.com>
> Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> (patched oops-tracing.txt)
> ---
>  Documentation/oops-tracing.txt |    2 ++
>  include/linux/kernel.h         |    1 +
>  kernel/module.c                |    5 +++++
>  kernel/panic.c                 |    2 ++
>  scripts/mod/modpost.c          |    7 +++++++
>  5 files changed, 17 insertions(+)

This patch prevents the use of lockdep for debugging out of tree
modules, which is rather mean.

-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1468 bytes --]

On Tue, 2011-10-25 at 11:38 -0400, Nick Bowler wrote:
> On 2011-10-25 14:26 +1030, Rusty Russell wrote:
> > From: Ben Hutchings <ben@decadent.org.uk>
> > Subject: module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree
> > Date: Mon, 24 Oct 2011 15:12:28 +0200
> > 
> > Use of the GPL or a compatible licence doesn't necessarily make the code
> > any good.  We already consider staging modules to be suspect, and this
> > should also be true for out-of-tree modules which may receive very
> > little review.
> > 
> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> > Reviewed-by: Dave Jones <davej@redhat.com>
> > Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
> > Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> (patched oops-tracing.txt)
> > ---
> >  Documentation/oops-tracing.txt |    2 ++
> >  include/linux/kernel.h         |    1 +
> >  kernel/module.c                |    5 +++++
> >  kernel/panic.c                 |    2 ++
> >  scripts/mod/modpost.c          |    7 +++++++
> >  5 files changed, 17 insertions(+)
> 
> This patch prevents the use of lockdep for debugging out of tree
> modules, which is rather mean.

It was already disabled for staging modules, which seems equally
unhelpful.  Maybe it should print taint flags at the top of its warnings
instead.

Ben.

-- 
Ben Hutchings
DNRC Motto:  I can please only one person per day.
Today is not your day.  Tomorrow isn't looking good either.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Nick Bowler]

On 2011-10-25 18:05 +0200, Ben Hutchings wrote:
> On Tue, 2011-10-25 at 11:38 -0400, Nick Bowler wrote:
> > This patch prevents the use of lockdep for debugging out of tree
> > modules, which is rather mean.
> 
> It was already disabled for staging modules, which seems equally
> unhelpful.

This is not the case: lockdep works fine with staging modules.

Cheers,
-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Greg KH]

On Tue, Oct 25, 2011 at 12:51:42PM -0400, Nick Bowler wrote:
> On 2011-10-25 18:05 +0200, Ben Hutchings wrote:
> > On Tue, 2011-10-25 at 11:38 -0400, Nick Bowler wrote:
> > > This patch prevents the use of lockdep for debugging out of tree
> > > modules, which is rather mean.
> > 
> > It was already disabled for staging modules, which seems equally
> > unhelpful.
> 
> This is not the case: lockdep works fine with staging modules.

Yes, that was fixed a few kernel versions ago.

Now you might want to update that fix for the TAINT_OOT_MODULE flag as
well, if you feel it is needed.

greg k-h

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Dave Jones]

On Tue, Oct 25, 2011 at 10:04:55PM +0200, Greg Kroah-Hartman wrote:
 > On Tue, Oct 25, 2011 at 12:51:42PM -0400, Nick Bowler wrote:
 > > On 2011-10-25 18:05 +0200, Ben Hutchings wrote:
 > > > On Tue, 2011-10-25 at 11:38 -0400, Nick Bowler wrote:
 > > > > This patch prevents the use of lockdep for debugging out of tree
 > > > > modules, which is rather mean.
 > > > 
 > > > It was already disabled for staging modules, which seems equally
 > > > unhelpful.
 > > 
 > > This is not the case: lockdep works fine with staging modules.
 > 
 > Yes, that was fixed a few kernel versions ago.
 > 
 > Now you might want to update that fix for the TAINT_OOT_MODULE flag as
 > well, if you feel it is needed.

I'm assuming you mean this patch ?

commit 7816c45bf13255157c00fb8aca86cb64d825e878
Author: Roland Vossen <rvossen@broadcom.com>
Date:   Thu Apr 7 11:20:58 2011 +0200

    modules: Enabled dynamic debugging for staging modules
    
    Driver modules from the staging directory are marked 'tainted'
    by module.c. Subsequently, tainted modules are denied dynamic
    debugging. This is unwanted behavior, since staging modules should
    be able to use the dynamic debugging mechanism.
    
    Please merge this also into the staging-linus branch.
    
    Signed-off-by: Roland Vossen <rvossen@broadcom.com>
    Acked-by: Jason Baron <jbaron@redhat.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff --git a/kernel/module.c b/kernel/module.c
index d5938a5..4d5c16a 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2790,7 +2790,7 @@ static struct module *load_module(void __user *umod,
        }
 
        /* This has to be done once we're sure module name is unique. */
-       if (!mod->taints)
+       if (!mod->taints || mod->taints == (1U<<TAINT_CRAP))
                dynamic_debug_setup(info.debug, info.num_debug);
 
        /* Find duplicate symbols */
@@ -2827,7 +2827,7 @@ static struct module *load_module(void __user *umod,
        module_bug_cleanup(mod);
 
  ddebug:
-       if (!mod->taints)
+       if (!mod->taints || mod->taints == (1U<<TAINT_CRAP))
                dynamic_debug_remove(info.debug);
  unlock:
        mutex_unlock(&module_mutex);



If we want to support out of tree modules with this, should we just nuke the
whole check, or do we still want to prevent certain types of tainted kernels
from using this stuff ?

(sidenote: it's not immediately obvious to me that this is the right patch,
as dynamic debug & lockdep are separate things, though this was the only
thing in kernel/module.c's history this year that sounds similar)

	Dave

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Greg KH]

On Tue, Oct 25, 2011 at 04:17:24PM -0400, Dave Jones wrote:
> On Tue, Oct 25, 2011 at 10:04:55PM +0200, Greg Kroah-Hartman wrote:
>  > On Tue, Oct 25, 2011 at 12:51:42PM -0400, Nick Bowler wrote:
>  > > On 2011-10-25 18:05 +0200, Ben Hutchings wrote:
>  > > > On Tue, 2011-10-25 at 11:38 -0400, Nick Bowler wrote:
>  > > > > This patch prevents the use of lockdep for debugging out of tree
>  > > > > modules, which is rather mean.
>  > > > 
>  > > > It was already disabled for staging modules, which seems equally
>  > > > unhelpful.
>  > > 
>  > > This is not the case: lockdep works fine with staging modules.
>  > 
>  > Yes, that was fixed a few kernel versions ago.
>  > 
>  > Now you might want to update that fix for the TAINT_OOT_MODULE flag as
>  > well, if you feel it is needed.
> 
> I'm assuming you mean this patch ?
> 
> commit 7816c45bf13255157c00fb8aca86cb64d825e878
> Author: Roland Vossen <rvossen@broadcom.com>
> Date:   Thu Apr 7 11:20:58 2011 +0200
> 
>     modules: Enabled dynamic debugging for staging modules

Hm, this is the patch I was thinking about yes.  But as you point out:

> If we want to support out of tree modules with this, should we just nuke the
> whole check, or do we still want to prevent certain types of tainted kernels
> from using this stuff ?

I don't know, there was some reason we didn't want to run dynamic_debug
for "normal" tainted kernel modules, but I can't recall it at the
moment, sorry.

> 
> (sidenote: it's not immediately obvious to me that this is the right patch,
> as dynamic debug & lockdep are separate things, though this was the only
> thing in kernel/module.c's history this year that sounds similar)

Perhaps the lockdep thing is totally different.  I don't know about that
check.

thanks,

greg k-h

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Rusty Russell]

On Tue, 25 Oct 2011 16:17:24 -0400, Dave Jones <davej@redhat.com> wrote:
> commit 7816c45bf13255157c00fb8aca86cb64d825e878
> Author: Roland Vossen <rvossen@broadcom.com>
> Date:   Thu Apr 7 11:20:58 2011 +0200
> 
>     modules: Enabled dynamic debugging for staging modules
...
>     
>     Signed-off-by: Roland Vossen <rvossen@broadcom.com>
>     Acked-by: Jason Baron <jbaron@redhat.com>
>     Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

Greg, you know better.  This is why we have maintainers: I can't track
patches I don't see.  Grrr...

> If we want to support out of tree modules with this, should we just nuke the
> whole check, or do we still want to prevent certain types of tainted kernels
> from using this stuff ?

It goes back to the first implementation of kernel markers.  IIRC, it
was to prevent dynamic debug stuff from circumventing licensing, but
testing for *any* taint seems overbroad.  Mathieu?

Thanks,
Rusty.
PS.  Can't see how this related to lockdep either...

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Mathieu Desnoyers]

* Rusty Russell (rusty@rustcorp.com.au) wrote:
> On Tue, 25 Oct 2011 16:17:24 -0400, Dave Jones <davej@redhat.com> wrote:
> > commit 7816c45bf13255157c00fb8aca86cb64d825e878
> > Author: Roland Vossen <rvossen@broadcom.com>
> > Date:   Thu Apr 7 11:20:58 2011 +0200
> > 
> >     modules: Enabled dynamic debugging for staging modules
> ...
> >     
> >     Signed-off-by: Roland Vossen <rvossen@broadcom.com>
> >     Acked-by: Jason Baron <jbaron@redhat.com>
> >     Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> 
> Greg, you know better.  This is why we have maintainers: I can't track
> patches I don't see.  Grrr...
> 
> > If we want to support out of tree modules with this, should we just nuke the
> > whole check, or do we still want to prevent certain types of tainted kernels
> > from using this stuff ?
> 
> It goes back to the first implementation of kernel markers.  IIRC, it
> was to prevent dynamic debug stuff from circumventing licensing, but
> testing for *any* taint seems overbroad.  Mathieu?

This check for tainted modules was first introduced with markers, and
then used by tracepoints, and then also by dynamic debug. The rationale
for this check was mainly to ensure that the marker/tracepoint code
would not trigger a crash when loading a module with incompatible module
header, originally compiled for an older kernel, into a newer kernel.
This problem would happen even if the said module does not contain any
marker/tracepoint, because we happen to try to use fields that are
non-existent in the module header.

AFAIK, dynamic debug use a similar trick that require extra members in
the module header, so checking that the module header format is
compatible with the kernel would be enough. Is there a taint flag that
allows us to check for this more narrowly ? TAINT_FORCED_MODULE would
probably be the closest one we have now, although we might want to be
more specific than that.

Thanks,

Mathieu

> 
> Thanks,
> Rusty.
> PS.  Can't see how this related to lockdep either...

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Nick Bowler]

On 2011-10-25 22:54 +0200, Greg KH wrote:
> On Tue, Oct 25, 2011 at 04:17:24PM -0400, Dave Jones wrote:
> > On Tue, Oct 25, 2011 at 10:04:55PM +0200, Greg Kroah-Hartman wrote:
> >  > On Tue, Oct 25, 2011 at 12:51:42PM -0400, Nick Bowler wrote:
> >  > > This is not the case: lockdep works fine with staging modules.
> >  > 
> >  > Yes, that was fixed a few kernel versions ago.
> >  > 
> >  > Now you might want to update that fix for the TAINT_OOT_MODULE flag as
> >  > well, if you feel it is needed.
> > 
> > I'm assuming you mean this patch ?
> > 
> > commit 7816c45bf13255157c00fb8aca86cb64d825e878
> > Author: Roland Vossen <rvossen@broadcom.com>
> > Date:   Thu Apr 7 11:20:58 2011 +0200
> > 
> >     modules: Enabled dynamic debugging for staging modules
> 
> Hm, this is the patch I was thinking about yes.  But as you point out:
[...]
> Perhaps the lockdep thing is totally different.  I don't know about that
> check.

Lockdep is disabled (for the whole system) by add_taint itself.  The
relevant commit that fixes TAINT_CRAP appears to be this one (circa
2.6.30):

  commit 574bbe782057fdf0490dc7dec906a2dc26363e20
  Author: Frederic Weisbecker <fweisbec@gmail.com>
  Date:   Sat Apr 11 03:17:18 2009 +0200
  
      lockdep: continue lock debugging despite some taints

I didn't know about the dynamic debug problem.  Is there more breakage
that we haven't found yet?  Remind me why we're trying to cripple out
of tree module users?

Cheers,
-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Rusty Russell]

On Wed, 26 Oct 2011 09:08:34 -0400, Nick Bowler <nbowler@elliptictech.com> wrote:
> On 2011-10-25 22:54 +0200, Greg KH wrote:
> > On Tue, Oct 25, 2011 at 04:17:24PM -0400, Dave Jones wrote:
> > > On Tue, Oct 25, 2011 at 10:04:55PM +0200, Greg Kroah-Hartman wrote:
> > >  > On Tue, Oct 25, 2011 at 12:51:42PM -0400, Nick Bowler wrote:
> > >  > > This is not the case: lockdep works fine with staging modules.
> > >  > 
> > >  > Yes, that was fixed a few kernel versions ago.
> > >  > 
> > >  > Now you might want to update that fix for the TAINT_OOT_MODULE flag as
> > >  > well, if you feel it is needed.
> > > 
> > > I'm assuming you mean this patch ?
> > > 
> > > commit 7816c45bf13255157c00fb8aca86cb64d825e878
> > > Author: Roland Vossen <rvossen@broadcom.com>
> > > Date:   Thu Apr 7 11:20:58 2011 +0200
> > > 
> > >     modules: Enabled dynamic debugging for staging modules
> > 
> > Hm, this is the patch I was thinking about yes.  But as you point out:
> [...]
> > Perhaps the lockdep thing is totally different.  I don't know about that
> > check.
> 
> Lockdep is disabled (for the whole system) by add_taint itself.  The
> relevant commit that fixes TAINT_CRAP appears to be this one (circa
> 2.6.30):
> 
>   commit 574bbe782057fdf0490dc7dec906a2dc26363e20
>   Author: Frederic Weisbecker <fweisbec@gmail.com>
>   Date:   Sat Apr 11 03:17:18 2009 +0200
>   
>       lockdep: continue lock debugging despite some taints
> 
> I didn't know about the dynamic debug problem.  Is there more breakage
> that we haven't found yet?  Remind me why we're trying to cripple out
> of tree module users?

Gah, people are overloading taint.

It doesn't mean "don't do stuff", it means "note the taint when
something goes wrong".

I think we need a "taint_string()" function, and instead of lockdep
disabling itself it should note the taint string in its reports.
Similarly for anything else (oops already does this).

Dynamic debugging should just crash like anything else if they force a
module load and get it wrong: it's the least of their problems.

If noone hugely objects, I'll create a patch series...

Thanks,
Rusty.

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Dave Jones]

On Thu, Oct 27, 2011 at 11:41:37AM +1030, Rusty Russell wrote:

 > I think we need a "taint_string()" function, and instead of lockdep
 > disabling itself it should note the taint string in its reports.
 > Similarly for anything else (oops already does this).

you mean like print_tainted() ?
 
	Dave
 

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Greg KH]

On Thu, Oct 27, 2011 at 11:41:37AM +1030, Rusty Russell wrote:
> On Wed, 26 Oct 2011 09:08:34 -0400, Nick Bowler <nbowler@elliptictech.com> wrote:
> > On 2011-10-25 22:54 +0200, Greg KH wrote:
> > > On Tue, Oct 25, 2011 at 04:17:24PM -0400, Dave Jones wrote:
> > > > On Tue, Oct 25, 2011 at 10:04:55PM +0200, Greg Kroah-Hartman wrote:
> > > >  > On Tue, Oct 25, 2011 at 12:51:42PM -0400, Nick Bowler wrote:
> > > >  > > This is not the case: lockdep works fine with staging modules.
> > > >  > 
> > > >  > Yes, that was fixed a few kernel versions ago.
> > > >  > 
> > > >  > Now you might want to update that fix for the TAINT_OOT_MODULE flag as
> > > >  > well, if you feel it is needed.
> > > > 
> > > > I'm assuming you mean this patch ?
> > > > 
> > > > commit 7816c45bf13255157c00fb8aca86cb64d825e878
> > > > Author: Roland Vossen <rvossen@broadcom.com>
> > > > Date:   Thu Apr 7 11:20:58 2011 +0200
> > > > 
> > > >     modules: Enabled dynamic debugging for staging modules
> > > 
> > > Hm, this is the patch I was thinking about yes.  But as you point out:
> > [...]
> > > Perhaps the lockdep thing is totally different.  I don't know about that
> > > check.
> > 
> > Lockdep is disabled (for the whole system) by add_taint itself.  The
> > relevant commit that fixes TAINT_CRAP appears to be this one (circa
> > 2.6.30):
> > 
> >   commit 574bbe782057fdf0490dc7dec906a2dc26363e20
> >   Author: Frederic Weisbecker <fweisbec@gmail.com>
> >   Date:   Sat Apr 11 03:17:18 2009 +0200
> >   
> >       lockdep: continue lock debugging despite some taints
> > 
> > I didn't know about the dynamic debug problem.  Is there more breakage
> > that we haven't found yet?  Remind me why we're trying to cripple out
> > of tree module users?
> 
> Gah, people are overloading taint.
> 
> It doesn't mean "don't do stuff", it means "note the taint when
> something goes wrong".

I agree, we shouldn't be changing logic in the kernel due to tainting,
so removing any such checks would be a good idea.

thanks,

greg k-h

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Rusty Russell]

On Wed, 26 Oct 2011 21:55:28 -0400, Dave Jones <davej@redhat.com> wrote:
> On Thu, Oct 27, 2011 at 11:41:37AM +1030, Rusty Russell wrote:
> 
>  > I think we need a "taint_string()" function, and instead of lockdep
>  > disabling itself it should note the taint string in its reports.
>  > Similarly for anything else (oops already does this).
> 
> you mean like print_tainted() ?

Wow, there's one terribly named function!

So, yeah, kinda like that.  Only remove the suckage.

Thanks,
Rusty.

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Luis R. Rodriguez]

On Mon, Oct 24, 2011 at 6:12 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
> Use of the GPL or a compatible licence doesn't necessarily make the code
> any good.  We already consider staging modules to be suspect, and this
> should also be true for out-of-tree modules which may receive very
> little review.
>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> Debian has been carrying this for the last few kernel versions.  The
> recent thread '[RFC] virtualbox tainting.' and discussions at KS suggest
> that this might be more generally useful.

This indeed seems like a good idea to advocate getting things upstream
(not just staging) but what about the case where we have upstream
drivers from future kernels backported to older kernels and the newer
driver is simply provided as a feature for users who may need new
features / chipset support on their old distribution kernel?

It seems this taint flag will be used for driers backported through
compat-wireless, the compat kernel module or any other backported
driver, even if it is indeed upstream and whereby kernel developer
*do* commit to actually fixing issues. In our experience
compat-wireless bugs *are real bugs*, not backport bugs so we do look
into them. In our latest linux-next.git based release for example
backport code consists only of 1.3804% of the code.

  Luis

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Ben Hutchings]

On Mon, Dec 12, 2011 at 01:40:44PM -0800, Luis R. Rodriguez wrote:
> On Mon, Oct 24, 2011 at 6:12 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
> > Use of the GPL or a compatible licence doesn't necessarily make the code
> > any good.  We already consider staging modules to be suspect, and this
> > should also be true for out-of-tree modules which may receive very
> > little review.
> >
> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> > ---
> > Debian has been carrying this for the last few kernel versions.  The
> > recent thread '[RFC] virtualbox tainting.' and discussions at KS suggest
> > that this might be more generally useful.
> 
> This indeed seems like a good idea to advocate getting things upstream
> (not just staging) but what about the case where we have upstream
> drivers from future kernels backported to older kernels and the newer
> driver is simply provided as a feature for users who may need new
> features / chipset support on their old distribution kernel?

They continue to work without any loss of functionality.  (After the
follow-up patches to keep dynamic debugging and lock debugging
working.)

> It seems this taint flag will be used for driers backported through
> compat-wireless, the compat kernel module or any other backported
> driver, even if it is indeed upstream and whereby kernel developer
> *do* commit to actually fixing issues. In our experience
> compat-wireless bugs *are real bugs*, not backport bugs so we do look
> into them. In our latest linux-next.git based release for example
> backport code consists only of 1.3804% of the code.

Now you can look for (O) after the module name in a BUG/Oops message
and you can tell whether the user really had the original or
compat-wireless version of the driver.

It is really up to each distributor or developer how they treat
bug reports with the O taint.  When handling Debian bug reports I
won't automatically reject such a tainted kernel but I will look
carefully at the module list.

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
                                                              - Albert Camus

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Luis R. Rodriguez]

On Mon, Dec 12, 2011 at 1:58 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
> On Mon, Dec 12, 2011 at 01:40:44PM -0800, Luis R. Rodriguez wrote:
>> On Mon, Oct 24, 2011 at 6:12 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
>> > Use of the GPL or a compatible licence doesn't necessarily make the code
>> > any good.  We already consider staging modules to be suspect, and this
>> > should also be true for out-of-tree modules which may receive very
>> > little review.
>> >
>> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
>> > ---
>> > Debian has been carrying this for the last few kernel versions.  The
>> > recent thread '[RFC] virtualbox tainting.' and discussions at KS suggest
>> > that this might be more generally useful.
>>
>> This indeed seems like a good idea to advocate getting things upstream
>> (not just staging) but what about the case where we have upstream
>> drivers from future kernels backported to older kernels and the newer
>> driver is simply provided as a feature for users who may need new
>> features / chipset support on their old distribution kernel?
>
> They continue to work without any loss of functionality.  (After the
> follow-up patches to keep dynamic debugging and lock debugging
> working.)

Great!

>> It seems this taint flag will be used for driers backported through
>> compat-wireless, the compat kernel module or any other backported
>> driver, even if it is indeed upstream and whereby kernel developer
>> *do* commit to actually fixing issues. In our experience
>> compat-wireless bugs *are real bugs*, not backport bugs so we do look
>> into them. In our latest linux-next.git based release for example
>> backport code consists only of 1.3804% of the code.
>
> Now you can look for (O) after the module name in a BUG/Oops message
> and you can tell whether the user really had the original or
> compat-wireless version of the driver.
>
> It is really up to each distributor or developer how they treat
> bug reports with the O taint.  When handling Debian bug reports I
> won't automatically reject such a tainted kernel but I will look
> carefully at the module list.

I'm working on getting my companies to abandon 802.11 proprietary
drivers for good. For Station mode of operation this is pretty much
mission complete. For AP products.. this is work in progress. The out
of tree flag is a good utility one can use to help justify working
upstream but if we treat any future-kernel-backported-driver equally
to any out of tree crap piece of shit driver, it seems to do unjustice
to the value of a properly upstream backported driver. I will note
that I put a lot of effort to ensure that the backport effort is
upstream-centric in an *extremely* upstream-biased way, see how I
label extra patches for tarballs [1]. If your patches are not upstream
the only way you get into these tarballs are by providing patches into
these directories:

  * pending-stable/ stable fixes from linux-next.git not yet on a stable release
  * linux-next-cherry-picks/ patches upstream but that won't go to the
stable release that we want to cherry pick
  * linux-next-pending/ patches posted on the public development
mailing list, patch not yet merged due to the maintainer being away on
vacation or whatever
  * crap/ patches not even posted publicly yet

Each tarball used also gets pegged with a letter if *any* patch from
any of these directories gets applied. The compat module, upon being
loaded, will also print the kernel ring buffer the exact release,
whether extra patches were provided, the upstream git tree used as
base and so on.

So -- although from a technical perspective this may mean Debian /
other kernel developers may ignore the taint flag for compat-wireless
it'd sure be nice to avoid it for them all together. Just can't think
of a way to do it yet... If you agree should we continue to think of a
way if its possible?

[1] http://wireless.kernel.org/en/users/Download/stable#Legend

  Luis

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Luis R. Rodriguez]

On Mon, Dec 12, 2011 at 2:47 PM, Luis R. Rodriguez <mcgrof@frijolero.org> wrote:
> On Mon, Dec 12, 2011 at 1:58 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
>> On Mon, Dec 12, 2011 at 01:40:44PM -0800, Luis R. Rodriguez wrote:
>>> On Mon, Oct 24, 2011 at 6:12 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
>>> > Use of the GPL or a compatible licence doesn't necessarily make the code
>>> > any good.  We already consider staging modules to be suspect, and this
>>> > should also be true for out-of-tree modules which may receive very
>>> > little review.
>>> >
>>> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
>>> > ---
>>> > Debian has been carrying this for the last few kernel versions.  The
>>> > recent thread '[RFC] virtualbox tainting.' and discussions at KS suggest
>>> > that this might be more generally useful.
>>>
>>> This indeed seems like a good idea to advocate getting things upstream
>>> (not just staging) but what about the case where we have upstream
>>> drivers from future kernels backported to older kernels and the newer
>>> driver is simply provided as a feature for users who may need new
>>> features / chipset support on their old distribution kernel?
>>
>> They continue to work without any loss of functionality.  (After the
>> follow-up patches to keep dynamic debugging and lock debugging
>> working.)
>
> Great!
>
>>> It seems this taint flag will be used for driers backported through
>>> compat-wireless, the compat kernel module or any other backported
>>> driver, even if it is indeed upstream and whereby kernel developer
>>> *do* commit to actually fixing issues. In our experience
>>> compat-wireless bugs *are real bugs*, not backport bugs so we do look
>>> into them. In our latest linux-next.git based release for example
>>> backport code consists only of 1.3804% of the code.
>>
>> Now you can look for (O) after the module name in a BUG/Oops message
>> and you can tell whether the user really had the original or
>> compat-wireless version of the driver.
>>
>> It is really up to each distributor or developer how they treat
>> bug reports with the O taint.  When handling Debian bug reports I
>> won't automatically reject such a tainted kernel but I will look
>> carefully at the module list.
>
> I'm working on getting my companies to abandon 802.11 proprietary
> drivers for good. For Station mode of operation this is pretty much
> mission complete. For AP products.. this is work in progress. The out
> of tree flag is a good utility one can use to help justify working
> upstream but if we treat any future-kernel-backported-driver equally
> to any out of tree crap piece of shit driver, it seems to do unjustice
> to the value of a properly upstream backported driver. I will note
> that I put a lot of effort to ensure that the backport effort is
> upstream-centric in an *extremely* upstream-biased way, see how I
> label extra patches for tarballs [1]. If your patches are not upstream
> the only way you get into these tarballs are by providing patches into
> these directories:
>
>  * pending-stable/ stable fixes from linux-next.git not yet on a stable release
>  * linux-next-cherry-picks/ patches upstream but that won't go to the
> stable release that we want to cherry pick
>  * linux-next-pending/ patches posted on the public development
> mailing list, patch not yet merged due to the maintainer being away on
> vacation or whatever
>  * crap/ patches not even posted publicly yet
>
> Each tarball used also gets pegged with a letter if *any* patch from
> any of these directories gets applied. The compat module, upon being
> loaded, will also print the kernel ring buffer the exact release,
> whether extra patches were provided, the upstream git tree used as
> base and so on.
>
> So -- although from a technical perspective this may mean Debian /
> other kernel developers may ignore the taint flag for compat-wireless
> it'd sure be nice to avoid it for them all together. Just can't think
> of a way to do it yet... If you agree should we continue to think of a
> way if its possible?
>
> [1] http://wireless.kernel.org/en/users/Download/stable#Legend

How about a way to peg a driver as a backport from future kernels?
Like maybe MODULE_COMPAT() ?

  Luis

Re: [PATCH] module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 3675 bytes --]

On Mon, 2011-12-12 at 14:47 -0800, Luis R. Rodriguez wrote:
> On Mon, Dec 12, 2011 at 1:58 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
> > On Mon, Dec 12, 2011 at 01:40:44PM -0800, Luis R. Rodriguez wrote:
[...]
> >> It seems this taint flag will be used for driers backported through
> >> compat-wireless, the compat kernel module or any other backported
> >> driver, even if it is indeed upstream and whereby kernel developer
> >> *do* commit to actually fixing issues. In our experience
> >> compat-wireless bugs *are real bugs*, not backport bugs so we do look
> >> into them. In our latest linux-next.git based release for example
> >> backport code consists only of 1.3804% of the code.
> >
> > Now you can look for (O) after the module name in a BUG/Oops message
> > and you can tell whether the user really had the original or
> > compat-wireless version of the driver.
> >
> > It is really up to each distributor or developer how they treat
> > bug reports with the O taint.  When handling Debian bug reports I
> > won't automatically reject such a tainted kernel but I will look
> > carefully at the module list.
> 
> I'm working on getting my companies to abandon 802.11 proprietary
> drivers for good. For Station mode of operation this is pretty much
> mission complete. For AP products.. this is work in progress. The out
> of tree flag is a good utility one can use to help justify working
> upstream but if we treat any future-kernel-backported-driver equally
> to any out of tree crap piece of shit driver, it seems to do unjustice
> to the value of a properly upstream backported driver.

Well, it's a warning that not all the kernel code comes from the
original source tree that the version string identifies.  It's not a
value judgement (unlike TAINT_CRAP).

> I will note
> that I put a lot of effort to ensure that the backport effort is
> upstream-centric in an *extremely* upstream-biased way, see how I
> label extra patches for tarballs [1]. If your patches are not upstream
> the only way you get into these tarballs are by providing patches into
> these directories:
> 
>   * pending-stable/ stable fixes from linux-next.git not yet on a stable release
>   * linux-next-cherry-picks/ patches upstream but that won't go to the
> stable release that we want to cherry pick
>   * linux-next-pending/ patches posted on the public development
> mailing list, patch not yet merged due to the maintainer being away on
> vacation or whatever
>   * crap/ patches not even posted publicly yet
> 
> Each tarball used also gets pegged with a letter if *any* patch from
> any of these directories gets applied. The compat module, upon being
> loaded, will also print the kernel ring buffer the exact release,
> whether extra patches were provided, the upstream git tree used as
> base and so on.

Thanks, I appreciate that.

> So -- although from a technical perspective this may mean Debian /
> other kernel developers may ignore the taint flag for compat-wireless
> it'd sure be nice to avoid it for them all together. Just can't think
> of a way to do it yet... If you agree should we continue to think of a
> way if its possible?

Maybe we should be talking about updating the distribution packages
instead.  For the Debian kernel packages, we backport various drivers to
the stable distribution to add support for new hardware.  Please mail
debian-kernel@lists.debian.org if you would like to work with us on
that.

Ben.

> [1] http://wireless.kernel.org/en/users/Download/stable#Legend
> 
>   Luis

-- 
Ben Hutchings
Computers are not intelligent.	They only think they are.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[RFC] modpost: add option to allow external modules to avoid taint

[John W. Linville]

In some cases, it might be desirable to package a module from an
external source tree alongside the base kernel.  In those cases, it
might also be desirable to not have those modules tainting the kernel.

This patch provides a mechanism for an external module build to declare
itself as an "integrated build".  Such a module is then treated the same
as an intree module.

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
Any thoughts on this?  I'm thinking of adding this to Fedora kernels,
where I have been working to integrate the compat-wireless package as
part of the base kernel RPM.

 scripts/Makefile.modpost |    1 +
 scripts/mod/modpost.c    |   10 ++++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 08dce14..160c6fb 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -81,6 +81,7 @@ modpost = scripts/mod/modpost                    \
  $(if $(KBUILD_EXTMOD),-o $(modulesymfile))      \
  $(if $(CONFIG_DEBUG_SECTION_MISMATCH),,-S)      \
  $(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w) \
+ $(if $(INTEGRATED_BUILD),-B) \
  $(if $(cross_build),-c)
 
 quiet_cmd_modpost = MODPOST $(words $(filter-out vmlinux FORCE, $^)) modules
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 2bd594e..5d077f9 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -43,6 +43,9 @@ static int warn_unresolved = 0;
 static int sec_mismatch_count = 0;
 static int sec_mismatch_verbose = 1;
 
+/* Is this a module being built as part of an integrated package? */
+static int integrated_build = 0;
+
 enum export {
 	export_plain,      export_unused,     export_gpl,
 	export_unused_gpl, export_gpl_future, export_unknown
@@ -1851,7 +1854,7 @@ static void add_header(struct buffer *b, struct module *mod)
 
 static void add_intree_flag(struct buffer *b, int is_intree)
 {
-	if (is_intree)
+	if (is_intree || integrated_build)
 		buf_printf(b, "\nMODULE_INFO(intree, \"Y\");\n");
 }
 
@@ -2101,7 +2104,7 @@ int main(int argc, char **argv)
 	struct ext_sym_list *extsym_iter;
 	struct ext_sym_list *extsym_start = NULL;
 
-	while ((opt = getopt(argc, argv, "i:I:e:cmsSo:awM:K:")) != -1) {
+	while ((opt = getopt(argc, argv, "i:I:e:cmsSo:awM:K:B")) != -1) {
 		switch (opt) {
 		case 'i':
 			kernel_read = optarg;
@@ -2139,6 +2142,9 @@ int main(int argc, char **argv)
 		case 'w':
 			warn_unresolved = 1;
 			break;
+		case 'B':
+			integrated_build = 1;
+			break;
 		default:
 			exit(1);
 		}
-- 
1.7.4.4

Re: [RFC] modpost: add option to allow external modules to avoid taint

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 870 bytes --]

On Wed, 2011-12-14 at 11:20 -0500, John W. Linville wrote:
> In some cases, it might be desirable to package a module from an
> external source tree alongside the base kernel.  In those cases, it
> might also be desirable to not have those modules tainting the kernel.
> 
> This patch provides a mechanism for an external module build to declare
> itself as an "integrated build".  Such a module is then treated the same
> as an intree module.
> 
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
> ---
> Any thoughts on this?  I'm thinking of adding this to Fedora kernels,
> where I have been working to integrate the compat-wireless package as
> part of the base kernel RPM.
[...]

If you're integrating it then why can't you *really* build it in-tree?

Ben.

-- 
Ben Hutchings
Computers are not intelligent.	They only think they are.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [RFC] modpost: add option to allow external modules to avoid taint

[John W. Linville]

On Wed, Dec 14, 2011 at 04:52:00PM +0000, Ben Hutchings wrote:
> On Wed, 2011-12-14 at 11:20 -0500, John W. Linville wrote:
> > In some cases, it might be desirable to package a module from an
> > external source tree alongside the base kernel.  In those cases, it
> > might also be desirable to not have those modules tainting the kernel.
> > 
> > This patch provides a mechanism for an external module build to declare
> > itself as an "integrated build".  Such a module is then treated the same
> > as an intree module.
> > 
> > Signed-off-by: John W. Linville <linville@tuxdriver.com>
> > ---
> > Any thoughts on this?  I'm thinking of adding this to Fedora kernels,
> > where I have been working to integrate the compat-wireless package as
> > part of the base kernel RPM.
> [...]
> 
> If you're integrating it then why can't you *really* build it in-tree?

Well, of course, I could.  But it would be a _huge_ waste of my time
doing so.  The compat-wireless package is already sitting there,
complete with all the necessary backporting infrastructure.  It's all
fat and juicy, waiting to be used.  Wasting time replicating that
just doesn't make any sense to my mind.

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [RFC] modpost: add option to allow external modules to avoid taint

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2511 bytes --]

On Fri, 2011-12-16 at 14:26 +1030, Rusty Russell wrote:
> On Wed, 14 Dec 2011 11:20:03 -0500, "John W. Linville" <linville@tuxdriver.com> wrote:
> > In some cases, it might be desirable to package a module from an
> > external source tree alongside the base kernel.  In those cases, it
> > might also be desirable to not have those modules tainting the kernel.
> > 
> > This patch provides a mechanism for an external module build to declare
> > itself as an "integrated build".  Such a module is then treated the same
> > as an intree module.
> > 
> > Signed-off-by: John W. Linville <linville@tuxdriver.com>
> > ---
> > Any thoughts on this?  I'm thinking of adding this to Fedora kernels,
> > where I have been working to integrate the compat-wireless package as
> > part of the base kernel RPM.
> 
> I don't think the feature is useful it it's too easy to disable.
> Experience has shown this with license tags.
> 
> We really want to indicate "out-of-support" which is only a 1:1
> mapping to out-of-tree for upstream kernels.

Who are 'we' in this instance?

> How does Debian handle this?

All the modules in Debian's kernel binary packages are built in-tree.
Backported modules are patched in as necessary.

Debian includes many packages of OOT modules, but those are supported by
their respective maintainers and not the kernel team.  So for the kernel
team, the 'O' flag does not mean 'unsupported' but may indicate that
another maintainer should handle the bug (or it may also be irrelevant
to the bug).

> Perhaps it makes more sense to use the proposed module signing stuff in
> a simplified mode to mark built-with-kernel modules (eg. just put the
> sha of known modules inside the kernel).

Unlike commercial distributions, no-one is paying Debian for support
contracts and no-one can game the system by hiding OOT modules.  So it's
probably not worthwhile for us to use module signing at all.

However, supposing we did go down this route, I would guess that
checksums for ~3000 modules take up more space than the signature
checking code.  Instead, we could perhaps generate a key pair during
build, include the public key in the kernel and then discard the private
key.  (But getting entropy would likely be a problem for the key
generation.)

Ben.

> I think we should revert this change, meanwhile, and figure out what to
> do.
> 
> Cheers,
> Rusty.
> 

-- 
Ben Hutchings
Computers are not intelligent.	They only think they are.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [RFC] modpost: add option to allow external modules to avoid taint

[Rusty Russell]

On Fri, 16 Dec 2011 04:39:49 +0000, Ben Hutchings <ben@decadent.org.uk> wrote:
Non-text part: multipart/signed
> On Fri, 2011-12-16 at 14:26 +1030, Rusty Russell wrote:
> > On Wed, 14 Dec 2011 11:20:03 -0500, "John W. Linville" <linville@tuxdriver.com> wrote:
> > We really want to indicate "out-of-support" which is only a 1:1
> > mapping to out-of-tree for upstream kernels.
> 
> Who are 'we' in this instance?

Whoever turns this flag on.  I was using friendly, inclusive language :)

> > How does Debian handle this?
> 
> All the modules in Debian's kernel binary packages are built in-tree.
> Backported modules are patched in as necessary.
> 
> Debian includes many packages of OOT modules, but those are supported by
> their respective maintainers and not the kernel team.  So for the kernel
> team, the 'O' flag does not mean 'unsupported' but may indicate that
> another maintainer should handle the bug (or it may also be irrelevant
> to the bug).

So, in your case, the kernel team want to know what's outside their
support, so this flag works well for you.

As John pointed out, it's a bit useless for them.  We could enable it
with a config option, or they could ignore it, since they're going to
module-signing route anyway.

> > Perhaps it makes more sense to use the proposed module signing stuff in
> > a simplified mode to mark built-with-kernel modules (eg. just put the
> > sha of known modules inside the kernel).
> 
> Unlike commercial distributions, no-one is paying Debian for support
> contracts and no-one can game the system by hiding OOT modules.  So it's
> probably not worthwhile for us to use module signing at all.
> 
> However, supposing we did go down this route, I would guess that
> checksums for ~3000 modules take up more space than the signature
> checking code.  Instead, we could perhaps generate a key pair during
> build, include the public key in the kernel and then discard the private
> key.  (But getting entropy would likely be a problem for the key
> generation.)

Agreed, 60k is a bit expensive for this minor feature.  

Thanks,
Rusty.