2009-01-12 20:31:27

by Suresh Siddha

[permalink] [raw]
Subject: [patch] x86: fix PTE corruption issue while mapping RAM using /dev/mem

From: Suresh Siddha <[email protected]>
Subject: x86: fix PTE corruption issue while mapping RAM using /dev/mem

Beschorner Daniel reported:
> hwinfo problem since 2.6.28, showing this in the oops:
> Corrupted page table at address 7fd04de3ec00

PaX Team reported a regression with this commit:

> commit 9542ada803198e6eba29d3289abb39ea82047b92
> Author: Suresh Siddha <[email protected]>
> Date: Wed Sep 24 08:53:33 2008 -0700
>
> x86: track memtype for RAM in page struct

This commit breaks mapping any RAM page through /dev/mem, as the
reserve_memtype() was not initializing the return attribute type and as such
corrupting the PTE entry that was setup with the return attribute type.

Because of this bug, application mapping this RAM page through /dev/mem
will die with "Corrupted page table at address xxxx" message in the kernel
log and also the kernel identity mapping which maps the underlying RAM
page gets converted to UC.

Fix this by initializing the return attribute type before calling
reserve_ram_pages_type()

Reported-by: PaX Team <[email protected]>
Reported-and-tested-by: Beschorner Daniel <[email protected]>
Tested-and-Acked-by: PaX Team <[email protected]>
Signed-off-by: Suresh Siddha <[email protected]>
Signed-off-by: Venkatesh Pallipadi <[email protected]>
Cc: <[email protected]>
---

diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
index 85cbd3c..d52ba1a 100644
--- a/arch/x86/mm/pat.c
+++ b/arch/x86/mm/pat.c
@@ -333,6 +333,9 @@ int reserve_memtype(u64 start, u64 end, unsigned long req_type,
req_type & _PAGE_CACHE_MASK);
}

+ if (new_type)
+ *new_type = actual_type;
+
is_range_ram = pagerange_is_ram(start, end);
if (is_range_ram == 1)
return reserve_ram_pages_type(start, end, req_type, new_type);
@@ -347,9 +350,6 @@ int reserve_memtype(u64 start, u64 end, unsigned long req_type,
new->end = end;
new->type = actual_type;

- if (new_type)
- *new_type = actual_type;
-
spin_lock(&memtype_lock);

if (cached_entry && start >= cached_start)


2009-01-12 23:52:14

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] x86: fix PTE corruption issue while mapping RAM using /dev/mem


* Suresh Siddha <[email protected]> wrote:

> From: Suresh Siddha <[email protected]>
> Subject: x86: fix PTE corruption issue while mapping RAM using /dev/mem
>
> Beschorner Daniel reported:
> > hwinfo problem since 2.6.28, showing this in the oops:
> > Corrupted page table at address 7fd04de3ec00
>
> PaX Team reported a regression with this commit:
>
> > commit 9542ada803198e6eba29d3289abb39ea82047b92
> > Author: Suresh Siddha <[email protected]>
> > Date: Wed Sep 24 08:53:33 2008 -0700
> >
> > x86: track memtype for RAM in page struct
>
> This commit breaks mapping any RAM page through /dev/mem, as the
> reserve_memtype() was not initializing the return attribute type and as such
> corrupting the PTE entry that was setup with the return attribute type.
>
> Because of this bug, application mapping this RAM page through /dev/mem
> will die with "Corrupted page table at address xxxx" message in the kernel
> log and also the kernel identity mapping which maps the underlying RAM
> page gets converted to UC.
>
> Fix this by initializing the return attribute type before calling
> reserve_ram_pages_type()
>
> Reported-by: PaX Team <[email protected]>
> Reported-and-tested-by: Beschorner Daniel <[email protected]>
> Tested-and-Acked-by: PaX Team <[email protected]>
> Signed-off-by: Suresh Siddha <[email protected]>
> Signed-off-by: Venkatesh Pallipadi <[email protected]>
> Cc: <[email protected]>
> ---

applied to tip/x86/urgent, thanks Suresh!

note that the patch caused some conflicts (there were other changes in
pat.c) - i merged it up manually but could you please check the final
commit in tip/master whether it's all OK?

Ingo

2009-01-13 00:32:18

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] x86: fix PTE corruption issue while mapping RAM using /dev/mem


* [email protected] <[email protected]> wrote:

