Skip to contentSkip to conversation

lib/crypto: gf128mul: Hide selection mask values from compilers

PATCH Applied Eric Biggers 2 participants 3 messages Oct 31 – Nov 3 Crypto API

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 lib/crypto: gf128mul: Hide selection mask values from compilers

Hide selection mask values from compilers, so that they cannot see that they are used to select between two values and potentially generate a secret-dependent branch. This is not known to be happening here, but we might as well be extra careful.

For the cases where the selected values are only 0x87 and 0, change the type of the mask to 'unsigned long' so that the compiler is not forced to materialize it in a register pair on 32-bit architectures.

---
 include/crypto/gf128mul.h | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)
include/crypto/gf128mul.h +9 -16
index 6ed2a8351902..8fb6f8dbbfa9 100644
@@ -171,35 +171,28 @@ void gf128mul_lle(be128 *a, const be128 *b);
  * They are defined here for performance.
  */
 
-static inline u64 gf128mul_mask_from_bit(u64 x, int which)
-{
-	/* a constant-time version of 'x & ((u64)1 << which) ? (u64)-1 : 0' */
-	return ((s64)(x << (63 - which)) >> 63);
-}
-
 static inline void gf128mul_x_lle(be128 *r, const be128 *x)
 {
 	u64 a = be64_to_cpu(x->a);
 	u64 b = be64_to_cpu(x->b);
+	u64 mask = -(b & 1);
 
-	/* equivalent to gf128mul_table_le[(b << 7) & 0xff] << 48
-	 * (see crypto/gf128mul.c): */
-	u64 _tt = gf128mul_mask_from_bit(b, 0) & ((u64)0xe1 << 56);
+	OPTIMIZER_HIDE_VAR(mask);
 
 	r->b = cpu_to_be64((b >> 1) | (a << 63));
-	r->a = cpu_to_be64((a >> 1) ^ _tt);
+	r->a = cpu_to_be64((a >> 1) ^ (mask & ((u64)0xe1 << 56)));
 }
 
 static inline void gf128mul_x_bbe(be128 *r, const be128 *x)
 {
 	u64 a = be64_to_cpu(x->a);
 	u64 b = be64_to_cpu(x->b);
+	unsigned long mask = (s64)a >> 63;
 
-	/* equivalent to gf128mul_table_be[a >> 63] (see crypto/gf128mul.c): */
-	u64 _tt = gf128mul_mask_from_bit(a, 63) & 0x87;
+	OPTIMIZER_HIDE_VAR(mask);
 
 	r->a = cpu_to_be64((a << 1) | (b >> 63));
-	r->b = cpu_to_be64((b << 1) ^ _tt);
+	r->b = cpu_to_be64((b << 1) ^ (mask & 0x87));
 }
 
 /* needed by XTS */
@@ -207,12 +200,12 @@ static inline void gf128mul_x_ble(le128 *r, const le128 *x)
 {
 	u64 a = le64_to_cpu(x->a);
 	u64 b = le64_to_cpu(x->b);
+	unsigned long mask = (s64)a >> 63;
 
-	/* equivalent to gf128mul_table_be[b >> 63] (see crypto/gf128mul.c): */
-	u64 _tt = gf128mul_mask_from_bit(a, 63) & 0x87;
+	OPTIMIZER_HIDE_VAR(mask);
 
 	r->a = cpu_to_le64((a << 1) | (b >> 63));
-	r->b = cpu_to_le64((b << 1) ^ _tt);
+	r->b = cpu_to_le64((b << 1) ^ (mask & 0x87));
 }
 
 void gf128mul_x8_ble(le128 *r, const le128 *x);

base-commit: 2a500a04d6f969318c05f46aedb6e73fc1963a00

Signature · 1 line
2.56.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 5 lines · Eric Biggers

Hide selection mask values from compilers, so that they cannot see that they are used to select between two values and potentially generate a secret-dependent branch. This is not known to be happening here, but we might as well be extra careful.

Quoted 9 lines

For the cases where the selected values are only 0x87 and 0, change the type of the mask to 'unsigned long' so that the compiler is not forced to materialize it in a register pair on 32-bit architectures.

---
 include/crypto/gf128mul.h | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)
Quoted 67 lines
include/crypto/gf128mul.h +9 -16
index 6ed2a8351902..8fb6f8dbbfa9 100644
@@ -171,35 +171,28 @@ void gf128mul_lle(be128 *a, const be128 *b);
  * They are defined here for performance.
  */
 
-static inline u64 gf128mul_mask_from_bit(u64 x, int which)
-{
-	/* a constant-time version of 'x & ((u64)1 << which) ? (u64)-1 : 0' */
-	return ((s64)(x << (63 - which)) >> 63);
-}
-
 static inline void gf128mul_x_lle(be128 *r, const be128 *x)
 {
 	u64 a = be64_to_cpu(x->a);
 	u64 b = be64_to_cpu(x->b);
+	u64 mask = -(b & 1);
 
-	/* equivalent to gf128mul_table_le[(b << 7) & 0xff] << 48
-	 * (see crypto/gf128mul.c): */
-	u64 _tt = gf128mul_mask_from_bit(b, 0) & ((u64)0xe1 << 56);
+	OPTIMIZER_HIDE_VAR(mask);
 
 	r->b = cpu_to_be64((b >> 1) | (a << 63));
-	r->a = cpu_to_be64((a >> 1) ^ _tt);
+	r->a = cpu_to_be64((a >> 1) ^ (mask & ((u64)0xe1 << 56)));
 }
 
 static inline void gf128mul_x_bbe(be128 *r, const be128 *x)
 {
 	u64 a = be64_to_cpu(x->a);
 	u64 b = be64_to_cpu(x->b);
+	unsigned long mask = (s64)a >> 63;
 
-	/* equivalent to gf128mul_table_be[a >> 63] (see crypto/gf128mul.c): */
-	u64 _tt = gf128mul_mask_from_bit(a, 63) & 0x87;
+	OPTIMIZER_HIDE_VAR(mask);
 
 	r->a = cpu_to_be64((a << 1) | (b >> 63));
-	r->b = cpu_to_be64((b << 1) ^ _tt);
+	r->b = cpu_to_be64((b << 1) ^ (mask & 0x87));
 }
 
 /* needed by XTS */
@@ -207,12 +200,12 @@ static inline void gf128mul_x_ble(le128 *r, const 
 le128 *x)
 {
 	u64 a = le64_to_cpu(x->a);
 	u64 b = le64_to_cpu(x->b);
+	unsigned long mask = (s64)a >> 63;
 
-	/* equivalent to gf128mul_table_be[b >> 63] (see crypto/gf128mul.c): */
-	u64 _tt = gf128mul_mask_from_bit(a, 63) & 0x87;
+	OPTIMIZER_HIDE_VAR(mask);
 
 	r->a = cpu_to_le64((a << 1) | (b >> 63));
-	r->b = cpu_to_le64((b << 1) ^ _tt);
+	r->b = cpu_to_le64((b << 1) ^ (mask & 0x87));
 }
 

void gf128mul_x8_ble(le128 *r, const le128 *x);

base-commit: 2a500a04d6f969318c05f46aedb6e73fc1963a00

Signature · 1 line
2.56.0
author Applied
#2 lore raw
In reply to Eric Biggers
Quoted 5 lines · Eric Biggers

Hide selection mask values from compilers, so that they cannot see that they are used to select between two values and potentially generate a secret-dependent branch. This is not known to be happening here, but we might as well be extra careful.

Quoted 8 lines

For the cases where the selected values are only 0x87 and 0, change the type of the mask to 'unsigned long' so that the compiler is not forced to materialize it in a register pair on 32-bit architectures.

---
 include/crypto/gf128mul.h | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=libcrypto-next

- Eric