[PATCH] Re: kvm: unable to handle kernel NULL pointer dereference

Tobias Diedrich
2008-06-29T18:29:01Z

Tobias Diedrich wrote:
> Chris Wright wrote:
> > * Tobias Diedrich (ranma+kernel@tdiedrich.de) wrote:
> > > PM: Creating hibernation image:
> > > PM: Need to copy 126181 pages
> > > PM: Normal pages needed: 126181 + 1024 + 38, available pages: 397721
> > > x86 PAT enabled: cpu 0, old 0x7040600070406, new 0x7010600070106
> > > svm_cpu_init: svm_data is NULL on 0
> >
> > Yeah, this is broken. What happens is:
> >
> > suspend:
> > hardware_disable
> > svm_hardware_disable
> > kfree(svm_data)
> > per_cpu(svm_data) = NULL
> >
> > resume:
> > hardware_enable
> > svm_hardware_enable
> > if(!svm_data) printk("svm_cpu_init: svm_data is NULL on 0")
> >
> > at this point it is broken.
> >
> > Same would happen on an SMP box by simply doing offline/online of a CPU.
> > This is definitely busted, looking into a patch.
>
> Any progress on this?

FWIW, it's still broken on 2.6.26-rc8, but the following patch works
for me (tm):

Index: linux-2.6.26-rc8.forcedwol/arch/x86/kvm/svm.c
===================================================================
--- linux-2.6.26-rc8.forcedwol.orig/arch/x86/kvm/svm.c 2008-06-29 20:04:20.000000000 +0200
+++ linux-2.6.26-rc8.forcedwol/arch/x86/kvm/svm.c 2008-06-29 20:12:15.000000000 +0200
@@ -268,6 +268,30 @@
return 1;
}

+static int svm_cpu_init(int cpu)
+{
+ struct svm_cpu_data *svm_data;
+ int r;
+
+ svm_data = kzalloc(sizeof(struct svm_cpu_data), GFP_KERNEL);
+ if (!svm_data)
+ return -ENOMEM;
+ svm_data->cpu = cpu;
+ svm_data->save_area = alloc_page(GFP_KERNEL);
+ r = -ENOMEM;
+ if (!svm_data->save_area)
+ goto err_1;
+
+ per_cpu(svm_data, cpu) = svm_data;
+
+ return 0;
+
+err_1:
+ kfree(svm_data);
+ return r;
+
+}
+
static void svm_hardware_disable(void *garbage)
{
struct svm_cpu_data *svm_data
@@ -293,11 +317,17 @@
struct desc_ptr gdt_descr;
struct desc_struct *gdt;
int me = raw_smp_processor_id();
+ int ret;

if (!has_svm()) {
- printk(KERN_ERR "svm_cpu_init: err EOPNOTSUPP on %d\n", me);
+ printk(KERN_ERR "svm_hardware_enable: err EOPNOTSUPP on %d\n", me);
+ return;
+ }
+ if ((ret = svm_cpu_init(me)) != 0) {
+ printk(KERN_ERR "svm_hardware_enable: svm_cpu_init failed on %d with status: %d\n", me, ret);
return;
}
+
svm_data = per_cpu(svm_data, me);

if (!svm_data) {
@@ -321,30 +351,6 @@
page_to_pfn(svm_data->save_area) << PAGE_SHIFT);
}

-static int svm_cpu_init(int cpu)
-{
- struct svm_cpu_data *svm_data;
- int r;
-
- svm_data = kzalloc(sizeof(struct svm_cpu_data), GFP_KERNEL);
- if (!svm_data)
- return -ENOMEM;
- svm_data->cpu = cpu;
- svm_data->save_area = alloc_page(GFP_KERNEL);
- r = -ENOMEM;
- if (!svm_data->save_area)
- goto err_1;
-
- per_cpu(svm_data, cpu) = svm_data;
-
- return 0;
-
-err_1:
- kfree(svm_data);
- return r;
-
-}
-
static void set_msr_interception(u32 *msrpm, unsigned msr,
int read, int write)
{