> On 13 Jan 2009 at 0:51, Ingo Molnar wrote:
>
> > applied to tip/x86/urgent, thanks Suresh!
> >
> > note that the patch caused some conflicts (there were other changes in
> > pat.c) - i merged it up manually but could you please check the final
> > commit in tip/master whether it's all OK?
>
> i don't think that it got applied properly:
>
> diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
> index 85cbd3c..ea36ed7 100644
> --- a/arch/x86/mm/pat.c
> +++ b/arch/x86/mm/pat.c
> @@ -347,9 +347,6 @@ int reserve_memtype(u64 start, u64 end, unsigned long req_type,
> new->end = end;
> new->type = actual_type;
>
> - if (new_type)
> - *new_type = actual_type;
> -
> spin_lock(&memtype_lock);
>
> if (cached_entry && start >= cached_start)
> @@ -437,6 +434,9 @@ int free_memtype(u64 start, u64 end)
> if (is_ISA_range(start, end - 1))
> return 0;
>
> + if (new_type)
> + *new_type = actual_type;
> +
> is_range_ram = pagerange_is_ram(start, end);
> if (is_range_ram == 1)
> return free_ram_pages_type(start, end);
>
> free_memtype doesn't even have a new_type argument for starters ;),
> patch must have applied it at the wrong place...

yeah, and Mr. Compiler agrees:

arch/x86/mm/pat.c: In function 'free_memtype':
arch/x86/mm/pat.c:437: error: 'new_type' undeclared (first use in this function)
arch/x86/mm/pat.c:437: error: (Each undeclared identifier is reported only once
arch/x86/mm/pat.c:437: error: for each function it appears in.)
arch/x86/mm/pat.c:438: error: 'actual_type' undeclared (first use in this function)

Suresh, could you please resend against tip/master? Thanks,

Ingo

2009-01-13 00:53:54

by PaX Team

[permalink] [raw]
Subject: Re: [patch] x86: fix PTE corruption issue while mapping RAM using /dev/mem

On 13 Jan 2009 at 0:51, Ingo Molnar wrote:

> applied to tip/x86/urgent, thanks Suresh!
>
> note that the patch caused some conflicts (there were other changes in
> pat.c) - i merged it up manually but could you please check the final
> commit in tip/master whether it's all OK?

i don't think that it got applied properly:

diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
index 85cbd3c..ea36ed7 100644
--- a/arch/x86/mm/pat.c
+++ b/arch/x86/mm/pat.c
@@ -347,9 +347,6 @@ int reserve_memtype(u64 start, u64 end, unsigned long req_type,
new->end = end;
new->type = actual_type;

- if (new_type)
- *new_type = actual_type;
-
spin_lock(&memtype_lock);

if (cached_entry && start >= cached_start)
@@ -437,6 +434,9 @@ int free_memtype(u64 start, u64 end)
if (is_ISA_range(start, end - 1))
return 0;

+ if (new_type)
+ *new_type = actual_type;
+
is_range_ram = pagerange_is_ram(start, end);
if (is_range_ram == 1)
return free_ram_pages_type(start, end);

free_memtype doesn't even have a new_type argument for starters ;), patch
must have applied it at the wrong place...

2009-01-13 18:22:00

by Suresh Siddha

[permalink] [raw]
Subject: Re: [patch] x86: fix PTE corruption issue while mapping RAM using /dev/mem

On Mon, Jan 12, 2009 at 04:31:47PM -0800, Ingo Molnar wrote:
>
> Suresh, could you please resend against tip/master? Thanks,

Ok. Here it is:

---
From: Suresh Siddha <[email protected]>
Subject: x86: fix PTE corruption issue while mapping RAM using /dev/mem

Beschorner Daniel reported:
> hwinfo problem since 2.6.28, showing this in the oops:
> Corrupted page table at address 7fd04de3ec00

PaX Team reported a regression with this commit:

> commit 9542ada803198e6eba29d3289abb39ea82047b92
> Author: Suresh Siddha <[email protected]>
> Date: Wed Sep 24 08:53:33 2008 -0700
>
> x86: track memtype for RAM in page struct

This commit breaks mapping any RAM page through /dev/mem, as the
reserve_memtype() was not initializing the return attribute type and as such
corrupting the PTE entry that was setup with the return attribute type.

Because of this bug, application mapping this RAM page through /dev/mem
will die with "Corrupted page table at address xxxx" message in the kernel
log and also the kernel identity mapping which maps the underlying RAM
page gets converted to UC.

Fix this by initializing the return attribute type before calling
reserve_ram_pages_type()

Reported-by: PaX Team <[email protected]>
Reported-and-tested-by: Beschorner Daniel <[email protected]>
Tested-and-Acked-by: PaX Team <[email protected]>
Signed-off-by: Suresh Siddha <[email protected]>
Signed-off-by: Venkatesh Pallipadi <[email protected]>
Cc: <[email protected]>
---

diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
index ec8cd49..c2a0538 100644
--- a/arch/x86/mm/pat.c
+++ b/arch/x86/mm/pat.c
@@ -333,6 +333,9 @@ int reserve_memtype(u64 start, u64 end, unsigned long req_type,
req_type & _PAGE_CACHE_MASK);
}

+ if (new_type)
+ *new_type = actual_type;
+
/*
* For legacy reasons, some parts of the physical address range in the
* legacy 1MB region is treated as non-RAM (even when listed as RAM in
@@ -356,9 +359,6 @@ int reserve_memtype(u64 start, u64 end, unsigned long req_type,
new->end = end;
new->type = actual_type;

- if (new_type)
- *new_type = actual_type;
-
spin_lock(&memtype_lock);

if (cached_entry && start >= cached_start)

2009-01-21 17:43:29

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] x86: fix PTE corruption issue while mapping RAM using /dev/mem


* Suresh Siddha <[email protected]> wrote:

> On Mon, Jan 12, 2009 at 04:31:47PM -0800, Ingo Molnar wrote:
> >
> > Suresh, could you please resend against tip/master? Thanks,
>
> Ok. Here it is:

applied to tip/x86/urgent, thanks Suresh!

Ingo