Bug 528901

Summary: qemu-kvm : msrs[] array in kvm_arch_save_regs() too small / may cause stack corruption
Product: [Fedora] Fedora Reporter: Ulrich Obergfell <uobergfe>
Component: qemuAssignee: Glauber Costa <gcosta>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: high    
Version: 11CC: berrange, ehabkost, gcosta, jforbes, markmc, quintela
Target Milestone: ---   
Target Release: ---   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: 0.10.6-9.fc11 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-11-04 07:12:02 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 480594    

Description Ulrich Obergfell 2009-10-14 05:44:11 EDT
1.  Description of Problem
    ----------------------


    kvm_arch_save_regs() is called in the following code path when the
    'savevm' command is being issued at the '(qemu)' monitor prompt.

      do_savevm
        qemu_savevm_state
          qemu_savevm_state_complete
            for (se = first_se; se != NULL; se = se->next)
                // se->save_state
                cpu_save
                  kvm_save_registers
                  ...
                  kvm_do_save_registers
                    kvm_arch_save_regs

    As shown in the following code fragment, kvm_arch_save_regs() allocates
    an array of 'struct kvm_msr_entry' in the local stackframe. The size of
    this array is 9 entries (for x86_64 architecture). However, this code
    needs up to 10 entries (*). Therefore, the array is too small.

    Please refer to further details in the example given below.



2.) code fragment from qemu-kvm-x86.c
    ---------------------------------


    118 #ifdef TARGET_X86_64
    119 #define MSR_COUNT 9
    120 #else
    121 #define MSR_COUNT 5
    122 #endif
     :  ...
    324 void kvm_arch_save_regs(CPUState *env)
    325 {
     :      ...
    329     struct kvm_msr_entry msrs[MSR_COUNT];
     :      ...
    436     /* msrs */                                -.
    437     n = 0;                                     |
    438     msrs[n++].index = MSR_IA32_SYSENTER_CS;    |
    439     msrs[n++].index = MSR_IA32_SYSENTER_ESP;   |
    440     msrs[n++].index = MSR_IA32_SYSENTER_EIP;   |
    441     if (kvm_has_msr_star)                      |
    442         msrs[n++].index = MSR_STAR;            |
    443     msrs[n++].index = MSR_IA32_TSC;            |   (*) up to
    444     if (kvm_has_vm_hsave_pa)                    >  10 entries
    445         msrs[n++].index = MSR_VM_HSAVE_PA;     |   are needed
    446 #ifdef TARGET_X86_64                           |
    447     if (lm_capable_kernel) {                   |
    448         msrs[n++].index = MSR_CSTAR;           |
    449         msrs[n++].index = MSR_KERNELGSBASE;    |
    450         msrs[n++].index = MSR_FMASK;           |
    451         msrs[n++].index = MSR_LSTAR;           |
    452     }                                         .'
    453 #endif
    454     rc = kvm_get_msrs(kvm_context, env->cpu_index, msrs, n);
     :      ...
    465 }



3.) example of problem
    ------------------


some configuration info ...


- installed RPMS

# rpm -q kernel
kernel-2.6.30.8-64.fc11.x86_64

# rpm -q qemu-kvm
qemu-kvm-0.10.6-6.fc11.x86_64


- processor type

# egrep "processor|model name" /proc/cpuinfo
processor        : 0
model name       : AMD Turion(tm) 64 X2 Mobile Technology TL-52
processor        : 1
model name       : AMD Turion(tm) 64 X2 Mobile Technology TL-52


- storage of virtual machine

# qemu-img info /scratch/lib/libvirt/images/rhel53vmfv-base.img
image: /scratch/lib/libvirt/images/rhel53vmfv-base.img
file format: qcow2
virtual size: 8.0G (8589934592 bytes)
disk size: 3.6G
cluster_size: 4096

# qemu-img info /scratch/lib/libvirt/images/rhel53vm1fv.img
image: /scratch/lib/libvirt/images/rhel53vm1fv.img
file format: qcow2
virtual size: 8.0G (8589934592 bytes)
disk size: 44K
cluster_size: 4096
backing file: rhel53vmfv-base.img (actual path: 
/scratch/lib/libvirt/images/rhel53vmfv-base.img)



steps to 'reproduce' ...


- in terminal window 1: Boot virtual machine to single-user mode and
                        switch to monitor by typing 'control-a c'.

# /usr/bin/qemu-kvm -M pc -m 512 -smp 1 -name rhel53vm1fvscratch -uuid 
11982ab0-3df2-4f29-9399-f1ffd7ab1f47 -monitor stdio -pidfile 
/var/run/libvirt/qemu/rhel53vm1fvscratch.pid -localtime -boot c -drive 
file=/scratch/lib/libvirt/images/rhel53vm1fv.img,if=ide,index=0,boot=on 
-drive file=,if=ide,media=cdrom,index=2 -net none -serial stdio -parallel 
none -usb -nographic -k de


- in terminal window 2: Set breakpoint at entry of kvm_get_msrs() routine
                        in 'qemu-kvm' process. Let process continue to run.

# ps -e | grep qemu-kvm
 2967 pts/10   00:00:47 qemu-kvm

