Illegal delay slot code causes abort on mips64

[Brian Campbell]

During some randomised testing of an experimental MIPS implementation I found an instruction sequence that also causes aborts on mainline qemu's MIPS support. The problem is triggered by an MSA branch instruction appearing in a delay slot when emulating a processor without MSA support.

For example, with the current repository HEAD (f073cd3a2bf1054135271b837c58a7da650dd84b) configured for mips64-softmmu, if I run the attached binary using

    mips64-softmmu/qemu-system-mips64 -bios ../abort2.bin -machine mipssim -nographic

it will report

    unknown branch 0x13000
    Aborted (core dumped)

The binary contains the following two instructions:

    00200008 jr at
    47081e61 bz.b w8,0xffffffffbfc0798c

The jr sets up a jump, and hflags is set accordingly in gen_compute_branch (in target/mips/translate.c). When processing the bz.b, check_insn generates an exception because the instruction isn't support, but gen_msa_branch skips the usual delay slot check for the same reason, and sets more bits in hflags, leading to an abort in gen_branch because the hflags are now invalid.

I suspect the best fix is to remove the instruction set condition from the delay slot check in gen_msa_branch.

[Brian Campbell (bacam)]

I've just found the same problem with gen_compute_branch1,

00200008 jr at
4540563a bc1any4f $fcc0,0xffffffffbfc158ec

The cause is the same - if the instruction set is wrong then the delay slot check is skipped.

[Yongbok Kim (yongbok-kim)]

Thanks for reporting this issue.
In fact, branches in a delay slot is "undefined" in the pre-Release 6 architecture.
MIPS architectre release 6 defines to signal Reserved Instruction exceptions for such cases.
However as it was undefined, it is better to signal RI and carry on rather than stopping simulation.
Hence I've made a patch for the msa case.
I will have a look into the other case. (sorry I've missed in the first place.)

[Thomas Huth (th-huth)]

http://git.qemu.org/?p=qemu.git;a=commitdiff;h=075a1fe788d36b271ec2

[Brian Campbell (bacam)]

Thanks for that fix. I've just noticed that the second part, in gen_compute_branch1, wasn't included, though. Could you take a look at it?

[martin short (martin-sk)]

I found the exact same bug. Tested on several hosts and qemu releases. The newest one I tested was on FreeBSD 12.1 host and qemu-4.1.1_1 built from ports.

Instructions:
  4000d0: 0320f809 jalr t9
  4000d4: 45454545 0x45454545 # bc1any4t $fcc1,0x800101f8

I was running qemu-mips as:

qemu-system-mipsel -s -m 1024 -M malta \
        -kernel vmlinux-3.16.0-6-4kc-malta -initrd initrd.img-3.16.0-6-4kc-malta \
 -device virtio-blk-pci,drive=hd0 -drive if=none,id=hd0,format=qcow2,file=debian_wheezy_mipsel_standard.qcow2 \
 -append "root=/dev/vda1" \
 -device virtio-net-pci,netdev=net0 \
 -netdev user,id=net0,hostfwd=tcp::1666-:22,ipv6=off \
 -curses

abort() was in target/mips/translate.c:12945, in gen_branch().

Doesn't really matter if the instruction is supported on given CPU, user can crash the qemu within guest.

[Philippe Mathieu-Daudé (philmd)]

Hi Brian,

You try to execute a CP1 instruction in a delay slot,
which triggers a Reserved Instruction exception.
Per the ISA the processor operation is UNPREDICTABLE in such case.

What is the behavior on real hardware?
An assertion() seems appropriate.

Your compiler might be buggy, or you are not compiling for the correct CPU
(or you are not using the correct QEMU cpu).

[martin short (martin-sk)]

I don't know how Brian go to his state.

I should've mentioned though I was using custom binary (shellcode) that triggered this behavior. This code was not generated by compiler.

However, I wanted to point out that user can crash the qemu host by running custom code from userspace.

Unfortunately I can't test this behavior on real HW right now.

[Peter Maydell (pmaydell)]

Yeah, QEMU crashing is definitely a bug that we should fix. (NB that it's not a 'security' bug, though -- we make no guarantee that malicious code run under QEMU with TCG emulation is unable to escape from it: there's too much unaudited and old code for us to be able to safely make that guarantee.)

[martin short (martin-sk)]

When I reread the thread I see Brian was doing some testing/fuzzing, that's why he found that out.

I managed to get my old router running. It's BCM5354 (BCM3302 v2.9) running on Linux 2.4.35.
I used the following code (gnu as compiled but replaced the nop after branch with the branch instruction above):

  4000d0: 10000003 b 4000e0 <__start+0x10>
  4000d4: 45454545 0x45454545
 ...
  4000e0: 2404002a li a0,42
  4000e4: 24020fa1 li v0,4001
  4000e8: 0000000c syscall
  4000ec: 00000000 nop

Program was terminated with the trap Illegal instruction.

[Brian Campbell (bacam)]

If my memory is correct, this problem doesn't need qemu to execute the code, it only needs it to translate the code. In the original test case the invalid instructions were actually dead code but still managed to crash qemu.

I suggest following Yongbok Kim's approach and signalling Reserved Instruction in the same way R6 does. I think that's architecturally allowed, although I admit that it's ages since I looked at this.

[Thomas Huth (th-huth)]

This is an automated cleanup. This bug report has been moved
to QEMU's new bug tracker on gitlab.com and thus gets marked
as 'expired' now. Please continue with the discussion here:

 https://gitlab.com/qemu-project/qemu/-/issues/63