Commit 488fd995 authored by Arjan van de Ven's avatar Arjan van de Ven Committed by Ingo Molnar

x86: fix pageattr-selftest

In Ingo's testing, he found a bug in the CPA selftest code. What would
happen is that the test would call change_page_attr_addr on a range of
memory, part of which was read only, part of which was writable. The
only thing the test wanted to change was the global bit...

What actually happened was that the selftest would take the permissions
of the first page, and then the change_page_attr_addr call would then
set the permissions of the entire range to this first page. In the
rodata section case, this resulted in pages after the .rodata becoming
read only... which made the kernel rather unhappy in many interesting
ways.

This is just another example of how dangerous the cpa API is (was); this
patch changes the test to use the incremental clear/set APIs
instead, and it changes the clear/set implementation to work on a 1 page
at a time basis.
Signed-off-by: default avatarArjan van de Ven <arjan@linux.intel.com>
Signed-off-by: default avatarIngo Molnar <mingo@elte.hu>
Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
parent 5398f985
...@@ -162,8 +162,8 @@ static __init int exercise_pageattr(void) ...@@ -162,8 +162,8 @@ static __init int exercise_pageattr(void)
continue; continue;
} }
err = change_page_attr_addr(addr[i], len[i], err = change_page_attr_clear(addr[i], len[i],
pte_pgprot(pte_clrhuge(pte_clrglobal(pte0)))); __pgprot(_PAGE_GLOBAL));
if (err < 0) { if (err < 0) {
printk(KERN_ERR "CPA %d failed %d\n", i, err); printk(KERN_ERR "CPA %d failed %d\n", i, err);
failed++; failed++;
...@@ -197,8 +197,8 @@ static __init int exercise_pageattr(void) ...@@ -197,8 +197,8 @@ static __init int exercise_pageattr(void)
failed++; failed++;
continue; continue;
} }
err = change_page_attr_addr(addr[i], len[i], err = change_page_attr_set(addr[i], len[i],
pte_pgprot(pte_mkglobal(*pte))); __pgprot(_PAGE_GLOBAL));
if (err < 0) { if (err < 0) {
printk(KERN_ERR "CPA reverting failed: %d\n", err); printk(KERN_ERR "CPA reverting failed: %d\n", err);
failed++; failed++;
......
...@@ -228,7 +228,6 @@ __change_page_attr(unsigned long address, unsigned long pfn, pgprot_t prot) ...@@ -228,7 +228,6 @@ __change_page_attr(unsigned long address, unsigned long pfn, pgprot_t prot)
/** /**
* change_page_attr_addr - Change page table attributes in linear mapping * change_page_attr_addr - Change page table attributes in linear mapping
* @address: Virtual address in linear mapping. * @address: Virtual address in linear mapping.
* @numpages: Number of pages to change
* @prot: New page table attribute (PAGE_*) * @prot: New page table attribute (PAGE_*)
* *
* Change page attributes of a page in the direct mapping. This is a variant * Change page attributes of a page in the direct mapping. This is a variant
...@@ -240,10 +239,10 @@ __change_page_attr(unsigned long address, unsigned long pfn, pgprot_t prot) ...@@ -240,10 +239,10 @@ __change_page_attr(unsigned long address, unsigned long pfn, pgprot_t prot)
* Modules and drivers should use the set_memory_* APIs instead. * Modules and drivers should use the set_memory_* APIs instead.
*/ */
static int change_page_attr_addr(unsigned long address, int numpages, static int change_page_attr_addr(unsigned long address, pgprot_t prot)
pgprot_t prot)
{ {
int err = 0, kernel_map = 0, i; int err = 0, kernel_map = 0;
unsigned long pfn = __pa(address) >> PAGE_SHIFT;
#ifdef CONFIG_X86_64 #ifdef CONFIG_X86_64
if (address >= __START_KERNEL_map && if (address >= __START_KERNEL_map &&
...@@ -254,30 +253,27 @@ static int change_page_attr_addr(unsigned long address, int numpages, ...@@ -254,30 +253,27 @@ static int change_page_attr_addr(unsigned long address, int numpages,
} }
#endif #endif
for (i = 0; i < numpages; i++, address += PAGE_SIZE) { if (!kernel_map || pte_present(pfn_pte(0, prot))) {
unsigned long pfn = __pa(address) >> PAGE_SHIFT; err = __change_page_attr(address, pfn, prot);
if (err)
return err;
}
if (!kernel_map || pte_present(pfn_pte(0, prot))) {
err = __change_page_attr(address, pfn, prot);
if (err)
break;
}
#ifdef CONFIG_X86_64 #ifdef CONFIG_X86_64
/* /*
* Handle kernel mapping too which aliases part of * Handle kernel mapping too which aliases part of
* lowmem: * lowmem:
*/ */
if (__pa(address) < KERNEL_TEXT_SIZE) { if (__pa(address) < KERNEL_TEXT_SIZE) {
unsigned long addr2; unsigned long addr2;
pgprot_t prot2; pgprot_t prot2;
addr2 = __START_KERNEL_map + __pa(address); addr2 = __START_KERNEL_map + __pa(address);
/* Make sure the kernel mappings stay executable */ /* Make sure the kernel mappings stay executable */
prot2 = pte_pgprot(pte_mkexec(pfn_pte(0, prot))); prot2 = pte_pgprot(pte_mkexec(pfn_pte(0, prot)));
err = __change_page_attr(addr2, pfn, prot2); err = __change_page_attr(addr2, pfn, prot2);
}
#endif
} }
#endif
return err; return err;
} }
...@@ -307,16 +303,24 @@ static int change_page_attr_set(unsigned long addr, int numpages, ...@@ -307,16 +303,24 @@ static int change_page_attr_set(unsigned long addr, int numpages,
pgprot_t current_prot; pgprot_t current_prot;
int level; int level;
pte_t *pte; pte_t *pte;
int i, ret;
pte = lookup_address(addr, &level); for (i = 0; i < numpages ; i++) {
if (pte)
current_prot = pte_pgprot(*pte);
else
pgprot_val(current_prot) = 0;
pgprot_val(prot) = pgprot_val(current_prot) | pgprot_val(prot); pte = lookup_address(addr, &level);
if (pte)
current_prot = pte_pgprot(*pte);
else
pgprot_val(current_prot) = 0;
return change_page_attr_addr(addr, numpages, prot); pgprot_val(prot) = pgprot_val(current_prot) | pgprot_val(prot);
ret = change_page_attr_addr(addr, prot);
if (ret)
return ret;
addr += PAGE_SIZE;
}
return 0;
} }
/** /**
...@@ -344,16 +348,24 @@ static int change_page_attr_clear(unsigned long addr, int numpages, ...@@ -344,16 +348,24 @@ static int change_page_attr_clear(unsigned long addr, int numpages,
pgprot_t current_prot; pgprot_t current_prot;
int level; int level;
pte_t *pte; pte_t *pte;
int i, ret;
pte = lookup_address(addr, &level);
if (pte) for (i = 0; i < numpages; i++) {
current_prot = pte_pgprot(*pte); pte = lookup_address(addr, &level);
else if (pte)
pgprot_val(current_prot) = 0; current_prot = pte_pgprot(*pte);
else
pgprot_val(prot) = pgprot_val(current_prot) & ~pgprot_val(prot); pgprot_val(current_prot) = 0;
return change_page_attr_addr(addr, numpages, prot); pgprot_val(prot) =
pgprot_val(current_prot) & ~pgprot_val(prot);
ret = change_page_attr_addr(addr, prot);
if (ret)
return ret;
addr += PAGE_SIZE;
}
return 0;
} }
int set_memory_uc(unsigned long addr, int numpages) int set_memory_uc(unsigned long addr, int numpages)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment