VMI paravirt-ops bugfix for 2.6.21

Critical bugfix; when using software RAID, potentially USB or AIO in highmem configurations, drivers are allowed to use kmap_atomic from interrupt context. This is incompatible with the current implementation of lazy MMU mode, and means the kmap will silently fail, causing either memory corruption or kernel panics. This bug is only visible with >970 megs of RAM and extreme memory pressure, but nontheless extremely serious.

The fix is to disable interrupts on the CPU when entering a lazy MMU state; this is totally safe, as preemption is already disabled, and lazy update state can neither be nested nor overlapping. Thus per-cpu variables to track the state and flags can be used to disable interrupts during this critical region.

Signed-off-by Zachary Amsden <[email protected]>

diff -r be8c61492e28 arch/i386/kernel/vmi.c

arch/i386/kernel/vmi.c +7 -0

@@ -69,6 +69,7 @@ struct {
 	void (*flush_tlb)(int);
 	void (*set_initial_ap_state)(int, int);
 	void (*halt)(void);
+ 	void (*set_lazy_mode)(int mode);
 } vmi_ops;
 
 /* XXX move this to alternative.h */

@@ -574,6 +575,31 @@ vmi_startup_ipi_hook(int phys_apicid, un
 }
 #endif
 
+static void vmi_set_lazy_mode(int new_mode)
+{
+	static DEFINE_PER_CPU(int, mode);
+	static DEFINE_PER_CPU(unsigned long, flags);
+	int cpu = smp_processor_id();
+

arch/i386/kernel/vmi.c vmi_startup_ipi_hook(int phys_apicid, un · line 582

Quoted 6 lines · On Sat, 31 Mar 2007 00:45:59 -0800 Zachary Amsden <[email protected]> wrote:

arch/i386/kernel/vmi.c +5 -0

+static void vmi_set_lazy_mode(int new_mode)
+{
+	static DEFINE_PER_CPU(int, mode);
+	static DEFINE_PER_CPU(unsigned long, flags);
+	int cpu = smp_processor_id();

That will cause upset if CONFIG_PREEMPT=y?

The comment only talks about disabling interrupts for lazy_mmu, but this seems to do it for lazy_cpu as well. Is that OK? What happens if someone wants to change interrupt states under lazy_cpu; I can't think of an inherent reason why that wouldn't be allowed (though I don't think it happens now).

This kind of logic is a bit clunky anyway; would it be better to simply have separate enable/disable functions? Or at least separate functions per mode?

So lazy MMU mode is vulnerable to interrupts coming in and issuing kmap_atomic, which does not work when under lazy MMU mode. The window for this is small, but it means highmem kernels, especially with heavy network, USB, or AIO workloads are vulnerable to getting invariably fatal pagefaults in interrupt handlers. For now, the best fix is to simply disable and re-enable interrupts when entering and exiting lazy mode (which, btw, is already guaranteed to have preempt disabled). For the future, a better fix is to simply exit lazy mode when issuing kmap_atomic, but I do not want to touch any generic code now for 2.6.21.

I think I would prefer you touch the generic code. This new hook is ugly.

And the lazy mode is currently only used by VMI anyways, isn't it? So you shouldn't impact anybody else