[PATCH v2 2/3] target/risvc: Fix vector whole ldst vstart check

Nicholas Piggin posted 3 patches 1 day, 6 hours ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
[PATCH v2 2/3] target/risvc: Fix vector whole ldst vstart check
Posted by Nicholas Piggin 1 day, 6 hours ago
The whole vector ldst instructions do not include a vstart check, so an
overflowed vstart can result in an underflowed memory address offset and
crash:

    accel/tcg/cputlb.c:1465:probe_access_flags:
      assertion failed: (-(addr | TARGET_PAGE_MASK) >= size)

Add the VSTART_CHECK_EARLY_EXIT() check for these helpers.

This was found with a verification test generator based on RiESCUE.

Reported-by: Nicholas Joaquin <njoaquin@tenstorrent.com>
Reported-by: Ganesh Valliappan <gvalliappan@tenstorrent.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/vector_helper.c             |  2 +
 tests/tcg/riscv64/Makefile.target        | 10 ++++
 tests/tcg/riscv64/test-vstart-overflow.c | 75 ++++++++++++++++++++++++
 3 files changed, 87 insertions(+)
 create mode 100644 tests/tcg/riscv64/test-vstart-overflow.c

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index fc85a34a84..e0e8735000 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -825,6 +825,8 @@ vext_ldst_whole(void *vd, target_ulong base, CPURISCVState *env, uint32_t desc,
     uint32_t esz = 1 << log2_esz;
     int mmu_index = riscv_env_mmu_index(env, false);
 
+    VSTART_CHECK_EARLY_EXIT(env, evl);
+
     /* Calculate the page range of first page */
     addr = base + (env->vstart << log2_esz);
     page_split = -(addr | TARGET_PAGE_MASK);
diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
index 4da5b9a3b3..daa0c6688f 100644
--- a/tests/tcg/riscv64/Makefile.target
+++ b/tests/tcg/riscv64/Makefile.target
@@ -1,6 +1,9 @@
 # -*- Mode: makefile -*-
 # RISC-V specific tweaks
 
+HAVE_RISCV_VECTOR_INTRINSICS := $(shell echo '#ifndef __riscv_v_intrinsic\n#error\n#endif' |  \
+                                  $(CC) -march=rv64gcv -E -x c - >/dev/null 2>&1 && echo y)
+
 VPATH += $(SRC_PATH)/tests/tcg/riscv64
 TESTS += test-div
 TESTS += noexec
@@ -18,3 +21,10 @@ TESTS += test-fcvtmod
 test-fcvtmod: CFLAGS += -march=rv64imafdc
 test-fcvtmod: LDFLAGS += -static
 run-test-fcvtmod: QEMU_OPTS += -cpu rv64,d=true,zfa=true
+
+ifeq ($(HAVE_RISCV_VECTOR_INTRINSICS),y)
+# Test for vstart >= vl
+TESTS += test-vstart-overflow
+test-vstart-overflow: CFLAGS += -march=rv64gcv
+run-test-vstart-overflow: QEMU_OPTS += -cpu rv64,v=on
+endif
diff --git a/tests/tcg/riscv64/test-vstart-overflow.c b/tests/tcg/riscv64/test-vstart-overflow.c
new file mode 100644
index 0000000000..72999f2c8a
--- /dev/null
+++ b/tests/tcg/riscv64/test-vstart-overflow.c
@@ -0,0 +1,75 @@
+/*
+ * Test for VSTART set to overflow VL
+ *
+ * TCG vector instructions should call VSTART_CHECK_EARLY_EXIT() to check
+ * this case, otherwise memory addresses can underflow and misbehave or
+ * crash QEMU.
+ *
+ * TODO: Add stores and other instructions.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include <stdint.h>
+#include <riscv_vector.h>
+
+#define VSTART_OVERFLOW_TEST(insn)                \
+({                                                \
+    uint8_t vmem[64] = { 0 };                     \
+    uint64_t vstart;                              \
+    asm volatile("                           \r\n \
+    # Set VL=52 and VSTART=56                \r\n \
+    li          t0, 52                       \r\n \
+    vsetvli     x0, t0, e8, m4, ta, ma       \r\n \
+    li          t0, 56                       \r\n \
+    csrrw       x0, vstart, t0               \r\n \
+    li          t1, 64                       \r\n \
+    " insn "                                 \r\n \
+    csrr        %0, vstart                   \r\n \
+    " : "=r"(vstart), "+A"(vmem) :: "t0", "t1", "v24", "memory"); \
+    vstart;                                       \
+})
+
+int run_vstart_overflow_tests()
+{
+    /*
+     * An implementation is permitted to raise an illegal instruction
+     * exception when executing a vector instruction if vstart is set to a
+     * value that could not be produced by the execution of that instruction
+     * with the same vtype. If TCG is changed to do this, then this test
+     * could be updated to handle the SIGILL.
+     */
+    if (VSTART_OVERFLOW_TEST("vl1re16.v    v24, %1")) {
+        return 1;
+    }
+
+    if (VSTART_OVERFLOW_TEST("vs1r.v       v24, %1")) {
+        return 1;
+    }
+
+    if (VSTART_OVERFLOW_TEST("vle16.v      v24, %1")) {
+        return 1;
+    }
+
+    if (VSTART_OVERFLOW_TEST("vse16.v      v24, %1")) {
+        return 1;
+    }
+
+    if (VSTART_OVERFLOW_TEST("vluxei8.v    v24, %1, v20")) {
+        return 1;
+    }
+
+    if (VSTART_OVERFLOW_TEST("vlse16.v     v24, %1, t1")) {
+        return 1;
+    }
+
+    if (VSTART_OVERFLOW_TEST("vlseg2e8.v  v24, %1")) {
+        return 1;
+    }
+
+    return 0;
+}
+
+int main()
+{
+    return run_vstart_overflow_tests();
+}
-- 
2.51.0