diff mbox

[v2] target-arm: check that LSB <= MSB in BFI instruction

Message ID 1422622788-29375-1-git-send-email-batuzovk@ispras.ru
State New
Headers show

Commit Message

Kirill Batuzov Jan. 30, 2015, 12:59 p.m. UTC
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 |    4 ++++
 1 file changed, 4 insertions(+)

Comments

Peter Maydell Feb. 3, 2015, 11:47 a.m. UTC | #1
On 30 January 2015 at 12:59, 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.
>
> 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>



Applied to target-arm.next, thanks.

-- PMM
diff mbox

Patch

diff --git a/target-arm/translate.c b/target-arm/translate.c
index bdfcdf1..2c1c2a7 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -8739,6 +8739,10 @@  static void disas_arm_insn(DisasContext *s, unsigned int insn)
                         ARCH(6T2);
                         shift = (insn >> 7) & 0x1f;
                         i = (insn >> 16) & 0x1f;
+                        if (i < shift) {
+                            /* UNPREDICTABLE; we choose to UNDEF */
+                            goto illegal_op;
+                        }
                         i = i + 1 - shift;
                         if (rm == 15) {
                             tmp = tcg_temp_new_i32();