linux-kernel - Re: [PATCH 52/74] x86, lto, paravirt: Don't rely on local assembler labels

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: <5030A31B.3060908@goop.org>
Date:	Sun, 19 Aug 2012 01:26:03 -0700
From:	Jeremy Fitzhardinge <jeremy@...p.org>
To:	Andi Kleen <andi@...stfloor.org>
CC:	linux-kernel@...r.kernel.org, x86@...nel.org, mmarek@...e.cz,
	linux-kbuild@...r.kernel.org, JBeulich@...e.com,
	akpm@...ux-foundation.org, Andi Kleen <ak@...ux.intel.com>
Subject: Re: [PATCH 52/74] x86, lto, paravirt: Don't rely on local assembler
 labels

On 08/18/2012 07:56 PM, Andi Kleen wrote:
> From: Andi Kleen <ak@...ux.intel.com>
>
> The paravirt patching code assumes that it can reference a
> local assembler label between two different top level assembler
> statements. This does not work with some experimental gcc builds,
> where the assembler code may end up in different assembler files.

Egad, what are those zany gcc chaps up to now?

    J

>
> Replace it with extern / global /asm linkage labels.
>
> This also removes one redundant copy of the macro.
>
> Cc: jeremy@...p.org
> Signed-off-by: Andi Kleen <ak@...ux.intel.com>
> ---
>  arch/x86/include/asm/paravirt_types.h |    9 +++++----
>  arch/x86/kernel/paravirt.c            |    5 -----
>  2 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
> index 4f262bc..6a464ba 100644
> --- a/arch/x86/include/asm/paravirt_types.h
> +++ b/arch/x86/include/asm/paravirt_types.h
> @@ -385,10 +385,11 @@ extern struct pv_lock_ops pv_lock_ops;
>  	_paravirt_alt(insn_string, "%c[paravirt_typenum]", "%c[paravirt_clobber]")
>  
>  /* Simple instruction patching code. */
> -#define DEF_NATIVE(ops, name, code) 					\
> -	extern const char start_##ops##_##name[] __visible, 		\
> -			  end_##ops##_##name[] __visible;		\
> -	asm("start_" #ops "_" #name ": " code "; end_" #ops "_" #name ":")
> +#define NATIVE_LABEL(a,x,b) "\n\t.globl " a #x "_" #b "\n" a #x "_" #b ":\n\t"
> +
> +#define DEF_NATIVE(ops, name, code)					\
> +	__visible extern const char start_##ops##_##name[], end_##ops##_##name[];	\
> +	asm(NATIVE_LABEL("start_", ops, name) code NATIVE_LABEL("end_", ops, name))
>  
>  unsigned paravirt_patch_nop(void);
>  unsigned paravirt_patch_ident_32(void *insnbuf, unsigned len);
> diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
> index 17fff18..947255e 100644
> --- a/arch/x86/kernel/paravirt.c
> +++ b/arch/x86/kernel/paravirt.c
> @@ -62,11 +62,6 @@ void __init default_banner(void)
>  	       pv_info.name);
>  }
>  
> -/* Simple instruction patching code. */
> -#define DEF_NATIVE(ops, name, code)					\
> -	extern const char start_##ops##_##name[], end_##ops##_##name[];	\
> -	asm("start_" #ops "_" #name ": " code "; end_" #ops "_" #name ":")
> -
>  /* Undefined instruction for dealing with missing ops pointers. */
>  static const unsigned char ud2a[] = { 0x0f, 0x0b };
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/