include/tcg/tcg.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Sometimes, when the address of the passed TCGTemp *ts variable is the same as tcg_ctx,
the index calculated in the temp_idx function, i.e., ts - tcg_ctx->temps,
can result in a particularly large value, causing overflow in the subsequent array access.
0 0x00007f79590132ac in test_bit (addr=<optimized out>, nr=<optimized out>)
at /data/system/jiangzw/release_version/qemu8.2/include/qemu/bitops.h:135
1 init_ts_info (ctx=ctx@entry=0x7f794bffe460, ts=0x7f76fc000e00) at ../tcg/optimize.c:148
2 0x00007f7959014b50 in init_arguments (nb_args=2, op=0x7f76fc0101f8, ctx=0x7f794bffe460) at ../tcg/optimize.c:792
3 fold_call (op=0x7f76fc0101f8, ctx=0x7f794bffe460) at ../tcg/optimize.c:1348
4 tcg_optimize (s=<optimized out>) at ../tcg/optimize.c:2369
5 0x00007f7958ffa136 in tcg_gen_code (s=0x7f76fc000e00, tb=0x7f7904202380, pc_start=140741246462840) at ../tcg/tcg.c:6066
Signed-off-by: Zhiwei Jiang <jiangzw@tecorigin.com>
---
include/tcg/tcg.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index 05a1912f8a..4b38d2702d 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -629,7 +629,7 @@ static inline size_t temp_idx(TCGTemp *ts)
*/
static inline TCGTemp *tcgv_i32_temp(TCGv_i32 v)
{
- return (void *)tcg_ctx + (uintptr_t)v;
+ return (void *)tcg_ctx->temps + (uintptr_t)v;
}
#endif
@@ -681,7 +681,7 @@ static inline TCGArg tcgv_vec_arg(TCGv_vec v)
static inline TCGv_i32 temp_tcgv_i32(TCGTemp *t)
{
(void)temp_idx(t); /* trigger embedded assert */
- return (TCGv_i32)((void *)t - (void *)tcg_ctx);
+ return (TCGv_i32)((void *)t - (void *)tcg_ctx->temps);
}
static inline TCGv_i64 temp_tcgv_i64(TCGTemp *t)
--
2.17.1
On 4/18/24 03:27, Zhiwei Jiang wrote: > Sometimes, when the address of the passed TCGTemp *ts variable is the same as tcg_ctx, Pardon? When would TCGTemp *ts == TCGContext *tcg_ctx? > the index calculated in the temp_idx function, i.e., ts - tcg_ctx->temps, > can result in a particularly large value, causing overflow in the subsequent array access. Or, assert: size_t temp_idx(TCGTemp *ts) { ptrdiff_t n = ts - tcg_ctx->temps; assert(n >= 0 && n < tcg_ctx->nb_temps); return n; } > static inline TCGTemp *tcgv_i32_temp(TCGv_i32 v) > { > - return (void *)tcg_ctx + (uintptr_t)v; > + return (void *)tcg_ctx->temps + (uintptr_t)v; > } This will generate 0 for the first temp, which will test as NULL. r~
> On 4/18/24 03:27, Zhiwei Jiang wrote: > > Sometimes, when the address of the passed TCGTemp *ts variable is the same as tcg_ctx, > > Pardon? When would TCGTemp *ts == TCGContext *tcg_ctx? > > > > the index calculated in the temp_idx function, i.e., ts - tcg_ctx->temps, > > can result in a particularly large value, causing overflow in the subsequent array access. > > Or, assert: > > size_t temp_idx(TCGTemp *ts) > { > ptrdiff_t n = ts - tcg_ctx->temps; > assert(n >= 0 && n < tcg_ctx->nb_temps); > return n; > } > > > static inline TCGTemp *tcgv_i32_temp(TCGv_i32 v) > > { > > - return (void *)tcg_ctx + (uintptr_t)v; > > + return (void *)tcg_ctx->temps + (uintptr_t)v; > > } > > This will generate 0 for the first temp, which will test as NULL. Hi Richard: You can reproduce this issue on the latest upstream QEMU version. Using the RISC-V QEMU version, if we compile a test program with the first instruction being '.insn r 0xf, 2, 0, x0, x0, x0',that is a RISC-V CBO instruction, qemu will crash with a segmentation fault upon execution. When the first instruction in the program is a CBO instruction, temp_idx in init_ts_info func returns a very large value, causing the subsequent test_bit function to access out-of-bounds memory. static void init_ts_info(OptContext *ctx, TCGTemp *ts) { size_t idx = temp_idx(ts); TempOptInfo *ti; if (test_bit(idx, ctx->temps_used.l)) { return; } ... I can fix this segmentation fault by applying the modification above, and it seems more logical in terms of code logic to match the allocation and indexing of TCGTemp. Ths
On Fri, 19 Apr 2024 at 04:49, 姜智伟 <jiangzw@tecorigin.com> wrote: > > > On 4/18/24 03:27, Zhiwei Jiang wrote: > > > Sometimes, when the address of the passed TCGTemp *ts variable is the same as tcg_ctx, > > > > Pardon? When would TCGTemp *ts == TCGContext *tcg_ctx? > > > > > > > the index calculated in the temp_idx function, i.e., ts - tcg_ctx->temps, > > > can result in a particularly large value, causing overflow in the subsequent array access. > > > > Or, assert: > > > > size_t temp_idx(TCGTemp *ts) > > { > > ptrdiff_t n = ts - tcg_ctx->temps; > > assert(n >= 0 && n < tcg_ctx->nb_temps); > > return n; > > } > > > > > static inline TCGTemp *tcgv_i32_temp(TCGv_i32 v) > > > { > > > - return (void *)tcg_ctx + (uintptr_t)v; > > > + return (void *)tcg_ctx->temps + (uintptr_t)v; > > > } > > > > This will generate 0 for the first temp, which will test as NULL. > > Hi Richard: > You can reproduce this issue on the latest upstream QEMU version. Using the RISC-V QEMU version, > if we compile a test program with the first instruction being '.insn r 0xf, 2, 0, x0, x0, x0',that is a RISC-V CBO instruction, > qemu will crash with a segmentation fault upon execution. > > When the first instruction in the program is a CBO instruction, temp_idx in init_ts_info func returns a very large value, > causing the subsequent test_bit function to access out-of-bounds memory. I feel like this might be a bug elsewhere. Can you provide a repro binary and command line? thanks -- PMM
> > > On 4/18/24 03:27, Zhiwei Jiang wrote: > > > > Sometimes, when the address of the passed TCGTemp *ts variable is the same as tcg_ctx, > > > > > > Pardon? When would TCGTemp *ts == TCGContext *tcg_ctx? > > > > > > > > > > the index calculated in the temp_idx function, i.e., ts - tcg_ctx->temps, > > > > can result in a particularly large value, causing overflow in the subsequent array access. > > > > > > Or, assert: > > > > > > size_t temp_idx(TCGTemp *ts) > > > { > > > ptrdiff_t n = ts - tcg_ctx->temps; > > > assert(n >= 0 && n < tcg_ctx->nb_temps); > > > return n; > > > } > > > > > > > static inline TCGTemp *tcgv_i32_temp(TCGv_i32 v) > > > > { > > > > - return (void *)tcg_ctx + (uintptr_t)v; > > > > + return (void *)tcg_ctx->temps + (uintptr_t)v; > > > > } > > > > > > This will generate 0 for the first temp, which will test as NULL. > > > > Hi Richard: > > You can reproduce this issue on the latest upstream QEMU version. Using the RISC-V QEMU version, > > if we compile a test program with the first instruction being '.insn r 0xf, 2, 0, x0, x0, x0',that is a RISC-V CBO instruction, > > qemu will crash with a segmentation fault upon execution. > > > > When the first instruction in the program is a CBO instruction, temp_idx in init_ts_info func returns a very large value, > > causing the subsequent test_bit function to access out-of-bounds memory. > > I feel like this might be a bug elsewhere. Can you provide > a repro binary and command line? The test file has been attached with RISCV CBO instruction as the first instruction to execute, with command-line arguments as ./build/qemu-system-riscv64 -M virt -smp 1 -nographic -bios crash_test.bin � � � � � � � � � � � � � � � � s4s 4s 4s%@���U�� ��s�R0s#P0c�b �"