Re: [PATCH v2] module: Ignore _GLOBAL_OFFSET_TABLE_ when warning for undefined symbols
On Thu, Jan 14, 2021 at 11:04 PM Marco Elver wrote:
>
> On Thu, 14 Jan 2021 at 22:54, Fangrui Song wrote:
> > clang-12 -fno-pic (since
> > https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de008232da3f1d6)
> > can emit `call __stack_chk_fail@PLT` instead of `call __stack_chk_fail`
> > on x86. The two forms should have identical behaviors on x86-64 but the
> > former causes GNU as<2.37 to produce an unreferenced undefined symbol
> > _GLOBAL_OFFSET_TABLE_.
> >
> > (On x86-32, there is an R_386_PC32 vs R_386_PLT32 difference but the
> > linker behavior is identical as far as Linux kernel is concerned.)
> >
> > Simply ignore _GLOBAL_OFFSET_TABLE_ for now, like what
> > scripts/mod/modpost.c:ignore_undef_symbol does. This also fixes the
> > problem for gcc/clang -fpie and -fpic, which may emit `call foo@PLT` for
> > external function calls on x86.
> >
> > Note: ld -z defs and dynamic loaders do not error for unreferenced
> > undefined symbols so the module loader is reading too much. If we ever
> > need to ignore more symbols, the code should be refactored to ignore
> > unreferenced symbols.
> >
> > Reported-by: Marco Elver
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1250
> > Signed-off-by: Fangrui Song
>
> Tested-by: Marco Elver
>
> Thank you for the patch!
>
> > ---
> > kernel/module.c | 20 ++--
> > 1 file changed, 18 insertions(+), 2 deletions(-)
> > ---
> > Changes in v2:
> > * Fix Marco's email address
> > * Add a function ignore_undef_symbol similar to
> > scripts/mod/modpost.c:ignore_undef_symbol
> >
> > diff --git a/kernel/module.c b/kernel/module.c
> > index 4bf30e4b3eaa..278f5129bde2 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -2348,6 +2348,20 @@ static int verify_exported_symbols(struct module
> > *mod)
> > return 0;
> > }
> >
> > +static int ignore_undef_symbol(Elf_Half emachine, const char *name)
>
> Why not 'bool' return-type?
Will use bool and false in v3.
> > +{
> > + /* On x86, PIC code and Clang non-PIC code may have call foo@PLT.
> > GNU as
>
> Not sure if checkpatch.pl warns about this, but this multi-line
> comment does not follow the normal kernel-style (see elsewhere in
> file):
It doesn't warn about this. (The commit description warning cannot be
fixed, even if I place the closing paren on the next line.)
% ./scripts/checkpatch.pl
v2-0001-module-Ignore-_GLOBAL_OFFSET_TABLE_-when-warning-.patch
WARNING: Possible unwrapped commit description (prefer a maximum 75
chars per line)
#8:
https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de008232da3f1d6)
total: 0 errors, 1 warnings, 32 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
v2-0001-module-Ignore-_GLOBAL_OFFSET_TABLE_-when-warning-.patch has
style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
> /*
> * ...
> */
>
> > +* before 2.37 produces an unreferenced _GLOBAL_OFFSET_TABLE_ on
> > x86-64.
> > +* i386 has a similar problem but may not deserve a fix.
> > +*
> > +* If we ever have to ignore many symbols, consider refactoring the
> > code to
> > +* only warn if referenced by a relocation.
> > +*/
> > + if (emachine == EM_386 || emachine == EM_X86_64)
> > + return !strcmp(name, "_GLOBAL_OFFSET_TABLE_");
> > + return 0;
> > +}
> > +
> > /* Change all symbols so that st_value encodes the pointer directly. */
> > static int simplify_symbols(struct module *mod, const struct load_info
> > *info)
> > {
> > @@ -2395,8 +2409,10 @@ static int simplify_symbols(struct module *mod,
> > const struct load_info *info)
> > break;
> > }
> >
> > - /* Ok if weak. */
> > - if (!ksym && ELF_ST_BIND(sym[i].st_info) ==
> > STB_WEAK)
> > + /* Ok if weak or ignored. */
> > + if (!ksym &&
> > + (ELF_ST_BIND(sym[i].st_info) == STB_WEAK ||
> > +ignore_undef_symbol(info->hdr->e_machine,
> > name)))
> > break;
> >
> > ret = PTR_ERR(ksym) ?: -ENOENT;
> > --
> > 2.30.0.296.g2bfb1c46d8-goog
> >
--
宋方睿
Re: [PATCH v2] module: Ignore _GLOBAL_OFFSET_TABLE_ when warning for undefined symbols
+++ Marco Elver [15/01/21 08:03 +0100]:
On Thu, 14 Jan 2021 at 22:54, Fangrui Song wrote:
clang-12 -fno-pic (since
https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de008232da3f1d6)
can emit `call __stack_chk_fail@PLT` instead of `call __stack_chk_fail`
on x86. The two forms should have identical behaviors on x86-64 but the
former causes GNU as<2.37 to produce an unreferenced undefined symbol
_GLOBAL_OFFSET_TABLE_.
(On x86-32, there is an R_386_PC32 vs R_386_PLT32 difference but the
linker behavior is identical as far as Linux kernel is concerned.)
Simply ignore _GLOBAL_OFFSET_TABLE_ for now, like what
scripts/mod/modpost.c:ignore_undef_symbol does. This also fixes the
problem for gcc/clang -fpie and -fpic, which may emit `call foo@PLT` for
external function calls on x86.
Note: ld -z defs and dynamic loaders do not error for unreferenced
undefined symbols so the module loader is reading too much. If we ever
need to ignore more symbols, the code should be refactored to ignore
unreferenced symbols.
Reported-by: Marco Elver
Link: https://github.com/ClangBuiltLinux/linux/issues/1250
Signed-off-by: Fangrui Song
Tested-by: Marco Elver
Thank you for the patch!
---
kernel/module.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
---
Changes in v2:
* Fix Marco's email address
* Add a function ignore_undef_symbol similar to
scripts/mod/modpost.c:ignore_undef_symbol
diff --git a/kernel/module.c b/kernel/module.c
index 4bf30e4b3eaa..278f5129bde2 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2348,6 +2348,20 @@ static int verify_exported_symbols(struct module *mod)
return 0;
}
+static int ignore_undef_symbol(Elf_Half emachine, const char *name)
Why not 'bool' return-type?
+{
+ /* On x86, PIC code and Clang non-PIC code may have call foo@PLT. GNU as
Not sure if checkpatch.pl warns about this, but this multi-line
comment does not follow the normal kernel-style (see elsewhere in
file):
/*
* ...
*/
+1 to Marco's comments. Otherwise, patch looks good to me.
Thanks Fangrui!
Jessica
Re: [PATCH v2] module: Ignore _GLOBAL_OFFSET_TABLE_ when warning for undefined symbols
+++ Nick Desaulniers [14/01/21 14:01 -0800]: On Thu, Jan 14, 2021 at 1:54 PM 'Fangrui Song' via Clang Built Linux wrote: clang-12 -fno-pic (since https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de008232da3f1d6) can emit `call __stack_chk_fail@PLT` instead of `call __stack_chk_fail` on x86. The two forms should have identical behaviors on x86-64 but the former causes GNU as<2.37 to produce an unreferenced undefined symbol _GLOBAL_OFFSET_TABLE_. (On x86-32, there is an R_386_PC32 vs R_386_PLT32 difference but the linker behavior is identical as far as Linux kernel is concerned.) Simply ignore _GLOBAL_OFFSET_TABLE_ for now, like what scripts/mod/modpost.c:ignore_undef_symbol does. This also fixes the problem for gcc/clang -fpie and -fpic, which may emit `call foo@PLT` for external function calls on x86. Note: ld -z defs and dynamic loaders do not error for unreferenced undefined symbols so the module loader is reading too much. If we ever need to ignore more symbols, the code should be refactored to ignore unreferenced symbols. Reported-by: Marco Elver Link: https://github.com/ClangBuiltLinux/linux/issues/1250 Signed-off-by: Fangrui Song Thanks for the patch. Reviewed-by: Nick Desaulniers Jessica, would you mind adding when applying: Cc: as I suspect we might want this fixed in stable tree's branches, too. It might of interest to add: Link: https://sourceware.org/bugzilla/show_bug.cgi?id=27178 too. Sure, will do! Thanks, Jessica
Re: [PATCH v2] module: Ignore _GLOBAL_OFFSET_TABLE_ when warning for undefined symbols
On Thu, 14 Jan 2021 at 22:54, Fangrui Song wrote:
> clang-12 -fno-pic (since
> https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de008232da3f1d6)
> can emit `call __stack_chk_fail@PLT` instead of `call __stack_chk_fail`
> on x86. The two forms should have identical behaviors on x86-64 but the
> former causes GNU as<2.37 to produce an unreferenced undefined symbol
> _GLOBAL_OFFSET_TABLE_.
>
> (On x86-32, there is an R_386_PC32 vs R_386_PLT32 difference but the
> linker behavior is identical as far as Linux kernel is concerned.)
>
> Simply ignore _GLOBAL_OFFSET_TABLE_ for now, like what
> scripts/mod/modpost.c:ignore_undef_symbol does. This also fixes the
> problem for gcc/clang -fpie and -fpic, which may emit `call foo@PLT` for
> external function calls on x86.
>
> Note: ld -z defs and dynamic loaders do not error for unreferenced
> undefined symbols so the module loader is reading too much. If we ever
> need to ignore more symbols, the code should be refactored to ignore
> unreferenced symbols.
>
> Reported-by: Marco Elver
> Link: https://github.com/ClangBuiltLinux/linux/issues/1250
> Signed-off-by: Fangrui Song
Tested-by: Marco Elver
Thank you for the patch!
> ---
> kernel/module.c | 20 ++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
> ---
> Changes in v2:
> * Fix Marco's email address
> * Add a function ignore_undef_symbol similar to
> scripts/mod/modpost.c:ignore_undef_symbol
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 4bf30e4b3eaa..278f5129bde2 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2348,6 +2348,20 @@ static int verify_exported_symbols(struct module *mod)
> return 0;
> }
>
> +static int ignore_undef_symbol(Elf_Half emachine, const char *name)
Why not 'bool' return-type?
> +{
> + /* On x86, PIC code and Clang non-PIC code may have call foo@PLT. GNU
> as
Not sure if checkpatch.pl warns about this, but this multi-line
comment does not follow the normal kernel-style (see elsewhere in
file):
/*
* ...
*/
> +* before 2.37 produces an unreferenced _GLOBAL_OFFSET_TABLE_ on
> x86-64.
> +* i386 has a similar problem but may not deserve a fix.
> +*
> +* If we ever have to ignore many symbols, consider refactoring the
> code to
> +* only warn if referenced by a relocation.
> +*/
> + if (emachine == EM_386 || emachine == EM_X86_64)
> + return !strcmp(name, "_GLOBAL_OFFSET_TABLE_");
> + return 0;
> +}
> +
> /* Change all symbols so that st_value encodes the pointer directly. */
> static int simplify_symbols(struct module *mod, const struct load_info *info)
> {
> @@ -2395,8 +2409,10 @@ static int simplify_symbols(struct module *mod, const
> struct load_info *info)
> break;
> }
>
> - /* Ok if weak. */
> - if (!ksym && ELF_ST_BIND(sym[i].st_info) == STB_WEAK)
> + /* Ok if weak or ignored. */
> + if (!ksym &&
> + (ELF_ST_BIND(sym[i].st_info) == STB_WEAK ||
> +ignore_undef_symbol(info->hdr->e_machine, name)))
> break;
>
> ret = PTR_ERR(ksym) ?: -ENOENT;
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
Re: [PATCH v2] module: Ignore _GLOBAL_OFFSET_TABLE_ when warning for undefined symbols
On Thu, Jan 14, 2021 at 1:54 PM 'Fangrui Song' via Clang Built Linux
wrote:
>
> clang-12 -fno-pic (since
> https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de008232da3f1d6)
> can emit `call __stack_chk_fail@PLT` instead of `call __stack_chk_fail`
> on x86. The two forms should have identical behaviors on x86-64 but the
> former causes GNU as<2.37 to produce an unreferenced undefined symbol
> _GLOBAL_OFFSET_TABLE_.
>
> (On x86-32, there is an R_386_PC32 vs R_386_PLT32 difference but the
> linker behavior is identical as far as Linux kernel is concerned.)
>
> Simply ignore _GLOBAL_OFFSET_TABLE_ for now, like what
> scripts/mod/modpost.c:ignore_undef_symbol does. This also fixes the
> problem for gcc/clang -fpie and -fpic, which may emit `call foo@PLT` for
> external function calls on x86.
>
> Note: ld -z defs and dynamic loaders do not error for unreferenced
> undefined symbols so the module loader is reading too much. If we ever
> need to ignore more symbols, the code should be refactored to ignore
> unreferenced symbols.
>
> Reported-by: Marco Elver
> Link: https://github.com/ClangBuiltLinux/linux/issues/1250
> Signed-off-by: Fangrui Song
Thanks for the patch.
Reviewed-by: Nick Desaulniers
Jessica, would you mind adding when applying:
Cc:
as I suspect we might want this fixed in stable tree's branches, too.
It might of interest to add:
Link: https://sourceware.org/bugzilla/show_bug.cgi?id=27178
too.
> ---
> kernel/module.c | 20 ++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
> ---
> Changes in v2:
> * Fix Marco's email address
> * Add a function ignore_undef_symbol similar to
> scripts/mod/modpost.c:ignore_undef_symbol
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 4bf30e4b3eaa..278f5129bde2 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2348,6 +2348,20 @@ static int verify_exported_symbols(struct module *mod)
> return 0;
> }
>
> +static int ignore_undef_symbol(Elf_Half emachine, const char *name)
> +{
> + /* On x86, PIC code and Clang non-PIC code may have call foo@PLT. GNU
> as
> +* before 2.37 produces an unreferenced _GLOBAL_OFFSET_TABLE_ on
> x86-64.
> +* i386 has a similar problem but may not deserve a fix.
> +*
> +* If we ever have to ignore many symbols, consider refactoring the
> code to
> +* only warn if referenced by a relocation.
> +*/
> + if (emachine == EM_386 || emachine == EM_X86_64)
> + return !strcmp(name, "_GLOBAL_OFFSET_TABLE_");
> + return 0;
> +}
> +
> /* Change all symbols so that st_value encodes the pointer directly. */
> static int simplify_symbols(struct module *mod, const struct load_info *info)
> {
> @@ -2395,8 +2409,10 @@ static int simplify_symbols(struct module *mod, const
> struct load_info *info)
> break;
> }
>
> - /* Ok if weak. */
> - if (!ksym && ELF_ST_BIND(sym[i].st_info) == STB_WEAK)
> + /* Ok if weak or ignored. */
> + if (!ksym &&
> + (ELF_ST_BIND(sym[i].st_info) == STB_WEAK ||
> +ignore_undef_symbol(info->hdr->e_machine, name)))
> break;
>
> ret = PTR_ERR(ksym) ?: -ENOENT;
> --
--
Thanks,
~Nick Desaulniers
