Hi Jessica,
we've had a bug in our resolution of _GPL modules since day one, that
is a module can claim to be GPL licensed and use _GPL exports, while
it also depends on symbols from non-GPL modules. This is used as a
circumvention of the _GPL exports by using a small shim module using
the _GPL exports and the other functionality. A recent example can
be found here:
https://lore.kernel.org/netdev/[email protected]/T/#md514322fdfa212afe9f1d3eb4e5f7eaefece36eb
Changes since v1:
- standardize on one spelling of "license"
- fix a commit message type
Diffstat:
include/linux/module.h | 26 +++-----------------------
kernel/module.c | 46 +++++++++++++++++++++++++++++-----------------
2 files changed, 32 insertions(+), 40 deletions(-)
ref_module isn't used anywhere outside of module.c.
Signed-off-by: Christoph Hellwig <[email protected]>
---
include/linux/module.h | 1 -
kernel/module.c | 6 ++----
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 2e6670860d275f..f1fdbeef2153a8 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -657,7 +657,6 @@ static inline void __module_get(struct module *module)
#define symbol_put_addr(p) do { } while (0)
#endif /* CONFIG_MODULE_UNLOAD */
-int ref_module(struct module *a, struct module *b);
/* This is a #define so the string doesn't get put in every .o file */
#define module_name(mod) \
diff --git a/kernel/module.c b/kernel/module.c
index aa183c9ac0a256..17d64dae756c80 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -869,7 +869,7 @@ static int add_module_usage(struct module *a, struct module *b)
}
/* Module a uses b: caller needs module_mutex() */
-int ref_module(struct module *a, struct module *b)
+static int ref_module(struct module *a, struct module *b)
{
int err;
@@ -888,7 +888,6 @@ int ref_module(struct module *a, struct module *b)
}
return 0;
}
-EXPORT_SYMBOL_GPL(ref_module);
/* Clear the unload stuff of the module. */
static void module_unload_free(struct module *mod)
@@ -1169,11 +1168,10 @@ static inline void module_unload_free(struct module *mod)
{
}
-int ref_module(struct module *a, struct module *b)
+static int ref_module(struct module *a, struct module *b)
{
return strong_try_module_get(b);
}
-EXPORT_SYMBOL_GPL(ref_module);
static inline int module_unload_init(struct module *mod)
{
--
2.27.0
__module_text_address is only used by built-in code.
Signed-off-by: Christoph Hellwig <[email protected]>
---
kernel/module.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/kernel/module.c b/kernel/module.c
index feeaa9629eb179..d241866f9d4a2b 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4508,7 +4508,6 @@ struct module *__module_text_address(unsigned long addr)
}
return mod;
}
-EXPORT_SYMBOL_GPL(__module_text_address);
/* Don't grab lock, we're oopsing. */
void print_modules(void)
--
2.27.0
each_symbol_section is only used inside of module.c.
Signed-off-by: Christoph Hellwig <[email protected]>
---
include/linux/module.h | 9 ---------
kernel/module.c | 3 +--
2 files changed, 1 insertion(+), 11 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 90bdc362be3681..b79219eed83c56 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -590,15 +590,6 @@ struct symsearch {
bool unused;
};
-/*
- * Walk the exported symbol table
- *
- * Must be called with module_mutex held or preemption disabled.
- */
-bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
- struct module *owner,
- void *data), void *data);
-
/* Returns 0 and fills in value, defined and namebuf, or -ERANGE if
symnum out of range. */
int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
diff --git a/kernel/module.c b/kernel/module.c
index 84da96a6d8241c..feeaa9629eb179 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -422,7 +422,7 @@ static bool each_symbol_in_section(const struct symsearch *arr,
}
/* Returns true as soon as fn returns true, otherwise false. */
-bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
+static bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
struct module *owner,
void *data),
void *data)
@@ -484,7 +484,6 @@ bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
}
return false;
}
-EXPORT_SYMBOL_GPL(each_symbol_section);
struct find_symbol_arg {
/* Input */
--
2.27.0
find_symbol is only used in module.c.
Signed-off-by: Christoph Hellwig <[email protected]>
---
include/linux/module.h | 11 -----------
kernel/module.c | 3 +--
2 files changed, 1 insertion(+), 13 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index f1fdbeef2153a8..90bdc362be3681 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -590,17 +590,6 @@ struct symsearch {
bool unused;
};
-/*
- * Search for an exported symbol by name.
- *
- * Must be called with module_mutex held or preemption disabled.
- */
-const struct kernel_symbol *find_symbol(const char *name,
- struct module **owner,
- const s32 **crc,
- bool gplok,
- bool warn);
-
/*
* Walk the exported symbol table
*
diff --git a/kernel/module.c b/kernel/module.c
index 17d64dae756c80..84da96a6d8241c 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -585,7 +585,7 @@ static bool find_exported_symbol_in_section(const struct symsearch *syms,
/* Find an exported symbol and return it, along with, (optional) crc and
* (optional) module which owns it. Needs preempt disabled or module_mutex. */
-const struct kernel_symbol *find_symbol(const char *name,
+static const struct kernel_symbol *find_symbol(const char *name,
struct module **owner,
const s32 **crc,
bool gplok,
@@ -608,7 +608,6 @@ const struct kernel_symbol *find_symbol(const char *name,
pr_debug("Failed to find symbol %s\n", name);
return NULL;
}
-EXPORT_SYMBOL_GPL(find_symbol);
/*
* Search for module by name: must hold module_mutex (or preempt disabled
--
2.27.0
Christoph Hellwig wrote:
> we've had a bug in our resolution of _GPL modules since day one, that
> is a module can claim to be GPL licensed and use _GPL exports, while
> it also depends on symbols from non-GPL modules. This is used as a
> circumvention of the _GPL exports by using a small shim module using
> the _GPL exports and the other functionality.
This looks great. You might also consider doing the reverse: if a module
imports any EXPORT_SYMBOL_GPL symbols, any symbols that module in turn
exports shouldn't be importable by any module that doesn't explicitly
claim to be GPL-compatible. Effectively, if a module imports any
EXPORT_SYMBOL_GPL symbols, all of its exported symbols would then be
treated as EXPORT_SYMBOL_GPL.
This would catch the case of attempting to "wrap" EXPORT_SYMBOL_GPL
symbols in the other direction, by re-exporting the same or similar
functions to another module. (This would help catch mistakes, not just
intentional malice.)
- Josh Triplett
[note: private reply now to start a flame fest with the usual suspects]
On Fri, Jul 31, 2020 at 01:11:46PM -0700, [email protected] wrote:
> Christoph Hellwig wrote:
> > we've had a bug in our resolution of _GPL modules since day one, that
> > is a module can claim to be GPL licensed and use _GPL exports, while
> > it also depends on symbols from non-GPL modules. This is used as a
> > circumvention of the _GPL exports by using a small shim module using
> > the _GPL exports and the other functionality.
>
> This looks great. You might also consider doing the reverse: if a module
> imports any EXPORT_SYMBOL_GPL symbols, any symbols that module in turn
> exports shouldn't be importable by any module that doesn't explicitly
> claim to be GPL-compatible. Effectively, if a module imports any
> EXPORT_SYMBOL_GPL symbols, all of its exported symbols would then be
> treated as EXPORT_SYMBOL_GPL.
>
> This would catch the case of attempting to "wrap" EXPORT_SYMBOL_GPL
> symbols in the other direction, by re-exporting the same or similar
> functions to another module. (This would help catch mistakes, not just
> intentional malice.)
I'd personally 100% agree with that, but I'd rather clear it with Linus
privately first. This would basically make most of the usual
modular subsystems unavailable to proprietary modules as all of them
use _GPL driver core exports, and I suspect he'd cave into the screaming.
On July 31, 2020 11:53:08 PM PDT, Christoph Hellwig <[email protected]> wrote:
>[note: private reply now to start a flame fest with the usual suspects]
[You still CCed LKML.]
>On Fri, Jul 31, 2020 at 01:11:46PM -0700, [email protected] wrote:
>> Christoph Hellwig wrote:
>> > we've had a bug in our resolution of _GPL modules since day one, that
>> > is a module can claim to be GPL licensed and use _GPL exports, while
>> > it also depends on symbols from non-GPL modules. This is used as a
>> > circumvention of the _GPL exports by using a small shim module using
>> > the _GPL exports and the other functionality.
>>
>> This looks great. You might also consider doing the reverse: if a module
>> imports any EXPORT_SYMBOL_GPL symbols, any symbols that module in turn
>> exports shouldn't be importable by any module that doesn't explicitly
>> claim to be GPL-compatible. Effectively, if a module imports any
>> EXPORT_SYMBOL_GPL symbols, all of its exported symbols would then be
>> treated as EXPORT_SYMBOL_GPL.
>>
>> This would catch the case of attempting to "wrap" EXPORT_SYMBOL_GPL
>> symbols in the other direction, by re-exporting the same or similar
>> functions to another module. (This would help catch mistakes, not just
>> intentional malice.)
>
>I'd personally 100% agree with that, but I'd rather clear it with Linus
>privately first. This would basically make most of the usual
>modular subsystems unavailable to proprietary modules as all of them
>use _GPL driver core exports, and I suspect he'd cave into the screaming.
As a start, what about applying that logic specifically to out-of-tree modules? That would address the shim problem. The justification would be that in-tree modules have at least gone through some level of review on what they're exporting.
(Standard disclaimer: suggesting enhancements to the symbol licensing framework should not be taken as implicit endorsement of any legitimacy for non-GPL-compatible modules.)