This small series contains some improvements for the riscv KASAN code:
- it brings a better readability of the code (patch 1/2)
- it fixes oversight regarding page table population which I uncovered
while working on my sv48 patchset (patch 3)
- it helps to have better performance by using hugepages when possible
(patch 4)
Alexandre Ghiti (4):
riscv: Improve kasan definitions
riscv: Use KASAN_SHADOW_INIT define for kasan memory initialization
riscv: Improve kasan population function
riscv: Improve kasan population by using hugepages when possible
arch/riscv/include/asm/kasan.h | 22 +++++-
arch/riscv/mm/kasan_init.c | 119 ++++++++++++++++++++++++---------
2 files changed, 108 insertions(+), 33 deletions(-)
--
2.20.1
There is no functional change here, only improvement in code readability
by adding comments to explain where the kasan constants come from and by
replacing hardcoded numerical constant by the corresponding define.
Note that the comments come from arm64.
Signed-off-by: Alexandre Ghiti <[email protected]>
---
arch/riscv/include/asm/kasan.h | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/include/asm/kasan.h b/arch/riscv/include/asm/kasan.h
index b04028c6218c..a2b3d9cdbc86 100644
--- a/arch/riscv/include/asm/kasan.h
+++ b/arch/riscv/include/asm/kasan.h
@@ -8,12 +8,28 @@
#ifdef CONFIG_KASAN
+/*
+ * The following comment was copied from arm64:
+ * KASAN_SHADOW_START: beginning of the kernel virtual addresses.
+ * KASAN_SHADOW_END: KASAN_SHADOW_START + 1/N of kernel virtual addresses,
+ * where N = (1 << KASAN_SHADOW_SCALE_SHIFT).
+ *
+ * KASAN_SHADOW_OFFSET:
+ * This value is used to map an address to the corresponding shadow
+ * address by the following formula:
+ * shadow_addr = (address >> KASAN_SHADOW_SCALE_SHIFT) + KASAN_SHADOW_OFFSET
+ *
+ * (1 << (64 - KASAN_SHADOW_SCALE_SHIFT)) shadow addresses that lie in range
+ * [KASAN_SHADOW_OFFSET, KASAN_SHADOW_END) cover all 64-bits of virtual
+ * addresses. So KASAN_SHADOW_OFFSET should satisfy the following equation:
+ * KASAN_SHADOW_OFFSET = KASAN_SHADOW_END -
+ * (1ULL << (64 - KASAN_SHADOW_SCALE_SHIFT))
+ */
#define KASAN_SHADOW_SCALE_SHIFT 3
-#define KASAN_SHADOW_SIZE (UL(1) << (38 - KASAN_SHADOW_SCALE_SHIFT))
-#define KASAN_SHADOW_START KERN_VIRT_START /* 2^64 - 2^38 */
+#define KASAN_SHADOW_SIZE (UL(1) << ((CONFIG_VA_BITS - 1) - KASAN_SHADOW_SCALE_SHIFT))
+#define KASAN_SHADOW_START KERN_VIRT_START
#define KASAN_SHADOW_END (KASAN_SHADOW_START + KASAN_SHADOW_SIZE)
-
#define KASAN_SHADOW_OFFSET (KASAN_SHADOW_END - (1ULL << \
(64 - KASAN_SHADOW_SCALE_SHIFT)))
--
2.20.1
Current population code populates a whole page table without taking care
of what could have been already allocated and without taking into account
possible index in page table, assuming the virtual address to map is always
aligned on the page table size, which, for example, won't be the case when
the kernel will get pushed to the end of the address space.
Address those problems by rewriting the kasan population function,
splitting it into subfunctions for each different page table level.
Signed-off-by: Alexandre Ghiti <[email protected]>
---
arch/riscv/mm/kasan_init.c | 91 ++++++++++++++++++++++++++------------
1 file changed, 63 insertions(+), 28 deletions(-)
diff --git a/arch/riscv/mm/kasan_init.c b/arch/riscv/mm/kasan_init.c
index 7bbe09416a2e..b7d4d9abd144 100644
--- a/arch/riscv/mm/kasan_init.c
+++ b/arch/riscv/mm/kasan_init.c
@@ -47,37 +47,72 @@ asmlinkage void __init kasan_early_init(void)
local_flush_tlb_all();
}
-static void __init populate(void *start, void *end)
+static void kasan_populate_pte(pmd_t *pmd, unsigned long vaddr, unsigned long end)
+{
+ phys_addr_t phys_addr;
+ pte_t *ptep, *base_pte;
+
+ if (pmd_none(*pmd))
+ base_pte = memblock_alloc(PTRS_PER_PTE * sizeof(pte_t), PAGE_SIZE);
+ else
+ base_pte = (pte_t *)pmd_page_vaddr(*pmd);
+
+ ptep = base_pte + pte_index(vaddr);
+
+ do {
+ if (pte_none(*ptep)) {
+ phys_addr = memblock_phys_alloc(PAGE_SIZE, PAGE_SIZE);
+ set_pte(ptep, pfn_pte(PFN_DOWN(phys_addr), PAGE_KERNEL));
+ }
+ } while (ptep++, vaddr += PAGE_SIZE, vaddr != end);
+
+ set_pmd(pmd, pfn_pmd(PFN_DOWN(__pa(base_pte)), PAGE_TABLE));
+}
+
+static void kasan_populate_pmd(pgd_t *pgd, unsigned long vaddr, unsigned long end)
+{
+ phys_addr_t phys_addr;
+ pmd_t *pmdp, *base_pmd;
+ unsigned long next;
+
+ base_pmd = (pmd_t *)pgd_page_vaddr(*pgd);
+ if (base_pmd == lm_alias(kasan_early_shadow_pmd))
+ base_pmd = memblock_alloc(PTRS_PER_PMD * sizeof(pmd_t), PAGE_SIZE);
+
+ pmdp = base_pmd + pmd_index(vaddr);
+
+ do {
+ next = pmd_addr_end(vaddr, end);
+ kasan_populate_pte(pmdp, vaddr, next);
+ } while (pmdp++, vaddr = next, vaddr != end);
+
+ /*
+ * Wait for the whole PGD to be populated before setting the PGD in
+ * the page table, otherwise, if we did set the PGD before populating
+ * it entirely, memblock could allocate a page at a physical address
+ * where KASAN is not populated yet and then we'd get a page fault.
+ */
+ set_pgd(pgd, pfn_pgd(PFN_DOWN(__pa(base_pmd)), PAGE_TABLE));
+}
+
+static void kasan_populate_pgd(unsigned long vaddr, unsigned long end)
+{
+ phys_addr_t phys_addr;
+ pgd_t *pgdp = pgd_offset_k(vaddr);
+ unsigned long next;
+
+ do {
+ next = pgd_addr_end(vaddr, end);
+ kasan_populate_pmd(pgdp, vaddr, next);
+ } while (pgdp++, vaddr = next, vaddr != end);
+}
+
+static void __init kasan_populate(void *start, void *end)
{
- unsigned long i, offset;
unsigned long vaddr = (unsigned long)start & PAGE_MASK;
unsigned long vend = PAGE_ALIGN((unsigned long)end);
- unsigned long n_pages = (vend - vaddr) / PAGE_SIZE;
- unsigned long n_ptes =
- ((n_pages + PTRS_PER_PTE) & -PTRS_PER_PTE) / PTRS_PER_PTE;
- unsigned long n_pmds =
- ((n_ptes + PTRS_PER_PMD) & -PTRS_PER_PMD) / PTRS_PER_PMD;
-
- pte_t *pte =
- memblock_alloc(n_ptes * PTRS_PER_PTE * sizeof(pte_t), PAGE_SIZE);
- pmd_t *pmd =
- memblock_alloc(n_pmds * PTRS_PER_PMD * sizeof(pmd_t), PAGE_SIZE);
- pgd_t *pgd = pgd_offset_k(vaddr);
-
- for (i = 0; i < n_pages; i++) {
- phys_addr_t phys = memblock_phys_alloc(PAGE_SIZE, PAGE_SIZE);
- set_pte(&pte[i], pfn_pte(PHYS_PFN(phys), PAGE_KERNEL));
- }
-
- for (i = 0, offset = 0; i < n_ptes; i++, offset += PTRS_PER_PTE)
- set_pmd(&pmd[i],
- pfn_pmd(PFN_DOWN(__pa(&pte[offset])),
- __pgprot(_PAGE_TABLE)));
- for (i = 0, offset = 0; i < n_pmds; i++, offset += PTRS_PER_PMD)
- set_pgd(&pgd[i],
- pfn_pgd(PFN_DOWN(__pa(&pmd[offset])),
- __pgprot(_PAGE_TABLE)));
+ kasan_populate_pgd(vaddr, vend);
local_flush_tlb_all();
memset(start, KASAN_SHADOW_INIT, end - start);
@@ -99,7 +134,7 @@ void __init kasan_init(void)
if (start >= end)
break;
- populate(kasan_mem_to_shadow(start), kasan_mem_to_shadow(end));
+ kasan_populate(kasan_mem_to_shadow(start), kasan_mem_to_shadow(end));
};
for (i = 0; i < PTRS_PER_PTE; i++)
--
2.20.1
Instead of hardcoding memory initialization to 0, use KASAN_SHADOW_INIT.
Signed-off-by: Alexandre Ghiti <[email protected]>
---
arch/riscv/mm/kasan_init.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/mm/kasan_init.c b/arch/riscv/mm/kasan_init.c
index a8a2ffd9114a..7bbe09416a2e 100644
--- a/arch/riscv/mm/kasan_init.c
+++ b/arch/riscv/mm/kasan_init.c
@@ -80,7 +80,7 @@ static void __init populate(void *start, void *end)
__pgprot(_PAGE_TABLE)));
local_flush_tlb_all();
- memset(start, 0, end - start);
+ memset(start, KASAN_SHADOW_INIT, end - start);
}
void __init kasan_init(void)
@@ -108,6 +108,6 @@ void __init kasan_init(void)
__pgprot(_PAGE_PRESENT | _PAGE_READ |
_PAGE_ACCESSED)));
- memset(kasan_early_shadow_page, 0, PAGE_SIZE);
+ memset(kasan_early_shadow_page, KASAN_SHADOW_INIT, PAGE_SIZE);
init_task.kasan_depth = 0;
}
--
2.20.1
The kasan functions that populates the shadow regions used to allocate them
page by page and did not take advantage of hugepages, so fix this by
trying to allocate hugepages of 1GB and fallback to 2MB hugepages or 4K
pages in case it fails.
This reduces the page table memory consumption and improves TLB usage,
as shown below:
Before this patch:
---[ Kasan shadow start ]---
0xffffffc000000000-0xffffffc400000000 0x00000000818ef000 16G PTE . A . . . . R V
0xffffffc400000000-0xffffffc447fc0000 0x00000002b7f4f000 1179392K PTE D A . . . W R V
0xffffffc480000000-0xffffffc800000000 0x00000000818ef000 14G PTE . A . . . . R V
---[ Kasan shadow end ]---
After this patch:
---[ Kasan shadow start ]---
0xffffffc000000000-0xffffffc400000000 0x00000000818ef000 16G PTE . A . . . . R V
0xffffffc400000000-0xffffffc440000000 0x0000000240000000 1G PGD D A . . . W R V
0xffffffc440000000-0xffffffc447e00000 0x00000002b7e00000 126M PMD D A . . . W R V
0xffffffc447e00000-0xffffffc447fc0000 0x00000002b818f000 1792K PTE D A . . . W R V
0xffffffc480000000-0xffffffc800000000 0x00000000818ef000 14G PTE . A . . . . R V
---[ Kasan shadow end ]---
Signed-off-by: Alexandre Ghiti <[email protected]>
---
arch/riscv/mm/kasan_init.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/arch/riscv/mm/kasan_init.c b/arch/riscv/mm/kasan_init.c
index b7d4d9abd144..2b196f512f07 100644
--- a/arch/riscv/mm/kasan_init.c
+++ b/arch/riscv/mm/kasan_init.c
@@ -83,6 +83,15 @@ static void kasan_populate_pmd(pgd_t *pgd, unsigned long vaddr, unsigned long en
do {
next = pmd_addr_end(vaddr, end);
+
+ if (pmd_none(*pmdp) && IS_ALIGNED(vaddr, PMD_SIZE) && (next - vaddr) >= PMD_SIZE) {
+ phys_addr = memblock_phys_alloc(PMD_SIZE, PMD_SIZE);
+ if (phys_addr) {
+ set_pmd(pmdp, pfn_pmd(PFN_DOWN(phys_addr), PAGE_KERNEL));
+ continue;
+ }
+ }
+
kasan_populate_pte(pmdp, vaddr, next);
} while (pmdp++, vaddr = next, vaddr != end);
@@ -103,6 +112,21 @@ static void kasan_populate_pgd(unsigned long vaddr, unsigned long end)
do {
next = pgd_addr_end(vaddr, end);
+
+ /*
+ * pgdp can't be none since kasan_early_init initialized all KASAN
+ * shadow region with kasan_early_shadow_pmd: if this is stillthe case,
+ * that means we can try to allocate a hugepage as a replacement.
+ */
+ if (pgd_page_vaddr(*pgdp) == (unsigned long)lm_alias(kasan_early_shadow_pmd) &&
+ IS_ALIGNED(vaddr, PGDIR_SIZE) && (next - vaddr) >= PGDIR_SIZE) {
+ phys_addr = memblock_phys_alloc(PGDIR_SIZE, PGDIR_SIZE);
+ if (phys_addr) {
+ set_pgd(pgdp, pfn_pgd(PFN_DOWN(phys_addr), PAGE_KERNEL));
+ continue;
+ }
+ }
+
kasan_populate_pmd(pgdp, vaddr, next);
} while (pgdp++, vaddr = next, vaddr != end);
}
--
2.20.1
Hi,
Le 2/8/21 ? 2:30 PM, Alexandre Ghiti a ?crit?:
> This small series contains some improvements for the riscv KASAN code:
>
> - it brings a better readability of the code (patch 1/2)
> - it fixes oversight regarding page table population which I uncovered
> while working on my sv48 patchset (patch 3)
> - it helps to have better performance by using hugepages when possible
> (patch 4)
>
> Alexandre Ghiti (4):
> riscv: Improve kasan definitions
> riscv: Use KASAN_SHADOW_INIT define for kasan memory initialization
> riscv: Improve kasan population function
> riscv: Improve kasan population by using hugepages when possible
>
> arch/riscv/include/asm/kasan.h | 22 +++++-
> arch/riscv/mm/kasan_init.c | 119 ++++++++++++++++++++++++---------
> 2 files changed, 108 insertions(+), 33 deletions(-)
>
I'm cc-ing linux-arch and linux-mm to get more chance to have reviewers
on this series.
Thanks,
Alex
On Sun, 21 Feb 2021 05:42:08 PST (-0800), [email protected] wrote:
> Hi,
>
> Le 2/8/21 à 2:30 PM, Alexandre Ghiti a écrit :
>> This small series contains some improvements for the riscv KASAN code:
>>
>> - it brings a better readability of the code (patch 1/2)
>> - it fixes oversight regarding page table population which I uncovered
>> while working on my sv48 patchset (patch 3)
>> - it helps to have better performance by using hugepages when possible
>> (patch 4)
>>
>> Alexandre Ghiti (4):
>> riscv: Improve kasan definitions
>> riscv: Use KASAN_SHADOW_INIT define for kasan memory initialization
>> riscv: Improve kasan population function
>> riscv: Improve kasan population by using hugepages when possible
>>
>> arch/riscv/include/asm/kasan.h | 22 +++++-
>> arch/riscv/mm/kasan_init.c | 119 ++++++++++++++++++++++++---------
>> 2 files changed, 108 insertions(+), 33 deletions(-)
>>
>
> I'm cc-ing linux-arch and linux-mm to get more chance to have reviewers
> on this series.
Sorry about that, I must have missed these. For some reason I remember having
read the big one, so I'm not sure what happened. They're on for-next.
Thanks!