Re: [PATCH] [4/8] CPA: Fix set_memory_x for ioremap




* Andi Kleen <ak@xxxxxxx> wrote:

EFI currently calls set_memory_x() on potentially ioremapped
addresses.

This is problematic for several reasons:

- The cpa code internally calls __pa on it which does not work for
remapped addresses and will give some random result.

Wrong. We do call __pa() on vmalloc ranges (which is a known
uncleanliness that we intend to fix), but contrary to your claim the
result is not "random result". On 64-bit it's guaranteed to have a value
above ~66 TB on 64-bit and hence fails all the filters later on so it
has zero practical relevance at the moment. On 32-bit we transform it
down to somewhere around 1GB - where we check it against the BIOS range
filters - which again cannot trigger. But I do agree that it's unclean
and needs fixing up.

Detailed analysis on 64-bit: we call __pa() here:

static int change_page_attr_addr(struct cpa_data *cpa)
...
unsigned long phys_addr = __pa(address);

which for vmalloc area virtual addresses will indeed yield some really
high (and invalid) physical address. That address will never trigger
this check:

if (within(address, HIGH_MAP_START, HIGH_MAP_END))
address = (unsigned long) __va(phys_addr);

or this check:

if (within(phys_addr, 0, KERNEL_TEXT_SIZE)) {

so we'll never actuall _use_ that phys_addr.

- cpa will try to change all potential aliases (like the kernel
mapping on x86-64), but that is not needed for NX because the caller
does only needs its specific virtual address executable. There is no
requirement in the x86 architecture for nx bits to be coherent between
mapping aliases. Also with the previous problem of __pa returning a
wrong address it would likely try to change some random other page if
you're unlucky and the random result would match the kernel text
range.

wrong. That "random other page" is guaranteed to be above 66 TB
physical.

Anyway, i agree that it's ugly and unintuitive and it's on our clean-up
list. But your patch is not a good cleanup because it just hides the
underlying weakness.

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/