[tip:x86/urgent] x86/alternatives: Fix alt_max_short macro to really be a max()

2017-10-09 Thread tip-bot for Mathias Krause
Commit-ID:  6b32c126d33d5cb379bca280ab8acedc1ca978ff
Gitweb: https://git.kernel.org/tip/6b32c126d33d5cb379bca280ab8acedc1ca978ff
Author: Mathias Krause 
AuthorDate: Thu, 5 Oct 2017 20:30:12 +0200
Committer:  Thomas Gleixner 
CommitDate: Mon, 9 Oct 2017 13:35:17 +0200

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 dbe4058a6a44 ("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#IntegerMinOrMax

Reviewed-and-tested-by: Borislav Petkov 
Fixes: dbe4058a6a44 ("x86/alternatives: Fix ALTERNATIVE_2 padding generation 
properly")
Signed-off-by: Mathias Krause 
Signed-off-by: Thomas Gleixner 
Cc: Borislav Petkov 
Cc: sta...@vger.kernel.org
Link: 
https://lkml.kernel.org/r/1507228213-13095-1-git-send-email-mini...@googlemail.com

---
 arch/x86/include/asm/alternative-asm.h | 4 +++-
 arch/x86/include/asm/alternative.h | 6 +++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/alternative-asm.h 
b/arch/x86/include/asm/alternative-asm.h
index e7636ba..6c98821 100644
--- a/arch/x86/include/asm/alternative-asm.h
+++ b/arch/x86/include/asm/alternative-asm.h
@@ -62,8 +62,10 @@
 #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
+ *
+ * The additional "-" is needed because gas uses a "true" value of -1.
  */
 #define alt_max_short(a, b)((a) ^ (((a) ^ (b)) & -(-((a) < (b)
 
diff --git a/arch/x86/include/asm/alternative.h 
b/arch/x86/include/asm/alternative.h
index c096624..ccbe24e 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -103,12 +103,12 @@ static inline int alternatives_text_reserved(void *start, 
void *end)
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
  *
- * 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


[tip:x86/urgent] x86/alternatives: Fix alt_max_short macro to really be a max()

2017-10-09 Thread tip-bot for Mathias Krause
Commit-ID:  6b32c126d33d5cb379bca280ab8acedc1ca978ff
Gitweb: https://git.kernel.org/tip/6b32c126d33d5cb379bca280ab8acedc1ca978ff
Author: Mathias Krause 
AuthorDate: Thu, 5 Oct 2017 20:30:12 +0200
Committer:  Thomas Gleixner 
CommitDate: Mon, 9 Oct 2017 13:35:17 +0200

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 dbe4058a6a44 ("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#IntegerMinOrMax

Reviewed-and-tested-by: Borislav Petkov 
Fixes: dbe4058a6a44 ("x86/alternatives: Fix ALTERNATIVE_2 padding generation 
properly")
Signed-off-by: Mathias Krause 
Signed-off-by: Thomas Gleixner 
Cc: Borislav Petkov 
Cc: sta...@vger.kernel.org
Link: 
https://lkml.kernel.org/r/1507228213-13095-1-git-send-email-mini...@googlemail.com

---
 arch/x86/include/asm/alternative-asm.h | 4 +++-
 arch/x86/include/asm/alternative.h | 6 +++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/alternative-asm.h 
b/arch/x86/include/asm/alternative-asm.h
index e7636ba..6c98821 100644
--- a/arch/x86/include/asm/alternative-asm.h
+++ b/arch/x86/include/asm/alternative-asm.h
@@ -62,8 +62,10 @@
 #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
+ *
+ * The additional "-" is needed because gas uses a "true" value of -1.
  */
 #define alt_max_short(a, b)((a) ^ (((a) ^ (b)) & -(-((a) < (b)
 
diff --git a/arch/x86/include/asm/alternative.h 
b/arch/x86/include/asm/alternative.h
index c096624..ccbe24e 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -103,12 +103,12 @@ static inline int alternatives_text_reserved(void *start, 
void *end)
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
  *
- * 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