linux-kernel - Re: [PATCH v3] module: Ignore _GLOBAL_OFFSET_TABLE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210115195530.GA3843886@ubuntu-m3-large-x86>
Date:   Fri, 15 Jan 2021 12:55:30 -0700
From:   Nathan Chancellor <natechancellor@...il.com>
To:     Fangrui Song <maskray@...gle.com>
Cc:     linux-kernel@...r.kernel.org, Jessica Yu <jeyu@...nel.org>,
        clang-built-linux@...glegroups.com,
        Sam Ravnborg <sam@...nborg.org>,
        Marco Elver <elver@...gle.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        stable@...r.kernel.org
Subject: Re: [PATCH v3] module: Ignore _GLOBAL_OFFSET_TABLE_ when warning for
 undefined symbols

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@...` 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@...` 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@...gle.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1250
> Signed-off-by: Fangrui Song <maskray@...gle.com>
> Reviewed-by: Nick Desaulniers <ndesaulniers@...gle.com>
> Tested-by: Marco Elver <elver@...gle.com>
> Cc: <stable@...r.kernel.org>

Reviewed-by: Nathan Chancellor <natechancellor@...il.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@.... 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@...glegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210115195222.3453262-1-maskray%40google.com.