GPL-only symbols issue

GPL-only symbols issue

[Jan Beulich]

Sam,

would it seem reasonable a request to detect imports of GPL-only symbols by non-GPL modules also at build time rather
than only at run time, and at least warn about such?

Thanks, Jan

Re: GPL-only symbols issue

[Sam Ravnborg]

On Mon, May 08, 2006 at 09:12:15AM +0200, Jan Beulich wrote:
> Sam,
> 
> would it seem reasonable a request to detect imports of GPL-only symbols by non-GPL modules also at build time rather
> than only at run time, and at least warn about such?

It should be trivial since we have the information. But I'm not the one
to say if it is reasonable.
If you do not beat me I can try to cook something up within a few
weeks (busy with work-work atm).

	Sam

Re: GPL-only symbols issue

[Greg KH]

On Mon, May 08, 2006 at 09:12:15AM +0200, Jan Beulich wrote:
> Sam,
> 
> would it seem reasonable a request to detect imports of GPL-only
> symbols by non-GPL modules also at build time rather than only at run
> time, and at least warn about such?

Ram has some tools that might catch this kind of thing.  He's posted his
scripts to lkml in the past, try looking in the archives.

thanks,

greg k-h

Re: GPL-only symbols issue

[Ram Pai]

On Mon, 2006-05-08 at 21:25 -0700, Greg KH wrote:
> On Mon, May 08, 2006 at 09:12:15AM +0200, Jan Beulich wrote:
> > Sam,
> > 
> > would it seem reasonable a request to detect imports of GPL-only
> > symbols by non-GPL modules also at build time rather than only at run
> > time, and at least warn about such?
> 
> Ram has some tools that might catch this kind of thing.  He's posted his
> scripts to lkml in the past, try looking in the archives.

The patches are at

http://sudhaa.com/~ram/misc/kernelpatch

The patch of interest for you would be modpost.patch
I have a script and some code that can poke into a given .ko file and
warn against symbols that don't match what the kernel exports. 

I can post that code if there is interest in that functionality,
RP

> 
> thanks,

> greg k-h

Re: GPL-only symbols issue

[Philip R. Auld]

Hi,

Rumor has it that on Mon, May 08, 2006 at 10:57:18PM -0700 Ram Pai said:
> On Mon, 2006-05-08 at 21:25 -0700, Greg KH wrote:
> > On Mon, May 08, 2006 at 09:12:15AM +0200, Jan Beulich wrote:
> > > Sam,
> > > 
> > > would it seem reasonable a request to detect imports of GPL-only
> > > symbols by non-GPL modules also at build time rather than only at run
> > > time, and at least warn about such?
> > 
> > Ram has some tools that might catch this kind of thing.  He's posted his
> > scripts to lkml in the past, try looking in the archives.
> 
> The patches are at
> 
> http://sudhaa.com/~ram/misc/kernelpatch
> 
> The patch of interest for you would be modpost.patch
> I have a script and some code that can poke into a given .ko file and
> warn against symbols that don't match what the kernel exports. 
> 
> I can post that code if there is interest in that functionality,

I'd love to see it. 

Thanks,

Phil

> RP
> 
> > 
> > thanks,
> 
> > greg k-h
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: GPL-only symbols issue

[Andreas Gruenbacher]

On Tuesday, 09 May 2006 07:57, Ram Pai wrote:
> On Mon, 2006-05-08 at 21:25 -0700, Greg KH wrote:
> > On Mon, May 08, 2006 at 09:12:15AM +0200, Jan Beulich wrote:
> > > Sam,
> > >
> > > would it seem reasonable a request to detect imports of GPL-only
> > > symbols by non-GPL modules also at build time rather than only at run
> > > time, and at least warn about such?

I would appreciate such a check: it avoids unnecessary errors later. See 
my proposed patch in the next message.

> > Ram has some tools that might catch this kind of thing.  He's posted his
> > scripts to lkml in the past, try looking in the archives.
>
> The patches are at
>
> http://sudhaa.com/~ram/misc/kernelpatch
>
> The patch of interest for you would be modpost.patch
> I have a script and some code that can poke into a given .ko file and
> warn against symbols that don't match what the kernel exports.

