require EXPORT_SYMBOL_GPL symbols for symbol_get
Subsystems (11)
Hi all,
this series changes symbol_get to only work on EXPORT_SYMBOL_GPL as nvidia is abusing the lack of this check to bypass restrictions on importing symbols from proprietary modules.
PATCH 1/5 ARM/pxa: use EXPORT_SYMBOL_GPL for sharpsl_battery_kick
sharpsl_battery_kick is only used via symbol_get, which was only ever intended for very internal symbols like this one. Use EXPORT_SYMBOL_GPL for it so that symbol_get can enforce only being used on EXPORT_SYMBOL_GPL symbols.
--- arch/arm/mach-pxa/sharpsl_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/mach-pxa/sharpsl_pm.c +1 -1
@@ -216,7 +216,7 @@ void sharpsl_battery_kick(void) {schedule_delayed_work (&sharpsl_bat,msecs_to_jiffies (125 )); } -EXPORT_SYMBOL (sharpsl_battery_kick); +EXPORT_SYMBOL_GPL (sharpsl_battery_kick);static void sharpsl_battery_thread (struct work_struct *private_)
Signature · 1 line
2.39.2
Re: [PATCH 1/5] ARM/pxa: use EXPORT_SYMBOL_GPL for sharpsl_battery_kick
Quoted 6 lines · Christoph Hellwig
sharpsl_battery_kickis only used viasymbol_get, which was only ever intended for very internal symbols like this one. UseEXPORT_SYMBOL_GPLfor it so thatsymbol_getcan enforce only being used onEXPORT_SYMBOL_GPLsymbols.Signed-off-by Christoph Hellwig <[email protected]>
The reasoning makes sense, and the patch looks good, so feel free to take this through your tree.
Or let me know if you want a better fix. Since sharpsl_pm.c and spitz.c are no longer loadable modules and just get linked together these days, I think the variant below would be simpler (this could be cleanup up further, endlessly, of course):
arch/arm/mach-pxa/spitz.c +1 -12
@@ -518,17 +518,6 @@ static struct gpiod_lookup_table spitz_ads7846_gpio_table = { }, }; -static void spitz_bl_kick_battery (void ) -{ -void (*kick_batt)(void ); - - kick_batt =symbol_get (sharpsl_battery_kick); -if (kick_batt) { -kick_batt (); -symbol_put (sharpsl_battery_kick); - } -} -static struct gpiod_lookup_table spitz_lcdcon_gpio_table = { .dev_id ="spi2.1" , .table = {
@@ -556,7 +545,7 @@ static struct corgi_lcd_platform_data spitz_lcdcon_info = { .max_intensity = 0x2f, .default_intensity = 0x1f, .limit_mask = 0x0b, - .kick_battery = spitz_bl_kick_battery, + .kick_battery = sharpsl_battery_kick, };static struct spi_board_info spitz_spi_devices[] = {
Re: [PATCH 1/5] ARM/pxa: use EXPORT_SYMBOL_GPL for sharpsl_battery_kick
Quoted 4 lines · Arnd Bergmann
Or let me know if you want a better fix. Since
sharpsl_pm.candspitz.care no longer loadable modules and just get linked together these days, I think the variant below would be simpler (this could be cleanup up further, endlessly, of course):
That actually looks way nicer, thanks!
If you give me a singoff I'll add it to the next version.
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 2/5 net: enetc: use EXPORT_SYMBOL_GPL for enetc_phc_index
In reply to Christoph Hellwigenetc_phc_index is only used via symbol_get, which was only ever intended for very internal symbols like this one. Use EXPORT_SYMBOL_GPL for it so that symbol_get can enforce only being used on EXPORT_SYMBOL_GPL symbols.
--- drivers/net/ethernet/freescale/enetc/enetc_ptp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/freescale/enetc/enetc_ptp.c +1 -1
@@ -8,7 +8,7 @@#include "enetc.h" int enetc_phc_index =-1 ; -EXPORT_SYMBOL (enetc_phc_index); +EXPORT_SYMBOL_GPL (enetc_phc_index);static struct ptp_clock_info enetc_ptp_caps = { .owner =THIS_MODULE ,
Signature · 1 line
2.39.2
Re: [PATCH 2/5] net: enetc: use EXPORT_SYMBOL_GPL for enetc_phc_index
Quoted 6 lines · On Mon, 31 Jul 2023 10:38:03 +0200 Christoph Hellwig wrote:
enetc_phc_indexis only used viasymbol_get, which was only ever intended for very internal symbols like this one. UseEXPORT_SYMBOL_GPLfor it so thatsymbol_getcan enforce only being used onEXPORT_SYMBOL_GPLsymbols.Signed-off-by Christoph Hellwig <[email protected]>
PATCH 3/5 rtc: ds1685: use EXPORT_SYMBOL_GPL for ds1685_rtc_poweroff
In reply to Christoph Hellwigds1685_rtc_poweroff is only used externally via symbol_get, which was only ever intended for very internal symbols like this one. Use EXPORT_SYMBOL_GPL for it so that symbol_get can enforce only being used on EXPORT_SYMBOL_GPL symbols.
--- drivers/rtc/rtc-ds1685.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/rtc/rtc-ds1685.c +1 -1
@@ -1432,7 +1432,7 @@ ds1685_rtc_poweroff(struct platform_device *pdev)unreachable (); } } -EXPORT_SYMBOL (ds1685_rtc_poweroff); +EXPORT_SYMBOL_GPL (ds1685_rtc_poweroff);/* ----------------------------------------------------------------------- */
Signature · 1 line
2.39.2
Re: [PATCH 3/5] rtc: ds1685: use EXPORT_SYMBOL_GPL for ds1685_rtc_poweroff
Quoted 15 lines · Christoph Hellwig
ds1685_rtc_poweroffis only used externally viasymbol_get, which was only ever intended for very internal symbols like this one. UseEXPORT_SYMBOL_GPLfor it so thatsymbol_getcan enforce only being used onEXPORT_SYMBOL_GPLsymbols.Signed-off-by Christoph Hellwig <[email protected]>--- drivers/rtc/rtc-ds1685.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/rtc/rtc-ds1685.c +0
-0
drivers/rtc/rtc-ds1685.c ds1685_rtc_poweroff(struct platform_device *pdev) · line 1438
Quoted 8 lines
drivers/rtc/rtc-ds1685.c +1
-1@@ -1432,7 +1432,7 @@ ds1685_rtc_poweroff(struct platform_device *pdev)unreachable (); } } -EXPORT_SYMBOL (ds1685_rtc_poweroff); +EXPORT_SYMBOL_GPL (ds1685_rtc_poweroff);/* ----------------------------------------------------------------------- */
PATCH 4/5 mmc: use EXPORT_SYMBOL_GPL for mmc_detect_change
In reply to Christoph Hellwigmmc_detect_change is used via symbol_get, which was only ever intended for very internal symbols like this one. Use EXPORT_SYMBOL_GPL for it so that symbol_get can enforce only being used on EXPORT_SYMBOL_GPL symbols.
--- drivers/mmc/core/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mmc/core/core.c +1 -1
@@ -1433,7 +1433,7 @@ void mmc_detect_change(struct mmc_host *host, unsigned long delay) {_mmc_detect_change (host, delay, true); } -EXPORT_SYMBOL (mmc_detect_change); +EXPORT_SYMBOL_GPL (mmc_detect_change);void mmc_init_erase (struct mmc_card *card) {
Signature · 1 line
2.39.2
Re: [PATCH 4/5] mmc: use EXPORT_SYMBOL_GPL for mmc_detect_change
Quoted 4 lines · Christoph Hellwig
mmc_detect_changeis used viasymbol_get, which was only ever intended for very internal symbols like this one. UseEXPORT_SYMBOL_GPLfor it so thatsymbol_getcan enforce only being used onEXPORT_SYMBOL_GPLsymbols.
Btw, I really wonder if this should actually be used through symbol_get. It seems like the MIPS/alchemy boards should simply require MMC to be built in and not modular, or the IRQ handlers should move into a driver.
That would be a much less mechanical change, but this use really looks a bit odd. And makes me wonder if we should only allow symbol_get on symbols specifically marked to supported it, but that would be another incremental step.
PATCH 5/5 modules: only allow symbol_get of EXPORT_SYMBOL_GPL modules
In reply to Christoph HellwigIt 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.
--- kernel/module/internal.h | 1 + kernel/module/main.c | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-)
kernel/module/internal.h +1 -0
@@ -93,6 +93,7 @@ struct find_symbol_arg {/* Input */ const char *name;bool gplok; +bool gplonly;bool warn;/* Output */
kernel/module/main.c +12 -5
@@ -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 ();
Signature · 1 line
2.39.2
Re: [PATCH 5/5] modules: only allow symbol_get of EXPORT_SYMBOL_GPL modules
Quoted 8 lines · Christoph Hellwig
It has recently come to my attention that nvidia is circumventing the protection added in
262e6ae7081d("modules: inheritTAINT_PROPRIETARY_MODULE") by importing exports from their propriertary modules into an allegedly GPL licensed module and then rexporting them.Given that
symbol_getwas only ever inteded for tightly cooperating modules using very internal symbols it is logical to restrict it to being used onEXPORY_SYMBOL_GPLand prevent nvidia from costly DMCA
"EXPORT" :)
Quoted 8 lines
circumvention of access controls law suites.
All symbols except for four used through
symbol_getwere already exported asEXPORT_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 <[email protected]>
Thanks for fixing this hole up, it's much needed.
Simon Horman
reviewerRe: [PATCH 5/5] modules: only allow symbol_get of EXPORT_SYMBOL_GPL modules
In reply to Christoph HellwigQuoted 6 lines · Christoph Hellwig
It has recently come to my attention that nvidia is circumventing the protection added in
262e6ae7081d("modules: inheritTAINT_PROPRIETARY_MODULE") by importing exports from their propriertary modules into an allegedly GPL licensed module and then rexporting them.Given that
symbol_getwas only ever inteded for tightly cooperating
nit: inteded -> intended
...
Luis Chamberlain
maintainerRe: [PATCH 5/5] modules: only allow symbol_get of EXPORT_SYMBOL_GPL modules
In reply to Christoph HellwigQuoted 10 lines · Christoph Hellwig
--- kernel/module/internal.h | 1 + kernel/module/main.c | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-)kernel/module/internal.h +0
-0
kernel/module/internal.h struct find_symbol_arg { · line 96
Quoted 4 lines
kernel/module/internal.h +1
-0@@ -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.
kernel/module/main.c static bool find_exported_symbol_in_section(const struct symsearch *syms, · line 284
Quoted 12 lines
kernel/module/internal.h +0
-0bool warn;/* Output */ kernel/module/main.c +1
-0@@ -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.
Quoted 13 lines
kernel/module/main.c +2
-3+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,
kernel/module/main.c static void free_module(struct module *mod) · line 1305
Quoted 10 lines
kernel/module/main.c +5
-0+ .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
Greg Kroah-Hartman
Reviewed-byQuoted 5 lines · Christoph Hellwig
Hi all,
this series changes
symbol_getto only work onEXPORT_SYMBOL_GPLas nvidia is abusing the lack of this check to bypass restrictions on importing symbols from proprietary modules.
For the whole series: