Bug 1198868 - Qemu crashed when start guest with invalid pvpanic ioport on rhel6.7
Summary: Qemu crashed when start guest with invalid pvpanic ioport on rhel6.7
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: qemu-kvm
Version: 6.7
Hardware: Unspecified
OS: Unspecified
low
low
Target Milestone: rc
: ---
Assignee: Laszlo Ersek
QA Contact: Virtualization Bugs
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-03-05 00:51 UTC by vivian zhang
Modified: 2015-03-13 05:53 UTC (History)
13 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-03-13 05:53:32 UTC
Target Upstream Version:


Attachments (Terms of Use)

Description vivian zhang 2015-03-05 00:51:07 UTC
Description:
When guest configured with invalid ioport for pvpanic device, guest start failed and qemu crashed.

Version-Release number of selected component (if applicable):
qemu-kvm-rhev-0.12.1.2-2.455.el6.x86_64
libvirt-0.10.2-49.el6.x86_64
2.6.32-541.el6.x86_64


How reproducible:
100%

Steps:
1. prepare a guest, configured with panic device, edit ioport to an invalid value (default iobase='0x505')
# virsh edit test1
...
    <panic>
      <address type='isa' iobase='0x9'/>
    </panic>

...


2. start guest failed, qemu crashed
# virsh start test1
error: Failed to start domain test1
error: Unable to read from monitor: Connection reset by peer

3. check qemu log and core dump information
qemu log:
2015-03-04 06:49:29.314+0000: starting up
LC_ALL=C PATH=/sbin:/usr/sbin:/bin:/usr/bin QEMU_AUDIO_DRV=none /usr/libexec/qemu-kvm -name test1 -S -machine rhel6.6.0,dump-guest-core=on -enable-kvm -m 1024 -realtime mlock=off -smp 1,sockets=1,cores=1,threads=1 -uuid fedd0a8d-91e2-d94d-69f9-f4fbc0986274 -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/test1.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -global PIIX4_PM.disable_s3=0 -global PIIX4_PM.disable_s4=0 -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x5.0x7 -device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x5 -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x5.0x1 -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x5.0x2 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x7 -drive file=/tmp/pl/rhel6.img,if=none,id=drive-ide0-0-0,format=qcow2,cache=none -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 -netdev tap,fd=30,id=hostnet0 -device rtl8139,netdev=hostnet0,id=net0,mac=52:54:00:e5:c2:eb,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -chardev spicevmc,id=charchannel0,name=vdagent -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0 -chardev socket,id=charchannel1,path=/var/lib/libvirt/qemu/test.org.qemu.guest_agent.0,server,nowait -device virtserialport,bus=virtio-serial0.0,nr=2,chardev=charchannel1,id=channel1,name=org.qemu.guest_agent.0 -vnc 127.0.0.1:0 -vga cirrus -device intel-hda,id=sound0,bus=pci.0,addr=0x4 -device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 -device pvpanic,ioport=9 -msg timestamp=on
....

qemu: hardware error: register_ioport_read: invalid opaque
CPU #0:
EAX=00000000 EBX=00000000 ECX=00000000 EDX=000006d3
ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000
EIP=0000fff0 EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =0000 00000000 0000ffff 00009300
CS =f000 ffff0000 0000ffff 00009b00
SS =0000 00000000 0000ffff 00009300
DS =0000 00000000 0000ffff 00009300
FS =0000 00000000 0000ffff 00009300
GS =0000 00000000 0000ffff 00009300
LDT=0000 00000000 0000ffff 00008200
TR =0000 00000000 0000ffff 00008b00
GDT=     00000000 0000ffff
IDT=     00000000 0000ffff
CR0=60000010 CR2=00000000 CR3=00000000 CR4=00000000
DR0=00000000 DR1=00000000 DR2=00000000 DR3=00000000
DR6=ffff0ff0 DR7=00000400
FCW=037f FSW=0000 [ST=0] FTW=00 MXCSR=00001f80
FPR0=0000000000000000 0000 FPR1=0000000000000000 0000
FPR2=0000000000000000 0000 FPR3=0000000000000000 0000
FPR4=0000000000000000 0000 FPR5=0000000000000000 0000
FPR6=0000000000000000 0000 FPR7=0000000000000000 0000
XMM00=00000000000000000000000000000000 XMM01=00000000000000000000000000000000
XMM02=00000000000000000000000000000000 XMM03=00000000000000000000000000000000
XMM04=00000000000000000000000000000000 XMM05=00000000000000000000000000000000
XMM06=00000000000000000000000000000000 XMM07=00000000000000000000000000000000
2015-03-04 06:49:33.818+0000: shutting down



Core dump:

