| Message ID | 20230731083806.453036-6-hch@lst.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [1/5] ARM/pxa: use EXPORT_SYMBOL_GPL for sharpsl_battery_kick | expand |
On Mon, Jul 31, 2023 at 10:38:06AM +0200, Christoph Hellwig wrote: > It has recently come to my attention that nvidia is circumventing the > protection added in 262e6ae7081d ("modules: inherit > TAINT_PROPRIETARY_MODULE") by importing exports from their propriertary > modules into an allegedly GPL licensed module and then rexporting them. > > Given that symbol_get was only ever inteded for tightly cooperating > modules using very internal symbols it is logical to restrict it to > being used on EXPORY_SYMBOL_GPL and prevent nvidia from costly DMCA "EXPORT" :) > circumvention of access controls law suites. > > All symbols except for four used through symbol_get were already exported > as EXPORT_SYMBOL_GPL, and the remaining four ones were switched over in > the preparation patches. > > Fixes: 262e6ae7081d ("modules: inherit TAINT_PROPRIETARY_MODULE") > Signed-off-by: Christoph Hellwig <hch@lst.de> Thanks for fixing this hole up, it's much needed. Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Mon, Jul 31, 2023 at 10:38:06AM +0200, Christoph Hellwig wrote: > It has recently come to my attention that nvidia is circumventing the > protection added in 262e6ae7081d ("modules: inherit > TAINT_PROPRIETARY_MODULE") by importing exports from their propriertary > modules into an allegedly GPL licensed module and then rexporting them. > > Given that symbol_get was only ever inteded for tightly cooperating nit: inteded -> intended ...
On Mon, Jul 31, 2023 at 10:38:06AM +0200, Christoph Hellwig wrote: > --- > kernel/module/internal.h | 1 + > kernel/module/main.c | 17 ++++++++++++----- > 2 files changed, 13 insertions(+), 5 deletions(-) > > diff --git a/kernel/module/internal.h b/kernel/module/internal.h > index c8b7b4dcf7820d..add687c2abde8b 100644 > --- a/kernel/module/internal.h > +++ b/kernel/module/internal.h > @@ -93,6 +93,7 @@ struct find_symbol_arg { > /* Input */ > const char *name; > bool gplok; > + bool gplonly; We'd want to add here a reason or something like that to allow the caller to know why we failed if we want to provide feedback. > bool warn; > > /* Output */ > diff --git a/kernel/module/main.c b/kernel/module/main.c > index 59b1d067e52890..85d3f00ca65758 100644 > --- a/kernel/module/main.c > +++ b/kernel/module/main.c > @@ -281,6 +281,8 @@ static bool find_exported_symbol_in_section(const struct symsearch *syms, > > if (!fsa->gplok && syms->license == GPL_ONLY) > return false; > + if (fsa->gplonly && syms->license != GPL_ONLY) And set it here to something other than perhaps a default of NOT_FOUND. > + return false; > > sym = bsearch(fsa->name, syms->start, syms->stop - syms->start, > sizeof(struct kernel_symbol), cmp_name); > @@ -776,8 +778,9 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user, > @@ -1289,14 +1292,18 @@ static void free_module(struct module *mod) > void *__symbol_get(const char *symbol) > { > struct find_symbol_arg fsa = { > - .name = symbol, > - .gplok = true, > - .warn = true, > + .name = symbol, > + .gplok = true, > + .gplonly = true, > + .warn = true, > }; > > preempt_disable(); > if (!find_symbol(&fsa) || strong_try_module_get(fsa.owner)) { > preempt_enable(); > + if (fsa.gplonly) > + pr_warn("failing symbol_get of non-GPLONLY symbol %s.\n", Because here fsa.gplonly is always true here so the above warn will print even if a symbol is just not found. Luis
diff --git a/kernel/module/internal.h b/kernel/module/internal.h index c8b7b4dcf7820d..add687c2abde8b 100644 --- a/kernel/module/internal.h +++ b/kernel/module/internal.h @@ -93,6 +93,7 @@ struct find_symbol_arg { /* Input */ const char *name; bool gplok; + bool gplonly; bool warn; /* Output */ diff --git a/kernel/module/main.c b/kernel/module/main.c index 59b1d067e52890..85d3f00ca65758 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -281,6 +281,8 @@ static bool find_exported_symbol_in_section(const struct symsearch *syms, if (!fsa->gplok && syms->license == GPL_ONLY) return false; + if (fsa->gplonly && syms->license != GPL_ONLY) + return false; sym = bsearch(fsa->name, syms->start, syms->stop - syms->start, sizeof(struct kernel_symbol), cmp_name); @@ -776,8 +778,9 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user, void __symbol_put(const char *symbol) { struct find_symbol_arg fsa = { - .name = symbol, - .gplok = true, + .name = symbol, + .gplok = true, + .gplonly = true, }; preempt_disable(); @@ -1289,14 +1292,18 @@ static void free_module(struct module *mod) void *__symbol_get(const char *symbol) { struct find_symbol_arg fsa = { - .name = symbol, - .gplok = true, - .warn = true, + .name = symbol, + .gplok = true, + .gplonly = true, + .warn = true, }; preempt_disable(); if (!find_symbol(&fsa) || strong_try_module_get(fsa.owner)) { preempt_enable(); + if (fsa.gplonly) + pr_warn("failing symbol_get of non-GPLONLY symbol %s.\n", + symbol); return NULL; } preempt_enable();
It has recently come to my attention that nvidia is circumventing the protection added in 262e6ae7081d ("modules: inherit TAINT_PROPRIETARY_MODULE") by importing exports from their propriertary modules into an allegedly GPL licensed module and then rexporting them. Given that symbol_get was only ever inteded for tightly cooperating modules using very internal symbols it is logical to restrict it to being used on EXPORY_SYMBOL_GPL and prevent nvidia from costly DMCA circumvention of access controls law suites. All symbols except for four used through symbol_get were already exported as EXPORT_SYMBOL_GPL, and the remaining four ones were switched over in the preparation patches. Fixes: 262e6ae7081d ("modules: inherit TAINT_PROPRIETARY_MODULE") Signed-off-by: Christoph Hellwig <hch@lst.de> --- kernel/module/internal.h | 1 + kernel/module/main.c | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-)