I like the extension that modpost.patch adds. Statistics gathering like your 
other patch does doesn't need to be in the mainline kernel IMVHO, but I don't 
really mind. Here is a bugfix to modpost.patch:

Index: linux-2.6.16/scripts/mod/modpost.c
===================================================================
--- linux-2.6.16.orig/scripts/mod/modpost.c
+++ linux-2.6.16/scripts/mod/modpost.c
@@ -811,8 +817,12 @@ static void read_dump(const char *fname,
 					*d != '\0')
 			goto fail;
 
-		if (!(strcmp(export, "EXPORT_GPL_ONLY")))
+		if (strcmp(export, "EXPORT_SYMBOL_GPL") == 0)
 			export_type = 1;
+		else if (strcmp(export, "EXPORT_SYMBOL") == 0)
+			export_type = 0;
+		else
+			goto fail;
 
 		if (!(mod = find_module(modname))) {
 			if (is_vmlinux(modname)) {

Andreas

[PATCH] Check for license compliance at build time

[Andreas Gruenbacher]

This patch on to of Ram Pai's modpost.diff patch at 
http://sudhaa.com/~ram/misc/kernelpatch implements license compliance testing 
in modpost. This prevents kbuild from producing modules that won't load.

Signed-off-by: Andreas Gruenbacher <agruen@suse.de>

Index: linux-2.6.16/include/linux/license.h
===================================================================
--- /dev/null
+++ linux-2.6.16/include/linux/license.h
@@ -0,0 +1,14 @@
+#ifndef __LICENSE_H
+#define __LICENSE_H
+
+static inline int license_is_gpl_compatible(const char *license)
+{
+	return (strcmp(license, "GPL") == 0
+		|| strcmp(license, "GPL v2") == 0
+		|| strcmp(license, "GPL and additional rights") == 0
+		|| strcmp(license, "Dual BSD/GPL") == 0
+		|| strcmp(license, "Dual MIT/GPL") == 0
+		|| strcmp(license, "Dual MPL/GPL") == 0);
+}
+
+#endif
Index: linux-2.6.16/kernel/module.c
===================================================================
--- linux-2.6.16.orig/kernel/module.c
+++ linux-2.6.16/kernel/module.c
@@ -43,6 +43,7 @@
 #include <asm/uaccess.h>
 #include <asm/semaphore.h>
 #include <asm/cacheflush.h>
+#include <linux/license.h>
 
 #if 0
 #define DEBUGP printk
@@ -1248,16 +1249,6 @@ static void layout_sections(struct modul
 	}
 }
 
-static inline int license_is_gpl_compatible(const char *license)
-{
-	return (strcmp(license, "GPL") == 0
-		|| strcmp(license, "GPL v2") == 0
-		|| strcmp(license, "GPL and additional rights") == 0
-		|| strcmp(license, "Dual BSD/GPL") == 0
-		|| strcmp(license, "Dual MIT/GPL") == 0
-		|| strcmp(license, "Dual MPL/GPL") == 0);
-}
-
 static void set_license(struct module *mod, const char *license)
 {
 	if (!license)
Index: linux-2.6.16/scripts/mod/modpost.c
===================================================================
--- linux-2.6.16.orig/scripts/mod/modpost.c
+++ linux-2.6.16/scripts/mod/modpost.c
@@ -13,6 +13,7 @@
 
 #include <ctype.h>
 #include "modpost.h"
+#include "../../include/linux/license.h"
 
 /* Are we using CONFIG_MODVERSIONS? */
 int modversions = 0;
@@ -101,6 +102,7 @@ static struct module *new_module(char *m
 
 	/* add to list */
 	mod->name = p;
+	mod->gpl_compatible = -1;
 	mod->next = modules;
 	modules = mod;
 
@@ -454,13 +456,18 @@ static char *next_string(char *string, u
 	return string;
 }
 
-static char *get_modinfo(void *modinfo, unsigned long modinfo_len,
-			 const char *tag)
+static char *get_next_modinfo(void *modinfo, unsigned long modinfo_len,
+			      const char *tag, char *info)
 {
 	char *p;
 	unsigned int taglen = strlen(tag);
 	unsigned long size = modinfo_len;
 
+	if (info) {
+		size -= info - (char *)modinfo;
+		modinfo = next_string(info, &size);
+	}
+
 	for (p = modinfo; p; p = next_string(p, &size)) {
 		if (strncmp(p, tag, taglen) == 0 && p[taglen] == '=')
 			return p + taglen + 1;
@@ -468,6 +475,13 @@ static char *get_modinfo(void *modinfo, 
 	return NULL;
 }
 
+static char *get_modinfo(void *modinfo, unsigned long modinfo_len,
+			 const char *tag)
+
+{
+	return get_next_modinfo(modinfo, modinfo_len, tag, NULL);
+}
+
 /**
  * Test if string s ends in string sub
  * return 0 if match
@@ -888,6 +902,7 @@ static void read_symbols(char *modname)
 {
 	const char *symname;
 	char *version;
+	char *license;
 	struct module *mod;
 	struct elf_info info = { };
 	Elf_Sym *sym;
@@ -903,6 +918,18 @@ static void read_symbols(char *modname)
 		mod->skip = 1;
 	}
 
+	license = get_modinfo(info.modinfo, info.modinfo_len, "license");
+	while (license) {
+		if (license_is_gpl_compatible(license))
+			mod->gpl_compatible = 1;
+		else {
+			mod->gpl_compatible = 0;
+			break;
+		}
+		license = get_next_modinfo(info.modinfo, info.modinfo_len,
+					   "license", license);
+	}
+
 	for (sym = info.symtab_start; sym < info.symtab_stop; sym++) {
 		symname = info.strtab + sym->st_name;
 
@@ -959,6 +986,31 @@ void buf_write(struct buffer *buf, const
 	buf->pos += len;
 }
 
+void check_license(struct module *mod)
+{
+	struct symbol *s, *exp;
+
+	for (s = mod->unres; s; s = s->next) {
+		if (mod->gpl_compatible == 1) {
+			/* GPL-compatible modules may use all symbols */
+			continue;
+		}
+		exp = find_symbol(s->name);
+		if (!exp || exp->module == mod)
+			continue;
+		if (exp->export_type == 1) {
+			const char *basename = strrchr(mod->name, '/');
+			if (basename)
+				basename++;
+
+			fatal("modpost: GPL-incompatible module %s uses the "
+			      "GPL-only symbol %s\n",
+			      basename ? basename : mod->name,
+			      exp->name);
+		}
+        }
+}
+
 /**
  * Header for the generated file
  **/
@@ -1244,6 +1296,12 @@ int main(int argc, char **argv)
 	for (mod = modules; mod; mod = mod->next) {
 		if (mod->skip)
 			continue;
+		check_license(mod);
+	}
+
+	for (mod = modules; mod; mod = mod->next) {
+		if (mod->skip)
+			continue;
 
 		buf.pos = 0;
 
Index: linux-2.6.16/scripts/mod/modpost.h
===================================================================
--- linux-2.6.16.orig/scripts/mod/modpost.h
+++ linux-2.6.16/scripts/mod/modpost.h
@@ -81,6 +81,7 @@ buf_write(struct buffer *buf, const char
 struct module {
 	struct module *next;
 	const char *name;
+	int gpl_compatible;
 	struct symbol *unres;
 	int seen;
 	int skip;

Re: [PATCH] Check for license compliance at build time

[Ram Pai]

On Tue, 2006-05-09 at 19:31 +0200, Andreas Gruenbacher wrote:
> This patch on to of Ram Pai's modpost.diff patch at 
> http://sudhaa.com/~ram/misc/kernelpatch implements license compliance testing 
> in modpost. This prevents kbuild from producing modules that won't load.

Yes, I like this patch. Its a early warning system for a module having
no chance of getting inserted into the kernel.

Sam : do you want all these patches submitted togather? 

RP


> 
> Signed-off-by: Andreas Gruenbacher <agruen@suse.de>
> 
> Index: linux-2.6.16/include/linux/license.h
> ===================================================================
> --- /dev/null
> +++ linux-2.6.16/include/linux/license.h
> @@ -0,0 +1,14 @@
> +#ifndef __LICENSE_H
> +#define __LICENSE_H
> +
> +static inline int license_is_gpl_compatible(const char *license)
> +{
> +	return (strcmp(license, "GPL") == 0
> +		|| strcmp(license, "GPL v2") == 0
> +		|| strcmp(license, "GPL and additional rights") == 0
> +		|| strcmp(license, "Dual BSD/GPL") == 0
> +		|| strcmp(license, "Dual MIT/GPL") == 0
> +		|| strcmp(license, "Dual MPL/GPL") == 0);
> +}
> +
> +#endif
> Index: linux-2.6.16/kernel/module.c
> ===================================================================
> --- linux-2.6.16.orig/kernel/module.c
> +++ linux-2.6.16/kernel/module.c
> @@ -43,6 +43,7 @@
>  #include <asm/uaccess.h>
>  #include <asm/semaphore.h>
>  #include <asm/cacheflush.h>
> +#include <linux/license.h>
>  
>  #if 0
>  #define DEBUGP printk
> @@ -1248,16 +1249,6 @@ static void layout_sections(struct modul
>  	}
>  }
>  
> -static inline int license_is_gpl_compatible(const char *license)
> -{
> -	return (strcmp(license, "GPL") == 0
> -		|| strcmp(license, "GPL v2") == 0
> -		|| strcmp(license, "GPL and additional rights") == 0
> -		|| strcmp(license, "Dual BSD/GPL") == 0
> -		|| strcmp(license, "Dual MIT/GPL") == 0
> -		|| strcmp(license, "Dual MPL/GPL") == 0);
> -}
> -
>  static void set_license(struct module *mod, const char *license)
>  {
>  	if (!license)
> Index: linux-2.6.16/scripts/mod/modpost.c
> ===================================================================
> --- linux-2.6.16.orig/scripts/mod/modpost.c
> +++ linux-2.6.16/scripts/mod/modpost.c
> @@ -13,6 +13,7 @@
>  
>  #include <ctype.h>
>  #include "modpost.h"
> +#include "../../include/linux/license.h"
>  
>  /* Are we using CONFIG_MODVERSIONS? */
>  int modversions = 0;
> @@ -101,6 +102,7 @@ static struct module *new_module(char *m
>  
>  	/* add to list */
>  	mod->name = p;
> +	mod->gpl_compatible = -1;
>  	mod->next = modules;
>  	modules = mod;
>  
> @@ -454,13 +456,18 @@ static char *next_string(char *string, u
>  	return string;
>  }
>  
> -static char *get_modinfo(void *modinfo, unsigned long modinfo_len,
> -			 const char *tag)
> +static char *get_next_modinfo(void *modinfo, unsigned long modinfo_len,
> +			      const char *tag, char *info)
>  {
>  	char *p;
>  	unsigned int taglen = strlen(tag);
>  	unsigned long size = modinfo_len;
>  
> +	if (info) {
> +		size -= info - (char *)modinfo;
> +		modinfo = next_string(info, &size);
> +	}
> +
>  	for (p = modinfo; p; p = next_string(p, &size)) {
>  		if (strncmp(p, tag, taglen) == 0 && p[taglen] == '=')
>  			return p + taglen + 1;
> @@ -468,6 +475,13 @@ static char *get_modinfo(void *modinfo, 
>  	return NULL;
>  }
>  
> +static char *get_modinfo(void *modinfo, unsigned long modinfo_len,
> +			 const char *tag)
> +
> +{
> +	return get_next_modinfo(modinfo, modinfo_len, tag, NULL);
> +}
> +
>  /**
>   * Test if string s ends in string sub
>   * return 0 if match
> @@ -888,6 +902,7 @@ static void read_symbols(char *modname)
>  {
>  	const char *symname;
>  	char *version;
> +	char *license;
>  	struct module *mod;
>  	struct elf_info info = { };
>  	Elf_Sym *sym;
> @@ -903,6 +918,18 @@ static void read_symbols(char *modname)
>  		mod->skip = 1;
>  	}
>  
> +	license = get_modinfo(info.modinfo, info.modinfo_len, "license");
> +	while (license) {
> +		if (license_is_gpl_compatible(license))
> +			mod->gpl_compatible = 1;
> +		else {
> +			mod->gpl_compatible = 0;
> +			break;
> +		}
> +		license = get_next_modinfo(info.modinfo, info.modinfo_len,
> +					   "license", license);
> +	}
> +
>  	for (sym = info.symtab_start; sym < info.symtab_stop; sym++) {
>  		symname = info.strtab + sym->st_name;
>  
> @@ -959,6 +986,31 @@ void buf_write(struct buffer *buf, const
>  	buf->pos += len;
>  }
>  
> +void check_license(struct module *mod)
> +{
> +	struct symbol *s, *exp;
> +
> +	for (s = mod->unres; s; s = s->next) {
> +		if (mod->gpl_compatible == 1) {
> +			/* GPL-compatible modules may use all symbols */
> +			continue;
> +		}
> +		exp = find_symbol(s->name);
> +		if (!exp || exp->module == mod)
> +			continue;
> +		if (exp->export_type == 1) {
> +			const char *basename = strrchr(mod->name, '/');
> +			if (basename)
> +				basename++;
> +
> +			fatal("modpost: GPL-incompatible module %s uses the "
> +			      "GPL-only symbol %s\n",
> +			      basename ? basename : mod->name,
> +			      exp->name);
> +		}
> +        }
> +}
> +
>  /**
>   * Header for the generated file
>   **/
> @@ -1244,6 +1296,12 @@ int main(int argc, char **argv)
>  	for (mod = modules; mod; mod = mod->next) {
>  		if (mod->skip)
>  			continue;
> +		check_license(mod);
> +	}
> +
> +	for (mod = modules; mod; mod = mod->next) {
> +		if (mod->skip)
> +			continue;
>  
>  		buf.pos = 0;
>  
> Index: linux-2.6.16/scripts/mod/modpost.h
> ===================================================================
> --- linux-2.6.16.orig/scripts/mod/modpost.h
> +++ linux-2.6.16/scripts/mod/modpost.h
> @@ -81,6 +81,7 @@ buf_write(struct buffer *buf, const char
>  struct module {
>  	struct module *next;
>  	const char *name;
> +	int gpl_compatible;
>  	struct symbol *unres;
>  	int seen;
>  	int skip;

Re: [PATCH] Check for license compliance at build time

[Sam Ravnborg]

On Tue, May 09, 2006 at 01:55:58PM -0700, Ram Pai wrote:
> On Tue, 2006-05-09 at 19:31 +0200, Andreas Gruenbacher wrote:
> > This patch on to of Ram Pai's modpost.diff patch at 
> > http://sudhaa.com/~ram/misc/kernelpatch implements license compliance testing 
> > in modpost. This prevents kbuild from producing modules that won't load.
> 
> Yes, I like this patch. Its a early warning system for a module having
> no chance of getting inserted into the kernel.
> 
> Sam : do you want all these patches submitted togather? 
Yes - please do. I have the originals queued up but it would be helpfull
to get it as a new serie.

And integrate the fix from Andreas so it is not a separate commit.

	Sam

Re: GPL-only symbols issue

[Sam Ravnborg]

On Mon, May 08, 2006 at 09:25:00PM -0700, Greg KH wrote:
> On Mon, May 08, 2006 at 09:12:15AM +0200, Jan Beulich wrote:
> > Sam,
> > 
> > would it seem reasonable a request to detect imports of GPL-only
> > symbols by non-GPL modules also at build time rather than only at run
> > time, and at least warn about such?
> 
> Ram has some tools that might catch this kind of thing.  He's posted his
> scripts to lkml in the past, try looking in the archives.

Responding to an old post here.

I have only recently integrated Ram's patches except for the kbuild
integration bits. On top of this Andreas' license compatibility thing so
in effect:
building a GPL module (or compatible) => no change
building a GPL-incompatible module =>
                           warn if module uses EXPORT_SYMBOL_GPL_FUTURE
                           error out if module uses EXPORT_SYMBOL_GPL

Patches are in kbuild.git.
The script to show module export statistics is not yet merged, I've
asked Ram to fix a few things first.

	Sam