[PATCH v3] module: Ignore _GLOBAL_OFFSET_TABLE_ when warning for undefined symbols

[PATCH v3] module: Ignore _GLOBAL_OFFSET_TABLE_ when warning for undefined symbols

[Fangrui Song]

clang-12 -fno-pic (since
https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6)
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 <elver@google.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/1250
Signed-off-by: Fangrui Song <maskray@google.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Marco Elver <elver@google.com>
Cc: <stable@vger.kernel.org>

---
Changes in v2:
* Fix Marco's email address
* Add a function ignore_undef_symbol similar to scripts/mod/modpost.c:ignore_undef_symbol
---
Changes in v3:
* Fix the style of a multi-line comment.
* Use static bool ignore_undef_symbol.
---
 kernel/module.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 4bf30e4b3eaa..805c49d1b86d 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2348,6 +2348,21 @@ static int verify_exported_symbols(struct module *mod)
 	return 0;
 }
 
+static bool 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 false;
+}
+
 /* 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 +2410,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 v3] module: Ignore _GLOBAL_OFFSET_TABLE_ when warning for undefined symbols

[Nathan Chancellor]

On Fri, Jan 15, 2021 at 11:52:22AM -0800, 'Fangrui Song' via Clang Built Linux wrote:
> clang-12 -fno-pic (since
> https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6)
> 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 <elver@google.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1250
> Signed-off-by: Fangrui Song <maskray@google.com>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Tested-by: Marco Elver <elver@google.com>
> Cc: <stable@vger.kernel.org>

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>

> 
> ---
> Changes in v2:
> * Fix Marco's email address
> * Add a function ignore_undef_symbol similar to scripts/mod/modpost.c:ignore_undef_symbol
> ---
> Changes in v3:
> * Fix the style of a multi-line comment.
> * Use static bool ignore_undef_symbol.
> ---
>  kernel/module.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 4bf30e4b3eaa..805c49d1b86d 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2348,6 +2348,21 @@ static int verify_exported_symbols(struct module *mod)
>  	return 0;
>  }
>  
> +static bool 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 false;
> +}
> +
>  /* 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 +2410,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
> 
> -- 
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210115195222.3453262-1-maskray%40google.com.

Re: [PATCH v3] module: Ignore _GLOBAL_OFFSET_TABLE_ when warning for undefined symbols

[Jessica Yu]

+++ Fangrui Song [15/01/21 11:52 -0800]:
>clang-12 -fno-pic (since
>https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6)
>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 <elver@google.com>
>Link: https://github.com/ClangBuiltLinux/linux/issues/1250
>Signed-off-by: Fangrui Song <maskray@google.com>
>Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>Tested-by: Marco Elver <elver@google.com>
>Cc: <stable@vger.kernel.org>
>
>---
>Changes in v2:
>* Fix Marco's email address
>* Add a function ignore_undef_symbol similar to scripts/mod/modpost.c:ignore_undef_symbol
>---
>Changes in v3:
>* Fix the style of a multi-line comment.
>* Use static bool ignore_undef_symbol.

Patch has been queued up on modules-next:

https://git.kernel.org/pub/scm/linux/kernel/git/jeyu/linux.git/commit/?h=modules-next&id=ebfac7b778fac8b0e8e92ec91d0b055f046b4604

Thanks!

Jessica