Commit 6b32c126 authored by Mathias Krause's avatar Mathias Krause Committed by Thomas Gleixner

x86/alternatives: Fix alt_max_short macro to really be a max()

The alt_max_short() macro in asm/alternative.h does not work as
intended, leading to nasty bugs. E.g. alt_max_short("1", "3")
evaluates to 3, but alt_max_short("3", "1") evaluates to 1 -- not
exactly the maximum of 1 and 3.

In fact, I had to learn it the hard way by crashing my kernel in not
so funny ways by attempting to make use of the ALTENATIVE_2 macro
with alternatives where the first one was larger than the second
one.

According to [1] and commit dbe4058a ("x86/alternatives: Fix
ALTERNATIVE_2 padding generation properly") the right handed side
should read "-(-(a < b))" not "-(-(a - b))". Fix that, to make the
macro work as intended.

While at it, fix up the comments regarding the additional "-", too.
It's not about gas' usage of s32 but brain dead logic of having a
"true" value of -1 for the < operator ... *sigh*

Btw., the one in asm/alternative-asm.h is correct. And, apparently,
all current users of ALTERNATIVE_2() pass same sized alternatives,
avoiding to hit the bug.

[1] http://graphics.stanford.edu/~seander/bithacks.html#IntegerMinOrMaxReviewed-and-tested-by: default avatarBorislav Petkov <bp@suse.de>
Fixes: dbe4058a ("x86/alternatives: Fix ALTERNATIVE_2 padding generation properly")
Signed-off-by: default avatarMathias Krause <minipli@googlemail.com>
Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@suse.de>
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/1507228213-13095-1-git-send-email-minipli@googlemail.com
parent 924c6b90
...@@ -62,8 +62,10 @@ ...@@ -62,8 +62,10 @@
#define new_len2 145f-144f #define new_len2 145f-144f
/* /*
* max without conditionals. Idea adapted from: * gas compatible max based on the idea from:
* http://graphics.stanford.edu/~seander/bithacks.html#IntegerMinOrMax * http://graphics.stanford.edu/~seander/bithacks.html#IntegerMinOrMax
*
* The additional "-" is needed because gas uses a "true" value of -1.
*/ */
#define alt_max_short(a, b) ((a) ^ (((a) ^ (b)) & -(-((a) < (b))))) #define alt_max_short(a, b) ((a) ^ (((a) ^ (b)) & -(-((a) < (b)))))
......
...@@ -103,12 +103,12 @@ static inline int alternatives_text_reserved(void *start, void *end) ...@@ -103,12 +103,12 @@ static inline int alternatives_text_reserved(void *start, void *end)
alt_end_marker ":\n" alt_end_marker ":\n"
/* /*
* max without conditionals. Idea adapted from: * gas compatible max based on the idea from:
* http://graphics.stanford.edu/~seander/bithacks.html#IntegerMinOrMax * http://graphics.stanford.edu/~seander/bithacks.html#IntegerMinOrMax
* *
* The additional "-" is needed because gas works with s32s. * The additional "-" is needed because gas uses a "true" value of -1.
*/ */
#define alt_max_short(a, b) "((" a ") ^ (((" a ") ^ (" b ")) & -(-((" a ") - (" b ")))))" #define alt_max_short(a, b) "((" a ") ^ (((" a ") ^ (" b ")) & -(-((" a ") < (" b ")))))"
/* /*
* Pad the second replacement alternative with additional NOPs if it is * Pad the second replacement alternative with additional NOPs if it is
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment