Alex, Richard,
On 9/4/25 2:16 AM, Nicholas Piggin wrote:
On Wed, Sep 03, 2025 at 05:13:36PM -0300, Daniel Henrique Barboza wrote:
Hi Nick,
^ typo in the patch subject: s/risvc/riscv
Well I'm off to a fine start :/
On 9/3/25 12:01 AM, Nicholas Piggin wrote:
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 <njoaq...@tenstorrent.com>
Reported-by: Ganesh Valliappan <gvalliap...@tenstorrent.com>
Signed-off-by: Nicholas Piggin <npig...@gmail.com>
---
target/riscv/vector_helper.c | 2 +
tests/tcg/riscv64/Makefile.target | 5 ++
tests/tcg/riscv64/test-vstart-overflow.c | 75 ++++++++++++++++++++++++
3 files changed, 82 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..19a49b6467 100644
--- a/tests/tcg/riscv64/Makefile.target
+++ b/tests/tcg/riscv64/Makefile.target
@@ -18,3 +18,8 @@ TESTS += test-fcvtmod
test-fcvtmod: CFLAGS += -march=rv64imafdc
test-fcvtmod: LDFLAGS += -static
run-test-fcvtmod: QEMU_OPTS += -cpu rv64,d=true,zfa=true
+
+# Test for vstart >= vl
+TESTS += test-vstart-overflow
+test-vstart-overflow: CFLAGS += -march=rv64gcv
+run-test-vstart-overflow: QEMU_OPTS += -cpu rv64,v=on
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>
The fix in vector_helper.c is fine but this patch (and patch 3) won't execute
'make check-tcg'. It complains about this header being missing in the docker
env.
To eliminate the possibility of my env being the problem I ran this series in
Gitlab. Same error:
https://gitlab.com/danielhb/qemu/-/jobs/11236091281
/builds/danielhb/qemu/tests/tcg/riscv64/test-vstart-overflow.c:13:10: fatal
error: riscv_vector.h: No such file or directory
3899
13 | #include <riscv_vector.h>
3900
| ^~~~~~~~~~~~~~~~
3901
compilation terminated.
3902
make[1]: *** [Makefile:122: test-vstart-overflow] Error 1
I believe you need to add the Docker changes you made in this patch. Same
thing for patch 3. And same thing for patch 4 of:
[PATCH 0/4] linux-user/riscv: add vector state to signal
Given that you're also using riscv_vector.h in there too. Thanks,
Hmm, thanks. It did work for my local build.
I think the header is provided by the compiler, so I might have
to work out a way to skip the test if the compiler is too old.
GCC13 might have been the first one to support.
How hard it is to update the GCC version we're running in the docker images for
"check-tcg"? We would like to use a RISC-V vector header that isn't supported
ATM.
Thanks,
Daniel
I was considering writing .S files for these. Should have done so
if I realized, but nevermind.
Thanks,
Nick
