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 | 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>
+
+#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.0Hi Nick,
^ typo in the patch subject: s/risvc/riscv
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 <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 | 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,
Daniel
> +
> +#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();
> +}
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 <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 | 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. I was considering writing .S files for these. Should have done so if I realized, but nevermind. Thanks, Nick
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 <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 | 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
On 9/4/25 13:06, Daniel Henrique Barboza wrote: > 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. If debian packages the gcc version, then it's easy: change gcc-riscv-linux-gnu to gcc-NN-riscv-linux-gnu If the version isn't packaged, then it's harder, and we would need to either build our own gcc within the container (see dockerfiles/debian-microblaze-cross.d/build-toolchain.sh), or you can host a pre-built version somewhere (see dockerfiles/debian-loongarch-cross.docker). r~
© 2016 - 2025 Red Hat, Inc.