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
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
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
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
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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
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
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 <[email protected]>
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;
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 <[email protected]>
>
> 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;
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