[sw,cryptolib] Fix trigger_fault_if_fg0 functions

The trigger_fault_if_fg0 functions had a bug where we never
trigger a fault. This new implementation should now correctly
trigger the fault.

For the case where no fault is triggered, we load address 0 into
w31. For the error case we try to load address 0 into
w39 (which doesn't exist), which triggers a ILLEGAL_INSN error.

Signed-off-by: Hakim Filali <[email protected]>

Author: h-filali

sw/otbn/crypto/p256_base.s

@@ -43,33 +43,27 @@
  * sensitive; since aborting the program will be quicker than completing it,
  * the flag's value is likely clearly visible to an attacker through timing.
  *
- * @param[in]    w31: all-zero
- * @param[in]  FG0.Z: boolean indicating fault condition
+ * @param[in]  FG0.Z: boolean indicating fault condition when 1
  *
- * clobbered registers: x2
+ * clobbered registers: x2, w31
  * clobbered flag groups: none
  */
 trigger_fault_if_fg0_z:
   /* Read the FG0.Z flag (position 3).
        x2 <= FG0.Z */
   csrrw     x2, FG0, x0
   andi      x2, x2, 8
-  srli      x2, x2, 3
+  addi      x2, x2, 31
 
-  /* Subtract FG0.Z from 0.
-       x2 <= 0 - x2 = FG0.Z ? 2^32 - 1 : 0 */
-  sub       x2, x0, x2
-
-  /* The `bn.lid` instruction causes an `BAD_DATA_ADDR` error if the
-     memory address is out of bounds. Therefore, if FG0.Z is 1, this
-     instruction causes an error, but if FG0.Z is 0 it simply loads the word at
-     address 0 into w31. */
-  li         x3, 31
-  bn.lid     x3, 0(x2)
+  /* The `bn.lid` instruction causes an `ILLEGAL_INSN` error if the index of the
+     bignum register (stored in x2 in this case) is invalid. Therefore, if FG0.Z
+     is 1, this instruction causes an error, but if FG0.Z is 0 it simply loads
+     the word at address 0 into w31. */
+  bn.lid    x2, 0(x0)
 
   /* If we get here, the flag must have been 0. Restore w31 to zero and return.
        w31 <= 0 */
-  bn.xor     w31, w31, w31
+  bn.xor    w31, w31, w31
 
   ret
 
@@ -84,29 +78,28 @@ trigger_fault_if_fg0_z:
  * sensitive; since aborting the program will be quicker than completing it,
  * the flag's value is likely clearly visible to an attacker through timing.
  *
- * @param[in]    w31: all-zero
- * @param[in]  FG0.Z: boolean indicating fault condition
+ * @param[in]  FG0.Z: boolean indicating fault condition when 0
  *
- * clobbered registers: x2
+ * clobbered registers: x2, w31
  * clobbered flag groups: none
  */
 trigger_fault_if_fg0_not_z:
   /* Read the FG0.Z flag (position 3).
        x2 <= FG0.Z */
   csrrw     x2, FG0, x0
   andi      x2, x2, 8
-  slli      x2, x2, 3
+  xori      x2, x2, 8
+  addi      x2, x2, 31
 
-  /* The `bn.lid` instruction causes an `BAD_DATA_ADDR` error if the
-     memory address is out of bounds. Therefore, if FG0.Z is 1, this
-     instruction causes an error, but if FG0.Z is 0 it simply loads the word at
-     address 0 into w31. */
-  li         x3, 31
-  bn.lid     x3, 0(x2)
+  /* The `bn.lid` instruction causes an `ILLEGAL_INSN` error if the index of the
+     bignum register (stored in x2 in this case) is invalid. Therefore, if FG0.Z
+     is 0, this instruction causes an error, but if FG0.Z is 1 it simply loads
+     the word at address 0 into w31. */
+  bn.lid    x2, 0(x0)
 
   /* If we get here, the flag must have been 1. Restore w31 to zero and return.
        w31 <= 0 */
-  bn.xor     w31, w31, w31
+  bn.xor    w31, w31, w31
 
   ret
 

sw/otbn/crypto/p384_isoncurve.s

@@ -27,10 +27,9 @@
  * sensitive; since aborting the program will be quicker than completing it,
  * the flag's value is likely clearly visible to an attacker through timing.
  *
- * @param[in]    w31: all-zero
- * @param[in]  FG0.Z: boolean indicating fault condition
+ * @param[in]  FG0.Z: boolean indicating fault condition when 0
  *
- * clobbered registers: x2, x3
+ * clobbered registers: x2, w31
  * clobbered flag groups: none
  */
 .globl trigger_fault_if_fg0_not_z
@@ -39,18 +38,18 @@ trigger_fault_if_fg0_not_z:
        x2 <= FG0.Z */
   csrrw     x2, FG0, x0
   andi      x2, x2, 8
-  slli      x2, x2, 3
+  xori      x2, x2, 8
+  addi      x2, x2, 31
 
-  /* The `bn.lid` instruction causes an `BAD_DATA_ADDR` error if the
-     memory address is out of bounds. Therefore, if FG0.Z is 1, this
-     instruction causes an error, but if FG0.Z is 0 it simply loads the word at
-     address 0 into w31. */
-  li         x3, 31
-  bn.lid     x3, 0(x2)
+  /* The `bn.lid` instruction causes an `ILLEGAL_INSN` error if the index of the
+     bignum register (stored in x2 in this case) is invalid. Therefore, if FG0.Z
+     is 1, this instruction causes an error, but if FG0.Z is 0 it simply loads
+     the word at address 0 into w31. */
+  bn.lid    x2, 0(x0)
 
   /* If we get here, the flag must have been 1. Restore w31 to zero and return.
        w31 <= 0 */
-  bn.xor     w31, w31, w31
+  bn.xor    w31, w31, w31
 
   ret