Re: [git pull] x86 page fault checker

From: Ingo Molnar
Date: Fri Feb 20 2009 - 10:51:01 EST



* Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> Ingo,
>
> This is not an urgent fix, but I based it on your urgent
> branch. The patch keeps the page fault handler from entering
> an infinite loop if the PMD does not match the PTE, and the
> PTE has the correct permissions but the PMD does not.
>
> With your latest change, this should not happen again. But if
> there's some other code out there that does have this bug, or
> if some future change creates it (never know with all the
> changes in virtualization) Perhaps it is still a good idea to
> have this check.
>
> This is not a fast path, and it should not hurt to have this
> level of paranoia.
>
> -- Steve
>
> Please pull the latest tip/x86/urgent tree, which can be found at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
> tip/x86/urgent
>
>
> Steven Rostedt (1):
> x86: check PMD in spurious_fault handler
>
> ----
> arch/x86/mm/fault.c | 13 ++++++++++++-
> 1 files changed, 12 insertions(+), 1 deletions(-)
> ---------------------------
> commit 8ef2333f1bdcc4a43cb37b1b5d8febf8e3d8cdc7
> Author: Steven Rostedt <srostedt@xxxxxxxxxx>
> Date: Thu Feb 19 11:46:36 2009 -0500
>
> x86: check PMD in spurious_fault handler
>
> Impact: fix to prevent hard lockup on bad PMD permissions
>
> If the PMD does not have the correct permissions for a page access,
> but the PTE does, the spurious fault handler will mistake the fault
> as a lazy TLB transaction. This will result in an infinite loop of:
>
> fault -> spurious_fault check (pass) -> return to code -> fault
>
> This patch adds a check and a warn on if the PTE passes the permissions
> but the PMD does not.
>
> Signed-off-by: Steven Rostedt <srostedt@xxxxxxxxxx>
>
> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> index c76ef1d..7b579a6 100644
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -455,6 +455,7 @@ static int spurious_fault(unsigned long address,
> pud_t *pud;
> pmd_t *pmd;
> pte_t *pte;
> + int ret;
>
> /* Reserved-bit violation or user access to kernel space? */
> if (error_code & (PF_USER | PF_RSVD))
> @@ -482,7 +483,17 @@ static int spurious_fault(unsigned long address,
> if (!pte_present(*pte))
> return 0;
>
> - return spurious_fault_check(error_code, pte);
> + ret = spurious_fault_check(error_code, pte);
> + if (!ret)
> + return 0;
> +
> + /*
> + * Make sure we have permissions in PMD
> + * If not, then there's a bug in the page tables.
> + */
> + ret = spurious_fault_check(error_code, (pte_t *) pmd);
> + WARN_ON(!ret);
> + return ret;
> }

i guess we could do this - but i'd rather have it as a
WARN_ONCE(), with some text - so that if it ever triggers it's
one surgical message.

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/