On Sun, Dec 9, 2018 at 11:23 AM Gert Robben <t2@xxxxxxx> wrote:
>
> Op 09-12-18 om 18:28 schreef Dan Williams:
> > On Sun, Dec 9, 2018 at 7:25 AM Andy Shevchenko
> > <andy.shevchenko@xxxxxxxxx> wrote:
> >>
> >> Cc: Dan (Dan, it seems in the file mentioned in the bug the only
> >> change between affected and non-affected verions is yours)
> >>
> >> Thanks for the report!
> >
> > Thanks for forwarding, can you give the attached patch a try to get
> > more information?
> >
> > We should remove the BUG_ON() regardless of the outcome,
> > reserve_memtype() failures should not bring down the system.
>
> Yes, I tried the patch, without the BUG_ON it seems to work fine at
> first glance.
> The relevant dmesgs and config are attached.
>
> Note, I forgot to save the kernel+config I used in the first email (it's
> not my routine).
> So I rebuilt and captured the error again with the same config, see
> dmesg-nodebug.
> Maybe it makes comparing easier.
>
> Thanks for the support!
Thanks, it had everything I needed. Can you give the attached fix a
try? If it looks good I'll submit to the x86 maintainers.
From eb395c35ec7ceffabce1f1ce240cc068aae02314 Mon Sep 17 00:00:00 2001
From: Dan Williams <dan.j.williams@xxxxxxxxx>
Date: Mon, 10 Dec 2018 13:30:07 -0800
Subject: [PATCH] x86/mm: Fix decoy address handling vs 32-bit builds
A decoy address is used by set_mce_nospec() to update the cache
attributes for a page that may contain poison (multi-bit ECC error)
while attempting to minimize the possibility of triggering a speculative
access to that page.
When reserve_memtype() is handling a decoy address it needs to convert
it to its real physical alias. The conversion, AND'ing with
__PHYSICAL_MASK, is broken for a 32-bit physical mask and
reserve_memtype() is passed the last physical page. Gert reports
triggering the:
BUG_ON(start >= end);
...assertion when running a 32-bit non-PAE build on a platform that has
a driver resource at the top of physical memory:
BIOS-e820: [mem 0x00000000fff00000-0x00000000ffffffff] reserved
Given that the decoy address scheme is only targeted at 64-bit builds
and assumes that the top of physical address space is free for use as a
decoy address range, simply bypass address sanitization in the 32-bit
case.
Lastly, there was no need to crash the system when this failure
occurred, and no need to crash future systems if the assumptions of
decoy addresses are violated in the future. Change the BUG_ON() to a
WARN() with an error return.
Reported-by: Gert Robben <t2@xxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx>
Fixes: 510ee090abc3 ("x86/mm/pat: Prepare {reserve, free}_memtype() for...")
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
arch/x86/mm/pat.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
index 08013524fba1..4fe956a63b25 100644
--- a/arch/x86/mm/pat.c
+++ b/arch/x86/mm/pat.c
@@ -519,8 +519,13 @@ static u64 sanitize_phys(u64 address)
* for a "decoy" virtual address (bit 63 clear) passed to
* set_memory_X(). __pa() on a "decoy" address results in a
* physical address with bit 63 set.
+ *
+ * Decoy addresses are not present for 32-bit builds, see
+ * set_mce_nospec().
*/
- return address & __PHYSICAL_MASK;
+ if (IS_ENABLED(CONFIG_X86_64))
+ return address & __PHYSICAL_MASK;
+ return address;
}
/*
@@ -546,7 +551,11 @@ int reserve_memtype(u64 start, u64 end, enum page_cache_mode req_type,
start = sanitize_phys(start);
end = sanitize_phys(end);
- BUG_ON(start >= end); /* end is exclusive */
+ if (start >= end) {
+ WARN(1, "%s failed: [mem %#010Lx-%#010Lx], req %s\n", __func__,
+ start, end - 1, cattr_name(req_type));
+ return -EINVAL;
+ }
if (!pat_enabled()) {
/* This is identical to page table setting without PAT */
--
2.17.2