# gdb -p 2967
...
(gdb) break kvm_get_msrs
Breakpoint 1 at 0x54df20: file libkvm-x86.c, line 345.

(gdb) continue
Continuing.


- in terminal window 1: Type 'savevm' at '(qemu)' monitor prompt


- in terminal window 2: The 'qemu-kvm' process reaches the breakpoint.
                        Please note that 'n' is equal 10, even though
                        the 'msrs[]' array has only room for 9 entries.


Breakpoint 1, kvm_get_msrs (kvm=0xfc2040, vcpu=0x0, msrs=0x7f1508289d20, 
n=0xa) at libkvm-x86.c:345
345     {

(gdb) bt
#0  kvm_get_msrs (kvm=0xfc2040, vcpu=0x0, msrs=0x7f1508289d20, n=0xa)
    at libkvm-x86.c:345                                           |
#1  0x0000000000544650 in kvm_arch_save_regs (env=0x1000470)      |
    at /usr/src/debug/qemu-kvm-0.10.6/qemu-kvm-x86.c:454          |
...                                                               |
                                                                  |
                                                 -.               |
(gdb) print msrs[0].index                         |               |
$1 = 0x174                  MSR_IA32_SYSENTER_CS  |               |
(gdb) print msrs[1].index                         |               |
$2 = 0x175                  MSR_IA32_SYSENTER_ESP |               |
(gdb) print msrs[2].index                         |               |
$3 = 0x176                  MSR_IA32_SYSENTER_EIP |               |
(gdb) print msrs[3].index                         |               |
$4 = 0xc0000081             MSR_STAR              |               |
(gdb) print msrs[4].index                         |               |
$5 = 0x10                   MSR_IA32_TSC           > 10 entries <-+
(gdb) print msrs[5].index                         |
$6 = 0xc0010117             MSR_VM_HSAVE_PA       |
(gdb) print msrs[6].index                         |
$7 = 0xc0000083             MSR_CSTAR             |
(gdb) print msrs[7].index                         |
$8 = 0xc0000102             MSR_KERNELGSBASE      |
(gdb) print msrs[8].index                         |
$9 = 0xc0000084             MSR_FMASK             |
(gdb) print msrs[9].index                         |
$10 = 0xc0000082            MSR_LSTAR             |
                                                 -'

(gdb) print kvm_has_msr_star <--------------------+
$11 = 0x1                                         |
(gdb) print kvm_has_vm_hsave_pa <-----------------)---+
$12 = 0x1                                         |   |
(gdb) print lm_capable_kernel                     |   |
$13 = 0x1                                         |   |
                                                  |   |
                                                  |   |
                                                  |   |
4.) from the KVM kernel modules                   |   |
    ---------------------------                   |   |
                                                  |   |
                                                  |   |
crash> px num_msrs_to_save                        |   |
num_msrs_to_save = $1 = 0xb                       |   |
                                                  |   |
crash> px msrs_to_save[0]                         |   |
$2 = 0x174                  MSR_IA32_SYSENTER_CS  |   |
crash> px msrs_to_save[1]                         |   |
$3 = 0x175                  MSR_IA32_SYSENTER_ESP |   |
crash> px msrs_to_save[2]                         |   |
$4 = 0x176                  MSR_IA32_SYSENTER_EIP |   |
crash> px msrs_to_save[3]                         |   |
$5 = 0xc0000081             MSR_STAR <------------+   |
crash> px msrs_to_save[4]                             |
$6 = 0xc0000083             MSR_CSTAR                 |
crash> px msrs_to_save[5]                             |
$7 = 0xc0000102             MSR_KERNELGSBASE          |
crash> px msrs_to_save[6]                             |
$8 = 0xc0000084             MSR_FMASK                 |
crash> px msrs_to_save[7]                             |
$9 = 0xc0000082             MSR_LSTAR                 |
crash> px msrs_to_save[8]                             |
$10 = 0x10                  MSR_IA32_TSC              |
crash> px msrs_to_save[9]                             |
$11 = 0x277                 MSR_IA32_CR_PAT           |
crash> px msrs_to_save[10]                            |
$12 = 0xc0010117            MSR_VM_HSAVE_PA <---------+
Comment 2 Eduardo Habkost 2009-10-14 14:29:12 EDT
Fix submitted upstream:
http://marc.info/?l=kvm&m=125554346023751
Comment 6 Mark McLoughlin 2009-10-16 09:45:54 EDT
Thanks, will include this in F-11 and F-12
Comment 7 Mark McLoughlin 2009-10-19 07:19:33 EDT
f12 tag request: https://fedorahosted.org/rel-eng/ticket/2529

pushing an f11 update too
Comment 8 Fedora Update System 2009-10-19 07:29:10 EDT
qemu-0.10.6-8.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/qemu-0.10.6-8.fc11
Comment 9 Fedora Update System 2009-10-20 20:39:52 EDT
qemu-0.10.6-8.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update qemu'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-10591
Comment 10 Fedora Update System 2009-10-27 02:41:04 EDT
qemu-0.10.6-9.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update qemu'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-10754
Comment 11 Fedora Update System 2009-11-04 07:11:48 EST
qemu-0.10.6-9.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.