(gdb) bt
 #0  0x00007fd18f461625 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
 #1  0x00007fd18f462e05 in abort () at abort.c:92
 #2  0x00007fd1929db782 in hw_error (fmt=<value optimized out>) at /usr/src/debug/qemu-kvm-0.12.1.2/vl.c:521
 #3  0x00007fd192a5a78d in register_ioport_read (start=<value optimized out>, length=<value optimized out>, size=6,
     func=0xffffffffffffffff, opaque=0x7fd1929239a0) at /usr/src/debug/qemu-kvm-0.12.1.2/ioport.c:152
 #4  0x00007fd192b97874 in pvpanic_isa_init (dev=0x7fd1935c5120) at /usr/src/debug/qemu-kvm-0.12.1.2/hw/pvpanic.c:80
 #5  0x00007fd192a76f58 in qdev_init (dev=0x7fd1935c5120) at /usr/src/debug/qemu-kvm-0.12.1.2/hw/qdev.c:285
 #6  0x00007fd192a7736f in qdev_device_add (opts=0x7fd19357d5f0) at /usr/src/debug/qemu-kvm-0.12.1.2/hw/qdev.c:260
 #7  0x00007fd1929d7aa9 in device_init_func (opts=<value optimized out>, opaque=<value optimized out>)
     at /usr/src/debug/qemu-kvm-0.12.1.2/vl.c:4803
 #8  0x00007fd192a108ea in qemu_opts_foreach (list=<value optimized out>, func=0x7fd1929d7aa0 <device_init_func>,
     opaque=0x0, abort_on_failure=<value optimized out>) at /usr/src/debug/qemu-kvm-0.12.1.2/qemu-option.c:1035
 #9  0x00007fd1929dc6bc in main (argc=<value optimized out>, argv=<value optimized out>, envp=<value optimized out>)
     at /usr/src/debug/qemu-kvm-0.12.1.2/vl.c:6539

4. this issue can not be hit on rhel7.1

Actual result:
Qemu crash when start guest with invalid pvpanic ioport

Expect result:
1. libvirt should check to forbid the invalid ioport configuration for panic
2. qemu could not crashed, guest start success

Comment 2 Laszlo Ersek 2015-03-06 18:09:59 UTC
This is the failing function:

/* size is the word size in byte */
int register_ioport_read(pio_addr_t start, int length, int size,
                         IOPortReadFunc *func, void *opaque)
{
    int i, bsize;

    if (ioport_bsize(size, &bsize)) {
        hw_error("register_ioport_read: invalid size");
        return -1;
    }
    for(i = start; i < start + length; i += size) {
        ioport_read_table[bsize][i] = func;
        if (ioport_opaque[i] != NULL && ioport_opaque[i] != opaque)
            hw_error("register_ioport_read: invalid opaque");
        ioport_opaque[i] = opaque;
    }
    return 0;
}

This infrastructure was completely replaced in qemu-1.6 by Jan Kiszka's series

[PATCH v3 00/14] Refactor portio dispatching
http://thread.gmane.org/gmane.comp.emulators.qemu/218232

upstream commits a8aec295^..5767e4e1.

Backporting this series to RHEL-6 is completely out of question; the best we can do here is figure out what exactly is wrong with placing pvpanic at ioport 9, and refusing it more gracefully.

Comment 3 Laszlo Ersek 2015-03-06 19:57:49 UTC
Okay, I know what's going on. Ioport 9 is already occupied,

Comment 4 Laszlo Ersek 2015-03-06 19:58:57 UTC
(previous comment saved unwittingly when modifying priority/severity)

