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. I was considering writing .S files for these. Should have done so if I realized, but nevermind. Thanks, Nick
