Re: [PATCH RFC] x86: check for and defend against BIOS memory corruption

Hugh Dickins
2008-08-29T13:21:55Z

[-- Attachment #1: Type: TEXT/PLAIN, Size: 7337 bytes --]

On Fri, 29 Aug 2008, Alan Jenkins wrote:
> Rafał Miłecki wrote:
> >
> > I tried your patch anyway (after applying Jeremy's patch of course)
> > and it doesn't seem to work. The only output is:
> > scanning 2 areas for BIOS corruption
> > after using s2ram. I do not get any
> > Corrupted low memory at*
> >
> It seemed to work for me. Did you remember to plug HDMI to trigger the
> corruption before you used s2ram?

I hope that's what got missed.

Here's my version of Jeremy's patch, that I've now tested on my machines,
as x86_32 and as x86_64. It addresses none of the points Alan Cox made,
and it stays silent for me, even after suspend+resume, unless I actually
introduce corruption myself. Omits Jeremy's check in fault.c, but does
a check every minute, so should soon detect Rafał's HDMI corruption
without any need to suspend+resume.

Hugh

--- 2.6.27-rc5/Documentation/kernel-parameters.txt 2008-08-29 01:02:34.000000000 +0100
+++ linux/Documentation/kernel-parameters.txt 2008-08-29 11:17:16.000000000 +0100
@@ -360,6 +360,11 @@ and is between 256 and 4096 characters.
Format: <io>,<irq>,<mode>
See header of drivers/net/hamradio/baycom_ser_hdx.c.

+ bios_corruption_check=0/1 [X86]
+ Some BIOSes seem to corrupt the first 64k of memory
+ when doing things like suspend/resume. Setting this
+ option will scan the memory looking for corruption.
+
boot_delay= Milliseconds to delay each printk during boot.
Values larger than 10 seconds (10000) are changed to
no delay (0).
--- 2.6.27-rc5/arch/x86/Kconfig 2008-08-29 01:02:35.000000000 +0100
+++ linux/arch/x86/Kconfig 2008-08-29 11:17:16.000000000 +0100
@@ -201,6 +201,9 @@ config X86_TRAMPOLINE
depends on X86_SMP || (X86_VOYAGER && SMP) || (64BIT && ACPI_SLEEP)
default y

+config X86_CHECK_BIOS_CORRUPTION
+ def_bool y
+
config KTIME_SCALAR
def_bool X86_32
source "init/Kconfig"
--- 2.6.27-rc5/arch/x86/kernel/setup.c 2008-08-29 01:02:35.000000000 +0100
+++ linux/arch/x86/kernel/setup.c 2008-08-29 13:50:19.000000000 +0100
@@ -579,6 +579,106 @@ static struct x86_quirks default_x86_qui
struct x86_quirks *x86_quirks __initdata = &default_x86_quirks;

/*
+ * Some BIOSes seem to corrupt the low 64k of memory during events
+ * like suspend/resume and unplugging an HDMI cable. Reserve all
+ * remaining free memory in that area and fill it with a distinct
+ * pattern.
+ */
+#ifdef CONFIG_X86_CHECK_BIOS_CORRUPTION
+#define MAX_SCAN_AREAS 8
+static struct e820entry scan_areas[MAX_SCAN_AREAS];
+static int num_scan_areas;
+
+static void __init setup_bios_corruption_check(void)
+{
+ u64 addr = PAGE_SIZE; /* assume first page is reserved anyway */
+
+ while (addr < 0x10000 && num_scan_areas < MAX_SCAN_AREAS) {
+ u64 size;
+ addr = find_e820_area_size(addr, &size, PAGE_SIZE);
+
+ if (addr == 0)
+ break;
+
+ if ((addr + size) > 0x10000)
+ size = 0x10000 - addr;
+
+ if (size == 0)
+ break;
+
+ e820_update_range(addr, size, E820_RAM, E820_RESERVED);
+ scan_areas[num_scan_areas].addr = addr;
+ scan_areas[num_scan_areas].size = size;
+ num_scan_areas++;
+
+ /* Assume we've already mapped this early memory */
+ memset(__va(addr), 0, size);
+
+ addr += size;
+ }
+
+ printk(KERN_INFO "scanning %d areas for BIOS corruption\n",
+ num_scan_areas);
+ update_e820();
+}
+
+static int __read_mostly bios_corruption_check = 1;
+static struct timer_list periodic_check_timer;
+
+void check_for_bios_corruption(void)
+{
+ int i;
+ int corruption = 0;
+
+ if (!bios_corruption_check)
+ return;
+
+ for (i = 0; i < num_scan_areas; i++) {
+ unsigned int *addr = __va(scan_areas[i].addr);
+ unsigned long size = scan_areas[i].size;
+
+ for (; size; addr++, size -= sizeof(unsigned int)) {
+ if (!*addr)
+ continue;
+ printk(KERN_ERR "Corrupted low memory at %p (%lx phys) = %08x\n",
+ addr, __pa(addr), *addr);
+ *addr = 0;
+ corruption = 1;
+ }
+ }
+
+ if (corruption)
+ dump_stack();
+}
+
+static void periodic_check_for_corruption(unsigned long data)
+{
+ check_for_bios_corruption();
+ mod_timer(&periodic_check_timer, jiffies + 60*HZ);
+}
+
+void start_periodic_check_for_corruption(void)
+{
+ if (!bios_corruption_check)
+ return;
+
+ init_timer(&periodic_check_timer);
+ periodic_check_timer.function = &periodic_check_for_corruption;
+ periodic_check_for_corruption(0);
+}
+
+static int set_bios_corruption_check(char *arg)
+{
+ char *end;
+
+ bios_corruption_check = simple_strtol(arg, &end, 10);
+
+ return (*end == 0) ? 0 : -EINVAL;
+}
+early_param("bios_corruption_check", set_bios_corruption_check);
+#endif
+
+/*
* Determine if we were loaded by an EFI loader. If so, then we have also been
* passed the efi memmap, systab, etc., so we should use these data structures
* for initialization. Note, the efi init code path is determined by the
@@ -750,6 +850,10 @@ void __init setup_arch(char **cmdline_p)
high_memory = (void *)__va(max_pfn * PAGE_SIZE - 1) + 1;
#endif

+#ifdef CONFIG_X86_CHECK_BIOS_CORRUPTION
+ setup_bios_corruption_check();
+#endif
+
/* max_pfn_mapped is updated here */
max_low_pfn_mapped = init_memory_mapping(0, max_low_pfn<<PAGE_SHIFT);
max_pfn_mapped = max_low_pfn_mapped;
--- 2.6.27-rc5/arch/x86/mm/init_32.c 2008-07-29 04:24:15.000000000 +0100
+++ linux/arch/x86/mm/init_32.c 2008-08-29 11:52:21.000000000 +0100
@@ -907,6 +907,8 @@ void __init mem_init(void)
int codesize, reservedpages, datasize, initsize;
int tmp;

+ start_periodic_check_for_corruption();
+
#ifdef CONFIG_FLATMEM
BUG_ON(!mem_map);
#endif
--- 2.6.27-rc5/arch/x86/mm/init_64.c 2008-08-29 01:02:35.000000000 +0100
+++ linux/arch/x86/mm/init_64.c 2008-08-29 11:53:09.000000000 +0100
@@ -769,6 +769,8 @@ void __init mem_init(void)
{
long codesize, reservedpages, datasize, initsize;

+ start_periodic_check_for_corruption();
+
pci_iommu_alloc();

/* clear_bss() already clear the empty_zero_page */
--- 2.6.27-rc5/drivers/base/power/main.c 2008-08-29 01:02:35.000000000 +0100
+++ linux/drivers/base/power/main.c 2008-08-29 11:17:16.000000000 +0100
@@ -254,6 +254,7 @@ static char *pm_verb(int event)

static void pm_dev_dbg(struct device *dev, pm_message_t state, char *info)
{
+ check_for_bios_corruption();
dev_dbg(dev, "%s%s%s\n", info, pm_verb(state.event),
((state.event & PM_EVENT_SLEEP) && device_may_wakeup(dev)) ?
", may wakeup" : "");
--- 2.6.27-rc5/include/linux/kernel.h 2008-08-13 04:14:50.000000000 +0100
+++ linux/include/linux/kernel.h 2008-08-29 11:49:23.000000000 +0100
@@ -240,6 +240,22 @@ extern const char *print_tainted(void);
extern void add_taint(unsigned);
extern int root_mountflags;

+#ifdef CONFIG_X86_CHECK_BIOS_CORRUPTION
+/*
+ * This is obviously not a great place for this, but we want to be
+ * able to scatter it around anywhere in the kernel.
+ */
+void check_for_bios_corruption(void);
+void start_periodic_check_for_corruption(void);
+#else
+static inline void check_for_bios_corruption(void)
+{
+}
+static inline void start_periodic_check_for_corruption(void)
+{
+}
+#endif
+
/* Values used for system_state */
extern enum system_states {
SYSTEM_BOOTING,