(In reply to Laszlo Ersek from comment #3)
> Okay, I know what's going on. Ioport 9 is already occupied,

... and I'll figure out later by what exactly.

Comment 5 Laszlo Ersek 2015-03-10 14:13:39 UTC
(In reply to vivian zhang from comment #0)
> 4. this issue can not be hit on rhel7.1

Alright, so in order to establish a rhel-7.1 baseline here, I added the following snippet to a VM's domain XML on a RHEL-7.1 host:

    <panic>
          <address type='isa' iobase='0x9'/>  
    </panic>

under <devices>. Indeed the VM starts up (on RHEL-7.1). Note however the following output from the

  # virsh qemu-monitor-command DOMAIN --hmp info mtree

command:

I/O
  ...
  0000000000000008-000000000000000f (prio 0, RW): dma-cont
  0000000000000009-0000000000000009 (prio 0, RW): pvpanic
  ...

That is, an ioport region is registered by "dma-cont" (see "hw/dma/i8257.c" -- DMA controller) at base 0x8, size 0x8. In particular, the ioport at 0x9 is called "request register", for the 8-bit DMA controller:

http://wiki.osdev.org/ISA_DMA#The_Registers

So, this configuration is irrevocably broken. RHEL-7.1 doesn't complain because it uses the more recent MemoryRegion API, which explicitly supports overlapping regions. For example, outputting a uint16_t at ioport base 0x8 would be dispatched differently from outputting a uint8_t at ioport 0x9, even though the second byte of the first uint16_t output would "overlap" the ioport at 0x9.

RHEL-6 is not so smart wrt. overlapping ranges, and it simply refuses the conflicting port assignment when it sees it (I still have to confirm the other party in the conflict on RHEL-6.) So this is simply a user / configuration error, not a QEMU bug.

(In reply to vivian zhang from comment #0)

> Expect result:
> 1. libvirt should check to forbid the invalid ioport configuration for panic
> 2. qemu could not crashed, guest start success

Unfortunately, none of these are acceptable for RHEL-6. First, libvirt has absolutely no business deciding what ioports are valid for what devices. Second, porting the MemoryRegion API to RHEL-6 qemu-kvm is out of question; it is incredibly intrusive. (Plus, anyway, in this specific configuration, the MemoryRegion API just masks the error in the configuration.)

See also the domain XML documentation <http://libvirt.org/formatdomain.html#elementsPanic>:

  The default ioport is 0x505. Most users don't need to specify an address. 

I don't know what use case justifies setting 0x9 as pvpanic port.

I think the most appropriate closure for this BZ would be NOTABUG. RHEL-6 qemu-kvm detects the invalid configuration and it exits with an error.

The best we could do is beautify the error message. We could print "conflicting ioport assignment". However, it doesn't seem possible to say what conflicts with what without introducing new stuff to track ioport assignments, and I think that's out of scope for RHEL-6.

So here's what we can do:
- close this as NOTABUG.
- write a patch that changes the error messages

  register_ioport_read: invalid opaque
  register_ioport_write: invalid opaque

  to

  register_ioport_read: invalid opaque (probable ioport assignment conflict)
  register_ioport_write: invalid opaque (probable ioport assignment conflict)

Please advise as to what you would prefer.

Comment 6 Laszlo Ersek 2015-03-10 14:44:24 UTC
(In reply to Laszlo Ersek from comment #5)

> RHEL-6 is not so smart wrt. overlapping ranges, and it simply refuses the
> conflicting port assignment when it sees it (I still have to confirm the
> other party in the conflict on RHEL-6.) So this is simply a user /
> configuration error, not a QEMU bug.

Indeed; the first function registered as read handler for ioport 9 is read_cont(), from "hw/dma.c". This call succeeds.

The second (offending) call tries to register pvpanic_ioport_read() for the same, and that triggers the hw_error() call (justifiedly).

Comment 7 Laszlo Ersek 2015-03-11 09:07:51 UTC
Needinfo about the last part of comment 5. I propose NOTABUG, but if you would like a slightly more informative error message than currently printed, that can be done.

To summarize again, mapping pvpanic to a randomly chosen ioport is a user error. RHEL-6 qemu-kvm is right to reject it. RHEL-7 qemu-kvm does not reject it, because in RHEL-7 there is some infrastructure in place that allows more overlapping configurations than possible in RHEL-6 (*), but that doesn't change the fact that in this specific case, mapping pvpanic over ioport 9 is wrong. RHEL-7 doesn't complain about it, but the 8-bit DMA controller will be broken under such a setup. In short: don't mess with the pvpanic ioport unless you're 100% sure that the port you select is unoccupied.

(*) There are some use cases when non-intuitive ioport overlappings are valid. For example, ioport 0xCF9 (byte wide) overlaps with 0xCF8..0xCFB (4 byte wide), and they validly have separate roles. It's simply impossible to tell in advance, in a generic fashion, when such an overlap is valid and when not. The pvpanic setting that is the subject of this BZ is a user error.

Comment 8 vivian zhang 2015-03-12 01:52:08 UTC
hi, Laszlo
Thanks for your deep checking for this bug. 
I agree with that using ioport with 0x09, or some other values, such as 0x01, 0x02... as an invalid ioport is user configuration error. But as a negative test scenario, QEMU crashed caused by this negative testing is not acceptable.
So, I don't think it is reasonable to close this bug with NOTABUG, I suggest we should fix qemu crashed issue firstly and meanwhile give a patch to update the reasonable error message.

thanks

Comment 9 Laszlo Ersek 2015-03-12 02:16:55 UTC
As I said before, qemu *did not crash*. It exited on purpose, after it detected the error in configuration (the overlapping ioport assignments).

The hw_error() function that RHEL-6 qemu-kvm uses for reporting this error entails the following:
- print error message to stderr,
- dump state of VCPUs,
- call abort().

The abort() function raises SIGABRT. (See the backtrace in comment 0.) In particular, the exit status of the process will show that the process exited due to the SIGABRT signal, and not with a usual (zero or nonzero) exit code.

We can discuss the characteristics of *how* qemu exits under these circumstances. For example, we could consider reporting the error with something else than hw_error(), in register_ioport_read(); calling _exit() ultimately instead of the abort() that is currently called by hw_error() internally. That would change the exit status from "terminated with SIGABRT" to "exited with nonzero code", and a core dump would not be generated.

What we can't change is *whether* RHEL-6 qemu-kvm exits under the above circumstances. It simply must.

To reiterate, just because qemu terminates with SIGABRT that it raises itself, with abort(), that doesn't make this a "crash". It is completely different from a SIGSEGV for example that is generated by the kernel in response to a null pointer dereference. The SIGABRT in this case is 100% intentional in qemu. We can tune the specifics of *how* qemu should exit in this case, but we can't change *whether* it will exit. It must.

Comment 10 Ademar Reis 2015-03-13 05:53:32 UTC
Given explanation in comment #9, I'm closing it as NEXTRELEASE (it's OK in RHEL7).

This is very low priority and not worth the extra time to just beautify the error message, at least not until a customer hits it and complains (which is very very unlikely).


Note You need to log in before you can comment on or make changes to this bug.