* KVM: fix cleanup_srcu_struct use-after-free
@ 2010-01-16 2:00 Marcelo Tosatti
2010-01-17 12:28 ` Avi Kivity
2010-01-19 13:50 ` Jan Kiszka
0 siblings, 2 replies; 4+ messages in thread
From: Marcelo Tosatti @ 2010-01-16 2:00 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
kvm_destroy_vm should free "struct kvm" after cleanup_srcu_struct.
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index e0a591d..c828a39 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -509,8 +509,8 @@ static void kvm_destroy_vm(struct kvm *kvm)
#else
kvm_arch_flush_shadow(kvm);
#endif
- kvm_arch_destroy_vm(kvm);
cleanup_srcu_struct(&kvm->srcu);
+ kvm_arch_destroy_vm(kvm);
hardware_disable_all();
mmdrop(mm);
}
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: KVM: fix cleanup_srcu_struct use-after-free
2010-01-16 2:00 KVM: fix cleanup_srcu_struct use-after-free Marcelo Tosatti
@ 2010-01-17 12:28 ` Avi Kivity
2010-01-19 13:50 ` Jan Kiszka
1 sibling, 0 replies; 4+ messages in thread
From: Avi Kivity @ 2010-01-17 12:28 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: kvm
On 01/16/2010 04:00 AM, Marcelo Tosatti wrote:
> kvm_destroy_vm should free "struct kvm" after cleanup_srcu_struct.
>
> Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index e0a591d..c828a39 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -509,8 +509,8 @@ static void kvm_destroy_vm(struct kvm *kvm)
> #else
> kvm_arch_flush_shadow(kvm);
> #endif
> - kvm_arch_destroy_vm(kvm);
> cleanup_srcu_struct(&kvm->srcu);
> + kvm_arch_destroy_vm(kvm);
> hardware_disable_all();
> mmdrop(mm);
> }
>
Applied, thanks.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: KVM: fix cleanup_srcu_struct use-after-free
2010-01-16 2:00 KVM: fix cleanup_srcu_struct use-after-free Marcelo Tosatti
2010-01-17 12:28 ` Avi Kivity
@ 2010-01-19 13:50 ` Jan Kiszka
2010-01-19 14:45 ` KVM: fix cleanup_srcu_struct on vm destruction Marcelo Tosatti
1 sibling, 1 reply; 4+ messages in thread
From: Jan Kiszka @ 2010-01-19 13:50 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Avi Kivity, kvm
Marcelo Tosatti wrote:
> kvm_destroy_vm should free "struct kvm" after cleanup_srcu_struct.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index e0a591d..c828a39 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -509,8 +509,8 @@ static void kvm_destroy_vm(struct kvm *kvm)
> #else
> kvm_arch_flush_shadow(kvm);
> #endif
> - kvm_arch_destroy_vm(kvm);
> cleanup_srcu_struct(&kvm->srcu);
> + kvm_arch_destroy_vm(kvm);
> hardware_disable_all();
> mmdrop(mm);
> }
This should be responsible for
BUG: unable to handle kernel paging request at ffffffffffffffff
IP: [<ffffffff802533d2>] srcu_read_lock+0x16/0x21
PGD 203067 PUD 204067 PMD 0
Oops: 0000 [1] SMP
last sysfs file: /sys/devices/virtual/backlight/acpi_video0/brightness
CPU 0
Modules linked in: kvm_intel(N) kvm(N) ip6t_LOG xt_pkttype xt_TCPMSS xt_tcpudp ipt_LOG xt_limit iptable_nat nf_nat xt_physdev coretemp(N) sco bridge stp rfcomm bnep l2cap snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device i915 drm af_packet ip6t_REJECT nf_conntrack_ipv6 ip6table_raw xt_NOTRACK ipt_REJECT xt_state iptable_raw iptable_filter ip6table_mangle nf_conntrack_netbios_ns nf_conntrack_ipv4 nf_conntrack ip_tables cpufreq_conservative cpufreq_userspace cpufreq_powersave acpi_cpufreq ip6table_filter ip6_tables x_tables ipv6 microcode fuse ohci_hcd loop arc4 ecb o2scr(N) ath5k(N) mac80211(N) led_class snd_hda_intel pcmcia ath(N) snd_pcm snd_timer snd_page_alloc snd_hwdep yenta_socket sdhci_pci rtc_cmos btusb rsrc_nonstatic iTCO_wdt i2c_i801 sdhci rtc_core cfg80211(N) video ppdev snd iTCO_
vendor_support ohci1394 pcmcia_core fujitsu_laptop output button pcspkr rfkill_backport(N) mmc_core ac battery joydev rtc_lib serio_raw bluetooth i2c_core soundcore parport_pc ieee1394 sky2
parport intel_agp sg sha256_generic aes_x86_64 aes_generic cbc dm_crypt crypto_blkcipher usbhid hid ff_memless uhci_hcd sd_mod crc_t10dif ehci_hcd usbcore dm_snapshot dm_mod edd ext3 mbcache jbd fan ata_piix ahci libata scsi_mod dock thermal processor thermal_sys hwmon [last unloaded: kvm]
Supported: No
Pid: 13994, comm: qemu-system-x86 Tainted: G W 2.6.27.42-0.1-default #1
RIP: 0010:[<ffffffff802533d2>] [<ffffffff802533d2>] srcu_read_lock+0x16/0x21
RSP: 0018:ffff88001e169ca0 EFLAGS: 00010202
RAX: 0000000000000001 RBX: ffff88004eacc040 RCX: 0000000000000000
RDX: ffffffffffffffff RSI: 0000000000000003 RDI: ffff88005b3e8030
RBP: 0000000000000000 R08: 0000000000000200 R09: ffff880001101600
R10: 0000000000000002 R11: ffffffffa0569efe R12: ffff88005b3e8008
R13: ffff88005b3e8010 R14: ffff88005b3e8000 R15: 0000000000000000
FS: 00007f47680bf950(0000) GS:ffffffff80a43080(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: ffffffffffffffff CR3: 000000001c53c000 CR4: 00000000000026e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process qemu-system-x86 (pid: 13994, threadinfo ffff88001e168000, task ffff8800439d61c0)
Stack: ffffffffa05354c4 0000000000000000 ffff88004eacc040 ffff88005b3e8008
ffffffffa05339c6 ffff88004eacc040 ffffffffa0569f7d ffff88005b3e8000
ffffffffa05357b5 ffff88005b3e8000 0000000000000003 ffff8800214e2180
Call Trace:
[<ffffffffa05354c4>] kvm_arch_vcpu_uninit+0x1b/0x48 [kvm]
[<ffffffffa05339c6>] kvm_vcpu_uninit+0x9/0x15 [kvm]
[<ffffffffa0569f7d>] vmx_free_vcpu+0x7f/0x8f [kvm_intel]
[<ffffffffa05357b5>] kvm_arch_destroy_vm+0x78/0x111 [kvm]
[<ffffffffa053315b>] kvm_put_kvm+0xd4/0xfe [kvm]
[<ffffffffa0533951>] kvm_vcpu_release+0x13/0x17 [kvm]
[<ffffffff802b27e8>] __fput+0xa1/0x165
[<ffffffff802afef1>] filp_close+0x5b/0x62
[<ffffffff8023f4b9>] put_files_struct+0x64/0xc2
[<ffffffff80241005>] do_exit+0x226/0x325
[<ffffffff8024116b>] do_group_exit+0x67/0x94
[<ffffffff80249d45>] get_signal_to_deliver+0x2f4/0x313
[<ffffffff8020b980>] do_signal+0x62/0x171
[<ffffffff8020babe>] do_notify_resume+0x2f/0x50
[<ffffffff8020c367>] ptregscall_common+0x67/0xb0
[<ffffffff8020c082>] sysret_signal+0x1a/0x25
[<00007f477f542fdd>] 0x7f477f542fdd
Code: ef e8 6f fc ff ff eb 02 31 db 5e 89 d8 5b 5d 41 5c 41 5d c3 90 8b 07 83 e0 01 48 8b 57 08 65 8b 0c 25 24 00 00 00 89 c9 48 f7 d2 <48> 8b 0c ca 48 63 d0 ff 04 91 c3 48 8b 47 08 48 63 f6 65 8b 14
RIP [<ffffffff802533d2>] srcu_read_lock+0x16/0x21
RSP <ffff88001e169ca0>
CR2: ffffffffffffffff
---[ end trace bf4e203c69d904ec ]---
Ie. we first release the srcu struct in kvm_destroy_vm but then try to
acquire the lock again in kvm_arch_vcpu_uninit.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 4+ messages in thread
* KVM: fix cleanup_srcu_struct on vm destruction
2010-01-19 13:50 ` Jan Kiszka
@ 2010-01-19 14:45 ` Marcelo Tosatti
0 siblings, 0 replies; 4+ messages in thread
From: Marcelo Tosatti @ 2010-01-19 14:45 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Avi Kivity, kvm
cleanup_srcu_struct on VM destruction remains broken:
BUG: unable to handle kernel paging request at ffffffffffffffff
IP: [<ffffffff802533d2>] srcu_read_lock+0x16/0x21
RIP: 0010:[<ffffffff802533d2>] [<ffffffff802533d2>] srcu_read_lock+0x16/0x21
Call Trace:
[<ffffffffa05354c4>] kvm_arch_vcpu_uninit+0x1b/0x48 [kvm]
[<ffffffffa05339c6>] kvm_vcpu_uninit+0x9/0x15 [kvm]
[<ffffffffa0569f7d>] vmx_free_vcpu+0x7f/0x8f [kvm_intel]
[<ffffffffa05357b5>] kvm_arch_destroy_vm+0x78/0x111 [kvm]
[<ffffffffa053315b>] kvm_put_kvm+0xd4/0xfe [kvm]
Move it to kvm_arch_destroy_vm.
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Reported-by: Jan Kiszka <jan.kiszka@siemens.com>
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index e6ac549..0618898 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -1404,6 +1404,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
kfree(kvm->arch.vioapic);
kvm_release_vm_pages(kvm);
kvm_free_physmem(kvm);
+ cleanup_srcu_struct(&kvm->srcu);
free_kvm(kvm);
}
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 2c29116..51aedd7 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -137,6 +137,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
{
kvmppc_free_vcpus(kvm);
kvm_free_physmem(kvm);
+ cleanup_srcu_struct(&kvm->srcu);
kfree(kvm);
}
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 8bedd31..8f09959 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -241,6 +241,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
kvm_free_physmem(kvm);
free_page((unsigned long)(kvm->arch.sca));
debug_unregister(kvm->arch.dbf);
+ cleanup_srcu_struct(&kvm->srcu);
kfree(kvm);
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 47c6e23..e691b35 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5546,6 +5546,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
put_page(kvm->arch.apic_access_page);
if (kvm->arch.ept_identity_pagetable)
put_page(kvm->arch.ept_identity_pagetable);
+ cleanup_srcu_struct(&kvm->srcu);
kfree(kvm->arch.aliases);
kfree(kvm);
}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 2b7cd6c..7c5c873 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -506,7 +506,6 @@ static void kvm_destroy_vm(struct kvm *kvm)
#else
kvm_arch_flush_shadow(kvm);
#endif
- cleanup_srcu_struct(&kvm->srcu);
kvm_arch_destroy_vm(kvm);
hardware_disable_all();
mmdrop(mm);
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-01-19 14:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-16 2:00 KVM: fix cleanup_srcu_struct use-after-free Marcelo Tosatti
2010-01-17 12:28 ` Avi Kivity
2010-01-19 13:50 ` Jan Kiszka
2010-01-19 14:45 ` KVM: fix cleanup_srcu_struct on vm destruction Marcelo Tosatti
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.