linux-kernel - Re: [PATCH v2] arch/powerpc: Rework local

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]

Date:   Tue, 26 Mar 2019 15:58:43 +1000
From:   Nicholas Piggin <npiggin@...il.com>
To:     Alastair D'Silva <alastair@....ibm.com>, alastair@...ilva.org
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Christophe Leroy <christophe.leroy@....fr>,
        linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        Mahesh Salgaonkar <mahesh@...ux.vnet.ibm.com>,
        Michal Hocko <mhocko@...e.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>,
        Paul Mackerras <paulus@...ba.org>,
        Mike Rapoport <rppt@...ux.vnet.ibm.com>
Subject: Re: [PATCH v2]  arch/powerpc: Rework local_paca to avoid LTO warnings

Alastair D'Silva's on March 14, 2019 12:31 pm:
> From: Alastair D'Silva <alastair@...ilva.org>
> 
> When building an LTO kernel, the existing code generates warnings:
>     ./arch/powerpc/include/asm/paca.h:37:30: warning: register of
>         ‘local_paca’ used for multiple global register variables
>      register struct paca_struct *local_paca asm("r13");
>                                   ^
>     ./arch/powerpc/include/asm/paca.h:37:30: note: conflicts with
>         ‘local_paca’

Isn't this a bogus warning? It doesn't look like there's a way to 
define it any other way.

> 
> This patch reworks local_paca into an inline getter & setter function,
> which addresses the warning.
> 
> Changelog:
> V2
>   - Address whitespace issues
>   - keep new implementation close to where the old implementation was
> 
> Signed-off-by: Alastair D'Silva <alastair@...ilva.org>
> ---
>  arch/powerpc/include/asm/paca.h | 37 +++++++++++++++++++++++++--------
>  arch/powerpc/kernel/paca.c      |  2 +-
>  2 files changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
> index e843bc5d1a0f..2fa0b43357c9 100644
> --- a/arch/powerpc/include/asm/paca.h
> +++ b/arch/powerpc/include/asm/paca.h
> @@ -34,19 +34,38 @@
>  #include <asm/cpuidle.h>
>  #include <asm/atomic.h>
>  
> -register struct paca_struct *local_paca asm("r13");
> -
>  #if defined(CONFIG_DEBUG_PREEMPT) && defined(CONFIG_SMP)
>  extern unsigned int debug_smp_processor_id(void); /* from linux/smp.h */
> -/*
> - * Add standard checks that preemption cannot occur when using get_paca():
> - * otherwise the paca_struct it points to may be the wrong one just after.
> - */
> -#define get_paca()	((void) debug_smp_processor_id(), local_paca)
> -#else
> -#define get_paca()	local_paca
>  #endif
>  
> +static inline struct paca_struct *get_paca_no_preempt_check(void)
> +{
> +	register struct paca_struct *paca asm("r13");
> +
> +	return paca;
> +}

Problem is it now changes the global register variable to a local 
register variable. The compiler would presumably be within its rights
to "cache" that return value or use another register for it, which
is not really what we want.

Thanks,
Nick