[PATCH -tip] kmemcheck: fix crash in PnP BIOS calls

[PATCH -tip] kmemcheck: fix crash in PnP BIOS calls

[Vegard Nossum]

>From 251667c0cd0bc9d3d3e32ae137f49e51c0bb062c Mon Sep 17 00:00:00 2001
From: Vegard Nossum <vegard.nossum@gmail.com>
Date: Sun, 5 Oct 2008 15:46:30 +0200
Subject: [PATCH] kmemcheck: fix crash in PnP BIOS calls

Ingo Molnar reported this crash:
> PnPBIOS: Scanning system for PnP BIOS support...
> PnPBIOS: Found PnP BIOS installation structure at 0xc00fc550
> PnPBIOS: PnP BIOS version 1.0, entry 0xf0000:0xc580, dseg 0xf0000
> BUG: unable to handle kernel paging request at 0000c6ef

It turns out that BIOS calls are made with a different code segment. So
when kmemcheck tries to dereference the EIP/RIP (using the kernel data
segment register), we get the unhandled page fault.

I think we can solve this by verifying (in the page fault handler) that
the faulting code is using the kernel CS.

Signed-off-by: Vegard Nossum <vegard.nossum@gmail.com>
---
 arch/x86/mm/kmemcheck/kmemcheck.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/x86/mm/kmemcheck/kmemcheck.c b/arch/x86/mm/kmemcheck/kmemcheck.c
index d649aa7..bd739a4 100644
--- a/arch/x86/mm/kmemcheck/kmemcheck.c
+++ b/arch/x86/mm/kmemcheck/kmemcheck.c
@@ -666,6 +666,17 @@ bool kmemcheck_fault(struct pt_regs *regs, unsigned long address,
 	pte_t *pte;
 	unsigned int level;
 
+	/*
+	 * XXX: Is it safe to assume that memory accesses from virtual 86
+	 * mode or non-kernel code segments will _never_ access kernel
+	 * memory (e.g. tracked pages)? For now, we need this to avoid
+	 * invoking kmemcheck for PnP BIOS calls.
+	 */
+	if (regs->flags & X86_VM_MASK)
+		return false;
+	if (regs->cs != __KERNEL_CS)
+		return false;
+
 	pte = lookup_address(address, &level);
 	if (!pte)
 		return false;
-- 
1.5.5.1

Re: [PATCH -tip] kmemcheck: fix crash in PnP BIOS calls

[Alan Cox]

> It turns out that BIOS calls are made with a different code segment. So
> when kmemcheck tries to dereference the EIP/RIP (using the kernel data
> segment register), we get the unhandled page fault.
> 
> I think we can solve this by verifying (in the page fault handler) that
> the faulting code is using the kernel CS.

That isn't an entirely safe assumption and some services such as BIOS32
are 32bit. Would it be better wrap BIOS calls with a kmemcheck wrapper
which changes the way kmemcheck works/dumps stuff and also gives you a
hook after BIOS calls to do stuff like corruption scans or change
detection on kernel pages ?

Alan

Re: [PATCH -tip] kmemcheck: fix crash in PnP BIOS calls

[Vegard Nossum]

Hi,

On Sun, Oct 5, 2008 at 8:15 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> It turns out that BIOS calls are made with a different code segment. So
>> when kmemcheck tries to dereference the EIP/RIP (using the kernel data
>> segment register), we get the unhandled page fault.
>>
>> I think we can solve this by verifying (in the page fault handler) that
>> the faulting code is using the kernel CS.
>
> That isn't an entirely safe assumption and some services such as BIOS32
> are 32bit. Would it be better wrap BIOS calls with a kmemcheck wrapper
> which changes the way kmemcheck works/dumps stuff and also gives you a
> hook after BIOS calls to do stuff like corruption scans or change
> detection on kernel pages ?

Thanks for the heads-up.

I think BIOS32 in particular is safe. I was looking at for instance
check_pcibios() in arch/x86/pci/pcbios.c. It doesn't change any
segment registers, so it means that whatever the actual code address
is, we should be able to dereference it (i.e. inspect the BIOS code)
without taking additional page faults.

And this is really all we need; BIOS code running with the kernel
segments can even access kernel memory and kmemcheck will be able to
catch those reads/writes just fine. I'm not sure if the BIOS code can
change segments (it would need to switch LDT/GDTs, I think), and even
if it did, I find it unlikely that it would directly access kernel
memory using these new segment registers. Do you happen to know of any
such cases?


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036

Re: [PATCH -tip] kmemcheck: fix crash in PnP BIOS calls

[Alan Cox]

> catch those reads/writes just fine. I'm not sure if the BIOS code can
> change segments (it would need to switch LDT/GDTs, I think), and even

It can do so if it wants as far as I can tell

> if it did, I find it unlikely that it would directly access kernel
> memory using these new segment registers. Do you happen to know of any
> such cases?

No. The code either seems to be very simple or just be an SMM trap in the
BIOSes I've seen