RV64: Add bounds assertions

Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>

Author: hanno-becker

.github/workflows/ci.yml

@@ -150,7 +150,7 @@ jobs:
           compile_mode: native
           cflags: "-DMLKEM_DEBUG -fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all"
           check_namespace: 'false'
-      - name: build + test (+debug+memsan+ubsan, cross, opt)
+      - name: build + test (+debug, cross, opt)
         uses: ./.github/actions/multi-functest
         # There is no native code yet on PPC64LE, riscv32 or AArch64_be, so no point running opt tests
         if: ${{ matrix.target.mode != 'native' && (matrix.target.arch != 'ppc64le' && matrix.target.arch != 'riscv32' && matrix.target.arch != 'aarch64_be') }}
@@ -159,7 +159,7 @@ jobs:
           nix-cache: ${{ matrix.target.mode == 'native' && 'false' || 'true' }}
           gh_token: ${{ secrets.GITHUB_TOKEN }}
           compile_mode: ${{ matrix.target.mode }}
-          cflags: "-DMLKEM_DEBUG -fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all"
+          cflags: "-DMLKEM_DEBUG"
           opt: 'opt'
   backend_tests:
     name: AArch64 FIPS202 backends (${{ matrix.backend }})

mlkem/mlkem_native.S

@@ -596,6 +596,12 @@
 #undef MLK_RVV_MONT_R2
 #undef MLK_RVV_QI
 #undef MLK_RVV_VLEN
+#undef mlk_assert_abs_bound_int16m1
+#undef mlk_assert_abs_bound_int16m2
+#undef mlk_assert_bound_int16m1
+#undef mlk_assert_bound_int16m2
+#undef mlk_debug_check_bounds_int16m1
+#undef mlk_debug_check_bounds_int16m2
 #endif /* MLK_SYS_RISCV64 */
 #endif /* MLK_CONFIG_USE_NATIVE_BACKEND_ARITH */
 #endif /* !MLK_CONFIG_MONOBUILD_KEEP_SHARED_HEADERS */

mlkem/mlkem_native.c

@@ -85,6 +85,7 @@
 #include "src/native/x86_64/src/rej_uniform_table.c"
 #endif
 #if defined(MLK_SYS_RISCV64)
+#include "src/native/riscv64/src/rv64v_debug.c"
 #include "src/native/riscv64/src/rv64v_poly.c"
 #endif
 #endif /* MLK_CONFIG_USE_NATIVE_BACKEND_ARITH */
@@ -584,6 +585,12 @@
 #undef MLK_RVV_MONT_R2
 #undef MLK_RVV_QI
 #undef MLK_RVV_VLEN
+#undef mlk_assert_abs_bound_int16m1
+#undef mlk_assert_abs_bound_int16m2
+#undef mlk_assert_bound_int16m1
+#undef mlk_assert_bound_int16m2
+#undef mlk_debug_check_bounds_int16m1
+#undef mlk_debug_check_bounds_int16m2
 #endif /* MLK_SYS_RISCV64 */
 #endif /* MLK_CONFIG_USE_NATIVE_BACKEND_ARITH */
 #endif /* !MLK_CONFIG_MONOBUILD_KEEP_SHARED_HEADERS */

mlkem/src/native/riscv64/src/rv64v_debug.c

@@ -0,0 +1,81 @@
+/*
+ * Copyright (c) The mlkem-native project authors
+ * SPDX-License-Identifier: Apache-2.0 OR ISC OR MIT
+ */
+
+/* NOTE: You can remove this file unless you compile with MLKEM_DEBUG. */
+
+#include "../../../common.h"
+
+#if defined(MLK_ARITH_BACKEND_RISCV64) && \
+    !defined(MLK_CONFIG_MULTILEVEL_NO_SHARED) && defined(MLKEM_DEBUG)
+
+#include <riscv_vector.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include "../../../debug.h"
+#include "rv64v_settings.h"
+
+#define MLK_DEBUG_ERROR_HEADER "[ERROR:%s:%04d] "
+
+/*************************************************
+ * Name:        mlk_debug_check_bounds_int16m1
+ *
+ * Description: Check whether values in a vint16m1_t vector
+ *              are within specified bounds.
+ *
+ * Implementation: Extract vector elements to a temporary array
+ *                and reuse existing array bounds checking.
+ **************************************************/
+void mlk_debug_check_bounds_int16m1(const char *file, int line, vint16m1_t vec,
+                                    size_t vl, int lower_bound_exclusive,
+                                    int upper_bound_exclusive)
+{
+  /* Allocate temporary array to store vector elements
+   * We use the maximum possible vector length to be safe */
+  int16_t temp_array[MLK_RVV_E16M1_VL];
+
+  /* Store vector elements to temporary array for inspection */
+  __riscv_vse16_v_i16m1(temp_array, vec, vl);
+
+  /* Reuse existing array bounds checking function */
+  mlk_debug_check_bounds(file, line, temp_array, (unsigned)vl,
+                         lower_bound_exclusive, upper_bound_exclusive);
+}
+
+/*************************************************
+ * Name:        mlk_debug_check_bounds_int16m2
+ *
+ * Description: Check whether values in a vint16m2_t vector
+ *              are within specified bounds.
+ *
+ * Implementation: Extract vector elements to a temporary array
+ *                and reuse existing array bounds checking.
+ **************************************************/
+void mlk_debug_check_bounds_int16m2(const char *file, int line, vint16m2_t vec,
+                                    size_t vl, int lower_bound_exclusive,
+                                    int upper_bound_exclusive)
+{
+  /* Allocate temporary array to store vector elements
+   * m2 vectors hold 2x the elements of m1 vectors */
+  int16_t temp_array[2 * MLK_RVV_E16M1_VL];
+
+  /* Store vector elements to temporary array for inspection */
+  __riscv_vse16_v_i16m2(temp_array, vec, 2 * vl);
+
+  /* Reuse existing array bounds checking function for all elements */
+  mlk_debug_check_bounds(file, line, temp_array, (unsigned)(2 * vl),
+                         lower_bound_exclusive, upper_bound_exclusive);
+}
+
+#else /* MLK_ARITH_BACKEND_RISCV64 && !MLK_CONFIG_MULTILEVEL_NO_SHARED && \
+         MLKEM_DEBUG */
+
+MLK_EMPTY_CU(rv64v_debug)
+
+#endif /* !(MLK_ARITH_BACKEND_RISCV64 && !MLK_CONFIG_MULTILEVEL_NO_SHARED && \
+          MLKEM_DEBUG) */
+
+/* To facilitate single-compilation-unit (SCU) builds, undefine all macros.
+ * Don't modify by hand -- this is auto-generated by scripts/autogen. */
+#undef MLK_DEBUG_ERROR_HEADER

mlkem/src/native/riscv64/src/rv64v_poly.c

@@ -21,7 +21,7 @@ static inline vint16m1_t fq_redc(vint16m1_t rh, vint16m1_t rl, size_t vl)
 
   t = __riscv_vmul_vx_i16m1(rl, MLK_RVV_QI, vl); /* t = l * -Q^-1  */
   t = __riscv_vmulh_vx_i16m1(t, MLKEM_Q, vl);    /* t = (t*Q) / R  */
-  c = __riscv_vmsne_vx_i16m1_b16(rl, 0, vl);     /* c = l == 0     */
+  c = __riscv_vmsne_vx_i16m1_b16(rl, 0, vl);     /* c = (l != 0)   */
   t = __riscv_vadc_vvm_i16m1(t, rh, c, vl);      /* t += h + c     */
 
   return t;
@@ -55,6 +55,7 @@ static inline vint16m1_t fq_barrett(vint16m1_t a, size_t vl)
   t = __riscv_vmul_vx_i16m1(t, MLKEM_Q, vl);
   t = __riscv_vsub_vv_i16m1(a, t, vl);
 
+  mlk_assert_abs_bound_int16m1(t, vl, MLKEM_Q_HALF);
   return t;
 }
 
@@ -66,6 +67,7 @@ static inline vint16m1_t fq_cadd(vint16m1_t rx, size_t vl)
 
   bn = __riscv_vmslt_vx_i16m1_b16(rx, 0, vl);             /*   if x < 0:   */
   rx = __riscv_vadd_vx_i16m1_mu(bn, rx, rx, MLKEM_Q, vl); /*     x += Q    */
+
   return rx;
 }
 
@@ -75,8 +77,9 @@ static inline vint16m1_t fq_csub(vint16m1_t rx, size_t vl)
 {
   vbool16_t bn;
 
-  bn = __riscv_vmsge_vx_i16m1_b16(rx, MLKEM_Q, vl);       /*   if x >= 0:  */
+  bn = __riscv_vmsge_vx_i16m1_b16(rx, MLKEM_Q, vl);       /*   if x >= Q:  */
   rx = __riscv_vsub_vx_i16m1_mu(bn, rx, rx, MLKEM_Q, vl); /*     x -= Q    */
+
   return rx;
 }
 
