linux-kernel - Re: [PATCH] x86: Reduce duplicated code in the x86

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:	Mon, 13 May 2013 09:31:08 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Joe Damato <ice799@...il.com>
Cc:	x86@...nel.org, linux-kernel@...r.kernel.org, mingo@...hat.com,
	hpa@...or.com, akpm@...ux-foundation.org
Subject: Re: [PATCH] x86: Reduce duplicated code in the x86_64 context switch
 path.

On Sun, May 12, 2013 at 10:14:51PM -0700, Joe Damato wrote:
> Signed-off-by: Joe Damato <ice799@...il.com>
> ---
>  arch/x86/include/asm/switch_to.h |   30 ++++++++++++++++++++++++++++++
>  arch/x86/kernel/process_64.c     |   29 ++---------------------------
>  2 files changed, 32 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/x86/include/asm/switch_to.h b/arch/x86/include/asm/switch_to.h
> index 4ec45b3..a322cc6 100644
> --- a/arch/x86/include/asm/switch_to.h
> +++ b/arch/x86/include/asm/switch_to.h
> @@ -124,6 +124,36 @@ do {									\
>  	       __switch_canary_iparam					  \
>  	     : "memory", "cc" __EXTRA_CLOBBER)
>  
> +#define loadsegment_fs(fs, index)					  \
> +	loadsegment(fs, index)
> +
> +#define loadsegment_gs(gs, index)					  \
> +	load_gs_index(index)
> +
> +#define switch_segment(prev, next, index, seg, msr)			       \
> +	do {								       \
> +		/*							       \
> +		 * Segment register != 0 always requires a reload.  Also       \
> +		 * reload when it has changed.  When prev process used 64bit   \
> +		 * base always reload to avoid an information leak.	       \
> +		 */							       \
> +		if (unlikely(index | next->index | prev->seg)) {	       \
> +			loadsegment_##seg(seg, next->index);		       \
> +			/*						       \
> +			 * Check if the user used a selector != 0; if yes      \
> +			 *  clear 64bit base, since overloaded base is always  \
> +			 *  mapped to the Null selector			       \
> +			 */						       \
> +			if (index)					       \
> +			  prev->seg = 0;				       \
> +		}							       \
> +									       \
> +		/* when next process has a 64bit base use it */		       \
> +		if (next->seg)						       \
> +		  wrmsrl(msr, next->seg);				       \
> +		prev->index = index;					       \
> +	} while (0)
> +
>  #endif /* CONFIG_X86_32 */
>  
For my part I'll never understand how code written as macros is supposed to
improve anything. I always find it confusing and risky, as it is very easy
to introduce side effects. Also, while it may reduce the source code size,
it often results in increased object size.

My take: If you can not write it as inline function(s), don't bother.

Guenter
--
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/