| Message ID | 1422621361-23408-1-git-send-email-batuzovk@ispras.ru |
|---|---|
| State | New |
| Headers | show |
On 30 January 2015 at 12:36, Kirill Batuzov <batuzovk@ispras.ru> wrote: > The documentation states that if LSB > MSB in BFI instruction behaviour > is unpredictable. Currently QEMU crashes because of assertion failure in > this case: > > tcg/tcg-op.h:2061: tcg_gen_deposit_i32: Assertion `len <= 32' failed. > > While assertion failure may meet the "unpredictable" definition this > behaviour is undesirable because it allows an unprivileged guest program > to crash the emulator with the OS and other programs. Thanks for this bug fix. Some minor nits: > diff --git a/target-arm/translate.c b/target-arm/translate.c > index bdfcdf1..2821289 100644 > --- a/target-arm/translate.c > +++ b/target-arm/translate.c > @@ -8739,6 +8739,8 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) > ARCH(6T2); > shift = (insn >> 7) & 0x1f; > i = (insn >> 16) & 0x1f; > + if (i < shift) > + goto illegal_op; This needs braces to comply with coding style. checkpatch.pl should warn you about this. I also like to comment this kind of check with /* UNPREDICTABLE; we choose to UNDEF */ > i = i + 1 - shift; > if (rm == 15) { > tmp = tcg_temp_new_i32(); I checked the Thumb decoder, and that code seems to have this test already, so it's just the ARM decoder that needs fixing. thanks -- PMM
diff --git a/target-arm/translate.c b/target-arm/translate.c index bdfcdf1..2821289 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -8739,6 +8739,8 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) ARCH(6T2); shift = (insn >> 7) & 0x1f; i = (insn >> 16) & 0x1f; + if (i < shift) + goto illegal_op; i = i + 1 - shift; if (rm == 15) { tmp = tcg_temp_new_i32();
The documentation states that if LSB > MSB in BFI instruction behaviour is unpredictable. Currently QEMU crashes because of assertion failure in this case: tcg/tcg-op.h:2061: tcg_gen_deposit_i32: Assertion `len <= 32' failed. While assertion failure may meet the "unpredictable" definition this behaviour is undesirable because it allows an unprivileged guest program to crash the emulator with the OS and other programs. This patch addresses the issue by throwing illegal instruction exception if LSB > MSB. Only ARM decoder is affected because Thumb decoder already has this check in place. To reproduce issue run the following program int main(void) { asm volatile (".long 0x07c00c12" :: ); return 0; } compiled with gcc -marm -static badop_arm.c -o badop_arm Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru> --- target-arm/translate.c | 2 ++ 1 file changed, 2 insertions(+)