@@ -106,7 +109,12 @@ static inline vint16m1_t fq_mul_vx(vint16m1_t rx, int16_t ry, size_t vl)
 
 static inline vint16m1_t fq_mulq_vx(vint16m1_t rx, int16_t ry, size_t vl)
 {
-  return fq_cadd(fq_mul_vx(rx, ry, vl), vl);
+  vint16m1_t result;
+
+  result = fq_cadd(fq_mul_vx(rx, ry, vl), vl);
+
+  mlk_assert_bound_int16m1(result, vl, 0, MLKEM_Q);
+  return result;
 }
 
 /*  create a permutation for swapping index bits a and b, a < b */
@@ -142,18 +150,30 @@ static vuint16m2_t bitswap_perm(unsigned a, unsigned b, size_t vl)
 
 /*  forward butterfly operation */
 
-#define MLK_RVV_BFLY_FX(u0, u1, ut, uc, vl) \
-  {                                         \
-    ut = fq_mul_vx(u1, uc, vl);             \
-    u1 = __riscv_vsub_vv_i16m1(u0, ut, vl); \
-    u0 = __riscv_vadd_vv_i16m1(u0, ut, vl); \
+#define MLK_RVV_BFLY_FX(u0, u1, ut, uc, vl, layer)                     \
+  {                                                                    \
+    mlk_assert_abs_bound(&uc, 1, MLKEM_Q_HALF);                        \
+                                                                       \
+    ut = fq_mul_vx(u1, uc, vl);                                        \
+    u1 = __riscv_vsub_vv_i16m1(u0, ut, vl);                            \
+    u0 = __riscv_vadd_vv_i16m1(u0, ut, vl);                            \
+                                                                       \
+    /* mlk_assert_abs_bound_int16m1(u0, vl, (layer + 1) * MLKEM_Q); */ \
+    /* mlk_assert_abs_bound_int16m1(u1, vl, (layer + 1) * MLKEM_Q); */ \
+    /* mlk_assert_abs_bound_int16m1(ut, vl, MLKEM_Q);		    */          \
   }
 
-#define MLK_RVV_BFLY_FV(u0, u1, ut, uc, vl) \
-  {                                         \
-    ut = fq_mul_vv(u1, uc, vl);             \
-    u1 = __riscv_vsub_vv_i16m1(u0, ut, vl); \
-    u0 = __riscv_vadd_vv_i16m1(u0, ut, vl); \
+#define MLK_RVV_BFLY_FV(u0, u1, ut, uc, vl, layer)                     \
+  {                                                                    \
+    mlk_assert_abs_bound_int16m1(uc, vl, MLKEM_Q_HALF);                \
+                                                                       \
+    ut = fq_mul_vv(u1, uc, vl);                                        \
+    u1 = __riscv_vsub_vv_i16m1(u0, ut, vl);                            \
+    u0 = __riscv_vadd_vv_i16m1(u0, ut, vl);                            \
+                                                                       \
+    /* mlk_assert_abs_bound_int16m1(ut, vl, MLKEM_Q);               */ \
+    /* mlk_assert_abs_bound_int16m1(u0, vl, (layer + 1) * MLKEM_Q); */ \
+    /* mlk_assert_abs_bound_int16m1(u1, vl, (layer + 1) * MLKEM_Q); */ \
   }
 
 static vint16m2_t mlk_rv64v_ntt2(vint16m2_t vp, vint16m1_t cz)
@@ -185,7 +205,7 @@ static vint16m2_t mlk_rv64v_ntt2(vint16m2_t vp, vint16m1_t cz)
   t1 = __riscv_vget_v_i16m2_i16m1(vp, 1);
 
   c0 = __riscv_vrgather_vv_i16m1(cz, cs8, vl);
-  MLK_RVV_BFLY_FV(t0, t1, vt, c0, vl);
+  MLK_RVV_BFLY_FV(t0, t1, vt, c0, vl, 5);
 
   /*    swap 4  */
   vp = __riscv_vcreate_v_i16m1_i16m2(t0, t1);
@@ -194,7 +214,7 @@ static vint16m2_t mlk_rv64v_ntt2(vint16m2_t vp, vint16m1_t cz)
   t1 = __riscv_vget_v_i16m2_i16m1(vp, 1);
 
   c0 = __riscv_vrgather_vv_i16m1(cz, cs4, vl);
-  MLK_RVV_BFLY_FV(t0, t1, vt, c0, vl);
+  MLK_RVV_BFLY_FV(t0, t1, vt, c0, vl, 6);
 
   /*    swap 2  */
   vp = __riscv_vcreate_v_i16m1_i16m2(t0, t1);
@@ -203,7 +223,7 @@ static vint16m2_t mlk_rv64v_ntt2(vint16m2_t vp, vint16m1_t cz)
   t1 = __riscv_vget_v_i16m2_i16m1(vp, 1);
 
   c0 = __riscv_vrgather_vv_i16m1(cz, cs2, vl);
-  MLK_RVV_BFLY_FV(t0, t1, vt, c0, vl);
+  MLK_RVV_BFLY_FV(t0, t1, vt, c0, vl, 7);
 
   /*    normalize   */
   t0 = fq_mulq_vx(t0, MLK_RVV_MONT_R1, vl);
@@ -253,41 +273,41 @@ void mlk_rv64v_poly_ntt(int16_t *r)
   ve = __riscv_vle16_v_i16m1(&r[0xe0], vl);
   vf = __riscv_vle16_v_i16m1(&r[0xf0], vl);
 
-  MLK_RVV_BFLY_FX(v0, v8, vt, zeta[0x01], vl);
-  MLK_RVV_BFLY_FX(v1, v9, vt, zeta[0x01], vl);
-  MLK_RVV_BFLY_FX(v2, va, vt, zeta[0x01], vl);
-  MLK_RVV_BFLY_FX(v3, vb, vt, zeta[0x01], vl);
-  MLK_RVV_BFLY_FX(v4, vc, vt, zeta[0x01], vl);
-  MLK_RVV_BFLY_FX(v5, vd, vt, zeta[0x01], vl);
-  MLK_RVV_BFLY_FX(v6, ve, vt, zeta[0x01], vl);
-  MLK_RVV_BFLY_FX(v7, vf, vt, zeta[0x01], vl);
-
-  MLK_RVV_BFLY_FX(v0, v4, vt, zeta[0x10], vl);
-  MLK_RVV_BFLY_FX(v1, v5, vt, zeta[0x10], vl);
-  MLK_RVV_BFLY_FX(v2, v6, vt, zeta[0x10], vl);
-  MLK_RVV_BFLY_FX(v3, v7, vt, zeta[0x10], vl);
-  MLK_RVV_BFLY_FX(v8, vc, vt, zeta[0x11], vl);
-  MLK_RVV_BFLY_FX(v9, vd, vt, zeta[0x11], vl);
-  MLK_RVV_BFLY_FX(va, ve, vt, zeta[0x11], vl);
-  MLK_RVV_BFLY_FX(vb, vf, vt, zeta[0x11], vl);
-
-  MLK_RVV_BFLY_FX(v0, v2, vt, zeta[0x20], vl);
-  MLK_RVV_BFLY_FX(v1, v3, vt, zeta[0x20], vl);
-  MLK_RVV_BFLY_FX(v4, v6, vt, zeta[0x21], vl);
-  MLK_RVV_BFLY_FX(v5, v7, vt, zeta[0x21], vl);
-  MLK_RVV_BFLY_FX(v8, va, vt, zeta[0x30], vl);
-  MLK_RVV_BFLY_FX(v9, vb, vt, zeta[0x30], vl);
-  MLK_RVV_BFLY_FX(vc, ve, vt, zeta[0x31], vl);
-  MLK_RVV_BFLY_FX(vd, vf, vt, zeta[0x31], vl);
-
-  MLK_RVV_BFLY_FX(v0, v1, vt, zeta[0x40], vl);
-  MLK_RVV_BFLY_FX(v2, v3, vt, zeta[0x41], vl);
-  MLK_RVV_BFLY_FX(v4, v5, vt, zeta[0x50], vl);
-  MLK_RVV_BFLY_FX(v6, v7, vt, zeta[0x51], vl);
-  MLK_RVV_BFLY_FX(v8, v9, vt, zeta[0x60], vl);
-  MLK_RVV_BFLY_FX(va, vb, vt, zeta[0x61], vl);
-  MLK_RVV_BFLY_FX(vc, vd, vt, zeta[0x70], vl);
-  MLK_RVV_BFLY_FX(ve, vf, vt, zeta[0x71], vl);
+  MLK_RVV_BFLY_FX(v0, v8, vt, zeta[0x01], vl, 1);
+  MLK_RVV_BFLY_FX(v1, v9, vt, zeta[0x01], vl, 1);
+  MLK_RVV_BFLY_FX(v2, va, vt, zeta[0x01], vl, 1);
+  MLK_RVV_BFLY_FX(v3, vb, vt, zeta[0x01], vl, 1);
+  MLK_RVV_BFLY_FX(v4, vc, vt, zeta[0x01], vl, 1);
+  MLK_RVV_BFLY_FX(v5, vd, vt, zeta[0x01], vl, 1);
+  MLK_RVV_BFLY_FX(v6, ve, vt, zeta[0x01], vl, 1);
+  MLK_RVV_BFLY_FX(v7, vf, vt, zeta[0x01], vl, 1);
+
+  MLK_RVV_BFLY_FX(v0, v4, vt, zeta[0x10], vl, 2);
+  MLK_RVV_BFLY_FX(v1, v5, vt, zeta[0x10], vl, 2);
+  MLK_RVV_BFLY_FX(v2, v6, vt, zeta[0x10], vl, 2);
+  MLK_RVV_BFLY_FX(v3, v7, vt, zeta[0x10], vl, 2);
+  MLK_RVV_BFLY_FX(v8, vc, vt, zeta[0x11], vl, 2);
+  MLK_RVV_BFLY_FX(v9, vd, vt, zeta[0x11], vl, 2);
+  MLK_RVV_BFLY_FX(va, ve, vt, zeta[0x11], vl, 2);
+  MLK_RVV_BFLY_FX(vb, vf, vt, zeta[0x11], vl, 2);
+
+  MLK_RVV_BFLY_FX(v0, v2, vt, zeta[0x20], vl, 3);
+  MLK_RVV_BFLY_FX(v1, v3, vt, zeta[0x20], vl, 3);
+  MLK_RVV_BFLY_FX(v4, v6, vt, zeta[0x21], vl, 3);
+  MLK_RVV_BFLY_FX(v5, v7, vt, zeta[0x21], vl, 3);
+  MLK_RVV_BFLY_FX(v8, va, vt, zeta[0x30], vl, 3);
+  MLK_RVV_BFLY_FX(v9, vb, vt, zeta[0x30], vl, 3);
+  MLK_RVV_BFLY_FX(vc, ve, vt, zeta[0x31], vl, 3);
+  MLK_RVV_BFLY_FX(vd, vf, vt, zeta[0x31], vl, 3);
+
+  MLK_RVV_BFLY_FX(v0, v1, vt, zeta[0x40], vl, 4);
+  MLK_RVV_BFLY_FX(v2, v3, vt, zeta[0x41], vl, 4);
+  MLK_RVV_BFLY_FX(v4, v5, vt, zeta[0x50], vl, 4);
+  MLK_RVV_BFLY_FX(v6, v7, vt, zeta[0x51], vl, 4);
+  MLK_RVV_BFLY_FX(v8, v9, vt, zeta[0x60], vl, 4);
+  MLK_RVV_BFLY_FX(va, vb, vt, zeta[0x61], vl, 4);
+  MLK_RVV_BFLY_FX(vc, vd, vt, zeta[0x70], vl, 4);
+  MLK_RVV_BFLY_FX(ve, vf, vt, zeta[0x71], vl, 4);
 
   __riscv_vse16_v_i16m2(
       &r[0x00], mlk_rv64v_ntt2(__riscv_vcreate_v_i16m1_i16m2(v0, v1), z0), vl2);
@@ -614,9 +634,9 @@ void mlk_rv64v_poly_tomont(int16_t *r)
 
   for (size_t i = 0; i < MLKEM_N; i += vl)
   {
-    __riscv_vse16_v_i16m1(
-        &r[i], fq_mul_vx(__riscv_vle16_v_i16m1(&r[i], vl), MLK_RVV_MONT_R2, vl),
-        vl);
+    vint16m1_t vec = __riscv_vle16_v_i16m1(&r[i], vl);
+    vec = fq_mul_vx(vec, MLK_RVV_MONT_R2, vl);
+    __riscv_vse16_v_i16m1(&r[i], vec, vl);
   }
 }
 
@@ -638,8 +658,6 @@ void mlk_rv64v_poly_reduce(int16_t *r)
   {
     vt = __riscv_vle16_v_i16m1(&r[i], vl);
     vt = fq_barrett(vt, vl);
-
-    /*  make positive */
     vt = fq_cadd(vt, vl);
     __riscv_vse16_v_i16m1(&r[i], vt, vl);
   }