* [PATCH RFC v2] riscv: Enable KFENCE for riscv64
@ 2021-05-14 3:44 Liu Shixin
2021-05-14 15:20 ` Marco Elver
0 siblings, 1 reply; 5+ messages in thread
From: Liu Shixin @ 2021-05-14 3:44 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexander Potapenko,
Marco Elver, Dmitry Vyukov
Cc: linux-riscv, linux-kernel, kasan-dev, Liu Shixin
Add architecture specific implementation details for KFENCE and enable
KFENCE for the riscv64 architecture. In particular, this implements the
required interface in <asm/kfence.h>.
KFENCE requires that attributes for pages from its memory pool can
individually be set. Therefore, force the kfence pool to be mapped at
page granularity.
I tested this patch using the testcases in kfence_test.c and all passed.
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
v1->v2: Change kmalloc() to pte_alloc_one_kernel() for allocating pte.
arch/riscv/Kconfig | 1 +
arch/riscv/include/asm/kfence.h | 51 +++++++++++++++++++++++++++++++++
arch/riscv/mm/fault.c | 11 ++++++-
3 files changed, 62 insertions(+), 1 deletion(-)
create mode 100644 arch/riscv/include/asm/kfence.h
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index c426e7d20907..000d8aba1030 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -64,6 +64,7 @@ config RISCV
select HAVE_ARCH_JUMP_LABEL_RELATIVE
select HAVE_ARCH_KASAN if MMU && 64BIT
select HAVE_ARCH_KASAN_VMALLOC if MMU && 64BIT
+ select HAVE_ARCH_KFENCE if MMU && 64BIT
select HAVE_ARCH_KGDB
select HAVE_ARCH_KGDB_QXFER_PKT
select HAVE_ARCH_MMAP_RND_BITS if MMU
diff --git a/arch/riscv/include/asm/kfence.h b/arch/riscv/include/asm/kfence.h
new file mode 100644
index 000000000000..c25d67e0b8ba
--- /dev/null
+++ b/arch/riscv/include/asm/kfence.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _ASM_RISCV_KFENCE_H
+#define _ASM_RISCV_KFENCE_H
+
+#include <linux/kfence.h>
+#include <linux/pfn.h>
+#include <asm-generic/pgalloc.h>
+#include <asm/pgtable.h>
+
+static inline bool arch_kfence_init_pool(void)
+{
+ int i;
+ unsigned long addr;
+ pte_t *pte;
+ pmd_t *pmd;
+
+ for (addr = (unsigned long)__kfence_pool; is_kfence_address((void *)addr);
+ addr += PAGE_SIZE) {
+ pte = virt_to_kpte(addr);
+ pmd = pmd_off_k(addr);
+
+ if (!pmd_leaf(*pmd) && pte_present(*pte))
+ continue;
+
+ pte = pte_alloc_one_kernel(&init_mm);
+ for (i = 0; i < PTRS_PER_PTE; i++)
+ set_pte(pte + i, pfn_pte(PFN_DOWN(__pa((addr & PMD_MASK) + i * PAGE_SIZE)), PAGE_KERNEL));
+
+ set_pmd(pmd, pfn_pmd(PFN_DOWN(__pa(pte)), PAGE_TABLE));
+ flush_tlb_kernel_range(addr, addr + PMD_SIZE);
+ }
+
+ return true;
+}
+
+static inline bool kfence_protect_page(unsigned long addr, bool protect)
+{
+ pte_t *pte = virt_to_kpte(addr);
+
+ if (protect)
+ set_pte(pte, __pte(pte_val(*pte) & ~_PAGE_PRESENT));
+ else
+ set_pte(pte, __pte(pte_val(*pte) | _PAGE_PRESENT));
+
+ flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
+
+ return true;
+}
+
+#endif /* _ASM_RISCV_KFENCE_H */
diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index 096463cc6fff..aa08dd2f8fae 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -14,6 +14,7 @@
#include <linux/signal.h>
#include <linux/uaccess.h>
#include <linux/kprobes.h>
+#include <linux/kfence.h>
#include <asm/ptrace.h>
#include <asm/tlbflush.h>
@@ -45,7 +46,15 @@ static inline void no_context(struct pt_regs *regs, unsigned long addr)
* Oops. The kernel tried to access some bad page. We'll have to
* terminate things with extreme prejudice.
*/
- msg = (addr < PAGE_SIZE) ? "NULL pointer dereference" : "paging request";
+ if (addr < PAGE_SIZE)
+ msg = "NULL pointer dereference";
+ else {
+ if (kfence_handle_page_fault(addr, regs->cause == EXC_STORE_PAGE_FAULT, regs))
+ return;
+
+ msg = "paging request";
+ }
+
die_kernel_fault(msg, addr, regs);
}
--
2.18.0.huawei.25
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH RFC v2] riscv: Enable KFENCE for riscv64
2021-05-14 3:44 [PATCH RFC v2] riscv: Enable KFENCE for riscv64 Liu Shixin
@ 2021-05-14 15:20 ` Marco Elver
2021-05-23 2:38 ` Palmer Dabbelt
0 siblings, 1 reply; 5+ messages in thread
From: Marco Elver @ 2021-05-14 15:20 UTC (permalink / raw)
To: Liu Shixin
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexander Potapenko,
Dmitry Vyukov, linux-riscv, LKML, kasan-dev
On Fri, 14 May 2021 at 05:11, Liu Shixin <liushixin2@huawei.com> wrote:
> Add architecture specific implementation details for KFENCE and enable
> KFENCE for the riscv64 architecture. In particular, this implements the
> required interface in <asm/kfence.h>.
>
> KFENCE requires that attributes for pages from its memory pool can
> individually be set. Therefore, force the kfence pool to be mapped at
> page granularity.
>
> I tested this patch using the testcases in kfence_test.c and all passed.
>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
Acked-by: Marco Elver <elver@google.com>
> ---
> v1->v2: Change kmalloc() to pte_alloc_one_kernel() for allocating pte.
>
> arch/riscv/Kconfig | 1 +
> arch/riscv/include/asm/kfence.h | 51 +++++++++++++++++++++++++++++++++
> arch/riscv/mm/fault.c | 11 ++++++-
> 3 files changed, 62 insertions(+), 1 deletion(-)
> create mode 100644 arch/riscv/include/asm/kfence.h
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index c426e7d20907..000d8aba1030 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -64,6 +64,7 @@ config RISCV
> select HAVE_ARCH_JUMP_LABEL_RELATIVE
> select HAVE_ARCH_KASAN if MMU && 64BIT
> select HAVE_ARCH_KASAN_VMALLOC if MMU && 64BIT
> + select HAVE_ARCH_KFENCE if MMU && 64BIT
> select HAVE_ARCH_KGDB
> select HAVE_ARCH_KGDB_QXFER_PKT
> select HAVE_ARCH_MMAP_RND_BITS if MMU
> diff --git a/arch/riscv/include/asm/kfence.h b/arch/riscv/include/asm/kfence.h
> new file mode 100644
> index 000000000000..c25d67e0b8ba
> --- /dev/null
> +++ b/arch/riscv/include/asm/kfence.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _ASM_RISCV_KFENCE_H
> +#define _ASM_RISCV_KFENCE_H
> +
> +#include <linux/kfence.h>
> +#include <linux/pfn.h>
> +#include <asm-generic/pgalloc.h>
> +#include <asm/pgtable.h>
> +
> +static inline bool arch_kfence_init_pool(void)
> +{
> + int i;
> + unsigned long addr;
> + pte_t *pte;
> + pmd_t *pmd;
> +
> + for (addr = (unsigned long)__kfence_pool; is_kfence_address((void *)addr);
> + addr += PAGE_SIZE) {
> + pte = virt_to_kpte(addr);
> + pmd = pmd_off_k(addr);
> +
> + if (!pmd_leaf(*pmd) && pte_present(*pte))
> + continue;
> +
> + pte = pte_alloc_one_kernel(&init_mm);
> + for (i = 0; i < PTRS_PER_PTE; i++)
> + set_pte(pte + i, pfn_pte(PFN_DOWN(__pa((addr & PMD_MASK) + i * PAGE_SIZE)), PAGE_KERNEL));
> +
> + set_pmd(pmd, pfn_pmd(PFN_DOWN(__pa(pte)), PAGE_TABLE));
> + flush_tlb_kernel_range(addr, addr + PMD_SIZE);
> + }
> +
> + return true;
> +}
> +
> +static inline bool kfence_protect_page(unsigned long addr, bool protect)
> +{
> + pte_t *pte = virt_to_kpte(addr);
> +
> + if (protect)
> + set_pte(pte, __pte(pte_val(*pte) & ~_PAGE_PRESENT));
> + else
> + set_pte(pte, __pte(pte_val(*pte) | _PAGE_PRESENT));
> +
> + flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
> +
> + return true;
> +}
> +
> +#endif /* _ASM_RISCV_KFENCE_H */
> diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
> index 096463cc6fff..aa08dd2f8fae 100644
> --- a/arch/riscv/mm/fault.c
> +++ b/arch/riscv/mm/fault.c
> @@ -14,6 +14,7 @@
> #include <linux/signal.h>
> #include <linux/uaccess.h>
> #include <linux/kprobes.h>
> +#include <linux/kfence.h>
>
> #include <asm/ptrace.h>
> #include <asm/tlbflush.h>
> @@ -45,7 +46,15 @@ static inline void no_context(struct pt_regs *regs, unsigned long addr)
> * Oops. The kernel tried to access some bad page. We'll have to
> * terminate things with extreme prejudice.
> */
> - msg = (addr < PAGE_SIZE) ? "NULL pointer dereference" : "paging request";
> + if (addr < PAGE_SIZE)
> + msg = "NULL pointer dereference";
> + else {
> + if (kfence_handle_page_fault(addr, regs->cause == EXC_STORE_PAGE_FAULT, regs))
> + return;
> +
> + msg = "paging request";
> + }
> +
> die_kernel_fault(msg, addr, regs);
> }
>
> --
> 2.18.0.huawei.25
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RFC v2] riscv: Enable KFENCE for riscv64
2021-05-14 15:20 ` Marco Elver
@ 2021-05-23 2:38 ` Palmer Dabbelt
2021-05-24 1:56 ` Liu Shixin
0 siblings, 1 reply; 5+ messages in thread
From: Palmer Dabbelt @ 2021-05-23 2:38 UTC (permalink / raw)
To: elver
Cc: liushixin2, Paul Walmsley, aou, glider, dvyukov, linux-riscv,
linux-kernel, kasan-dev
On Fri, 14 May 2021 08:20:10 PDT (-0700), elver@google.com wrote:
> On Fri, 14 May 2021 at 05:11, Liu Shixin <liushixin2@huawei.com> wrote:
>> Add architecture specific implementation details for KFENCE and enable
>> KFENCE for the riscv64 architecture. In particular, this implements the
>> required interface in <asm/kfence.h>.
>>
>> KFENCE requires that attributes for pages from its memory pool can
>> individually be set. Therefore, force the kfence pool to be mapped at
>> page granularity.
>>
>> I tested this patch using the testcases in kfence_test.c and all passed.
>>
>> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
>
> Acked-by: Marco Elver <elver@google.com>
>
>
>> ---
>> v1->v2: Change kmalloc() to pte_alloc_one_kernel() for allocating pte.
>>
>> arch/riscv/Kconfig | 1 +
>> arch/riscv/include/asm/kfence.h | 51 +++++++++++++++++++++++++++++++++
>> arch/riscv/mm/fault.c | 11 ++++++-
>> 3 files changed, 62 insertions(+), 1 deletion(-)
>> create mode 100644 arch/riscv/include/asm/kfence.h
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index c426e7d20907..000d8aba1030 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -64,6 +64,7 @@ config RISCV
>> select HAVE_ARCH_JUMP_LABEL_RELATIVE
>> select HAVE_ARCH_KASAN if MMU && 64BIT
>> select HAVE_ARCH_KASAN_VMALLOC if MMU && 64BIT
>> + select HAVE_ARCH_KFENCE if MMU && 64BIT
>> select HAVE_ARCH_KGDB
>> select HAVE_ARCH_KGDB_QXFER_PKT
>> select HAVE_ARCH_MMAP_RND_BITS if MMU
>> diff --git a/arch/riscv/include/asm/kfence.h b/arch/riscv/include/asm/kfence.h
>> new file mode 100644
>> index 000000000000..c25d67e0b8ba
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/kfence.h
>> @@ -0,0 +1,51 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +#ifndef _ASM_RISCV_KFENCE_H
>> +#define _ASM_RISCV_KFENCE_H
>> +
>> +#include <linux/kfence.h>
>> +#include <linux/pfn.h>
>> +#include <asm-generic/pgalloc.h>
>> +#include <asm/pgtable.h>
>> +
>> +static inline bool arch_kfence_init_pool(void)
>> +{
>> + int i;
>> + unsigned long addr;
>> + pte_t *pte;
>> + pmd_t *pmd;
>> +
>> + for (addr = (unsigned long)__kfence_pool; is_kfence_address((void *)addr);
>> + addr += PAGE_SIZE) {
>> + pte = virt_to_kpte(addr);
>> + pmd = pmd_off_k(addr);
>> +
>> + if (!pmd_leaf(*pmd) && pte_present(*pte))
>> + continue;
>> +
>> + pte = pte_alloc_one_kernel(&init_mm);
>> + for (i = 0; i < PTRS_PER_PTE; i++)
>> + set_pte(pte + i, pfn_pte(PFN_DOWN(__pa((addr & PMD_MASK) + i * PAGE_SIZE)), PAGE_KERNEL));
>> +
>> + set_pmd(pmd, pfn_pmd(PFN_DOWN(__pa(pte)), PAGE_TABLE));
>> + flush_tlb_kernel_range(addr, addr + PMD_SIZE);
>> + }
>> +
>> + return true;
>> +}
I'm not fundamentally opposed to this, but the arm64 approach where
pages are split at runtime when they have mis-matched permissions seems
cleaner to me. I'm not sure why x86 is doing it during init, though, as
IIUC set_memory_4k() will work for both.
Upgrading our __set_memory() with the ability to split pages (like arm64
has) seems generally useful, and would let us trivially implement the
dynamic version of this. We'll probably end up with the ability to
split pages anyway, so that would be the least code in the long run.
If there's some reason to prefer statically allocating the pages I'm
fine with this, though.
>> +
>> +static inline bool kfence_protect_page(unsigned long addr, bool protect)
>> +{
>> + pte_t *pte = virt_to_kpte(addr);
>> +
>> + if (protect)
>> + set_pte(pte, __pte(pte_val(*pte) & ~_PAGE_PRESENT));
>> + else
>> + set_pte(pte, __pte(pte_val(*pte) | _PAGE_PRESENT));
>> +
>> + flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
>> +
>> + return true;
>> +}
>> +
>> +#endif /* _ASM_RISCV_KFENCE_H */
>> diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
>> index 096463cc6fff..aa08dd2f8fae 100644
>> --- a/arch/riscv/mm/fault.c
>> +++ b/arch/riscv/mm/fault.c
>> @@ -14,6 +14,7 @@
>> #include <linux/signal.h>
>> #include <linux/uaccess.h>
>> #include <linux/kprobes.h>
>> +#include <linux/kfence.h>
>>
>> #include <asm/ptrace.h>
>> #include <asm/tlbflush.h>
>> @@ -45,7 +46,15 @@ static inline void no_context(struct pt_regs *regs, unsigned long addr)
>> * Oops. The kernel tried to access some bad page. We'll have to
>> * terminate things with extreme prejudice.
>> */
>> - msg = (addr < PAGE_SIZE) ? "NULL pointer dereference" : "paging request";
>> + if (addr < PAGE_SIZE)
>> + msg = "NULL pointer dereference";
>> + else {
>> + if (kfence_handle_page_fault(addr, regs->cause == EXC_STORE_PAGE_FAULT, regs))
>> + return;
>> +
>> + msg = "paging request";
>> + }
>> +
>> die_kernel_fault(msg, addr, regs);
>> }
>>
>> --
>> 2.18.0.huawei.25
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RFC v2] riscv: Enable KFENCE for riscv64
2021-05-23 2:38 ` Palmer Dabbelt
@ 2021-05-24 1:56 ` Liu Shixin
2021-05-25 8:52 ` Marco Elver
0 siblings, 1 reply; 5+ messages in thread
From: Liu Shixin @ 2021-05-24 1:56 UTC (permalink / raw)
To: Palmer Dabbelt, elver
Cc: Paul Walmsley, aou, glider, dvyukov, linux-riscv, linux-kernel,
kasan-dev
On 2021/5/23 10:38, Palmer Dabbelt wrote:
> On Fri, 14 May 2021 08:20:10 PDT (-0700), elver@google.com wrote:
>> On Fri, 14 May 2021 at 05:11, Liu Shixin <liushixin2@huawei.com> wrote:
>>> Add architecture specific implementation details for KFENCE and enable
>>> KFENCE for the riscv64 architecture. In particular, this implements the
>>> required interface in <asm/kfence.h>.
>>>
>>> KFENCE requires that attributes for pages from its memory pool can
>>> individually be set. Therefore, force the kfence pool to be mapped at
>>> page granularity.
>>>
>>> I tested this patch using the testcases in kfence_test.c and all passed.
>>>
>>> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
>>
>> Acked-by: Marco Elver <elver@google.com>
>>
>>
>>> ---
>>> v1->v2: Change kmalloc() to pte_alloc_one_kernel() for allocating pte.
>>>
>>> arch/riscv/Kconfig | 1 +
>>> arch/riscv/include/asm/kfence.h | 51 +++++++++++++++++++++++++++++++++
>>> arch/riscv/mm/fault.c | 11 ++++++-
>>> 3 files changed, 62 insertions(+), 1 deletion(-)
>>> create mode 100644 arch/riscv/include/asm/kfence.h
>>>
>>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>>> index c426e7d20907..000d8aba1030 100644
>>> --- a/arch/riscv/Kconfig
>>> +++ b/arch/riscv/Kconfig
>>> @@ -64,6 +64,7 @@ config RISCV
>>> select HAVE_ARCH_JUMP_LABEL_RELATIVE
>>> select HAVE_ARCH_KASAN if MMU && 64BIT
>>> select HAVE_ARCH_KASAN_VMALLOC if MMU && 64BIT
>>> + select HAVE_ARCH_KFENCE if MMU && 64BIT
>>> select HAVE_ARCH_KGDB
>>> select HAVE_ARCH_KGDB_QXFER_PKT
>>> select HAVE_ARCH_MMAP_RND_BITS if MMU
>>> diff --git a/arch/riscv/include/asm/kfence.h b/arch/riscv/include/asm/kfence.h
>>> new file mode 100644
>>> index 000000000000..c25d67e0b8ba
>>> --- /dev/null
>>> +++ b/arch/riscv/include/asm/kfence.h
>>> @@ -0,0 +1,51 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +
>>> +#ifndef _ASM_RISCV_KFENCE_H
>>> +#define _ASM_RISCV_KFENCE_H
>>> +
>>> +#include <linux/kfence.h>
>>> +#include <linux/pfn.h>
>>> +#include <asm-generic/pgalloc.h>
>>> +#include <asm/pgtable.h>
>>> +
>>> +static inline bool arch_kfence_init_pool(void)
>>> +{
>>> + int i;
>>> + unsigned long addr;
>>> + pte_t *pte;
>>> + pmd_t *pmd;
>>> +
>>> + for (addr = (unsigned long)__kfence_pool; is_kfence_address((void *)addr);
>>> + addr += PAGE_SIZE) {
>>> + pte = virt_to_kpte(addr);
>>> + pmd = pmd_off_k(addr);
>>> +
>>> + if (!pmd_leaf(*pmd) && pte_present(*pte))
>>> + continue;
>>> +
>>> + pte = pte_alloc_one_kernel(&init_mm);
>>> + for (i = 0; i < PTRS_PER_PTE; i++)
>>> + set_pte(pte + i, pfn_pte(PFN_DOWN(__pa((addr & PMD_MASK) + i * PAGE_SIZE)), PAGE_KERNEL));
>>> +
>>> + set_pmd(pmd, pfn_pmd(PFN_DOWN(__pa(pte)), PAGE_TABLE));
>>> + flush_tlb_kernel_range(addr, addr + PMD_SIZE);
>>> + }
>>> +
>>> + return true;
>>> +}
>
> I'm not fundamentally opposed to this, but the arm64 approach where pages are split at runtime when they have mis-matched permissions seems cleaner to me. I'm not sure why x86 is doing it during init, though, as IIUC set_memory_4k() will work for both.
>
> Upgrading our __set_memory() with the ability to split pages (like arm64 has) seems generally useful, and would let us trivially implement the dynamic version of this. We'll probably end up with the ability to split pages anyway, so that would be the least code in the long run.
>
> If there's some reason to prefer statically allocating the pages I'm fine with this, though.
>
As I understand，the arm64 approach does not implement dynamic splitting.
If kfence is enabled in arch arm64, the linear map need to be forcibly mapped
at page granularity. But x86 does not have such constraints as it only split pages
in the kfence pool, so I think the x86 approach is better as it has less influence
on the whole.
>>> +
>>> +static inline bool kfence_protect_page(unsigned long addr, bool protect)
>>> +{
>>> + pte_t *pte = virt_to_kpte(addr);
>>> +
>>> + if (protect)
>>> + set_pte(pte, __pte(pte_val(*pte) & ~_PAGE_PRESENT));
>>> + else
>>> + set_pte(pte, __pte(pte_val(*pte) | _PAGE_PRESENT));
>>> +
>>> + flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
>>> +
>>> + return true;
>>> +}
>>> +
>>> +#endif /* _ASM_RISCV_KFENCE_H */
>>> diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
>>> index 096463cc6fff..aa08dd2f8fae 100644
>>> --- a/arch/riscv/mm/fault.c
>>> +++ b/arch/riscv/mm/fault.c
>>> @@ -14,6 +14,7 @@
>>> #include <linux/signal.h>
>>> #include <linux/uaccess.h>
>>> #include <linux/kprobes.h>
>>> +#include <linux/kfence.h>
>>>
>>> #include <asm/ptrace.h>
>>> #include <asm/tlbflush.h>
>>> @@ -45,7 +46,15 @@ static inline void no_context(struct pt_regs *regs, unsigned long addr)
>>> * Oops. The kernel tried to access some bad page. We'll have to
>>> * terminate things with extreme prejudice.
>>> */
>>> - msg = (addr < PAGE_SIZE) ? "NULL pointer dereference" : "paging request";
>>> + if (addr < PAGE_SIZE)
>>> + msg = "NULL pointer dereference";
>>> + else {
>>> + if (kfence_handle_page_fault(addr, regs->cause == EXC_STORE_PAGE_FAULT, regs))
>>> + return;
>>> +
>>> + msg = "paging request";
>>> + }
>>> +
>>> die_kernel_fault(msg, addr, regs);
>>> }
>>>
>>> --
>>> 2.18.0.huawei.25
>>>
>
> .
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RFC v2] riscv: Enable KFENCE for riscv64
2021-05-24 1:56 ` Liu Shixin
@ 2021-05-25 8:52 ` Marco Elver
0 siblings, 0 replies; 5+ messages in thread
From: Marco Elver @ 2021-05-25 8:52 UTC (permalink / raw)
To: Liu Shixin
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Alexander Potapenko,
Dmitry Vyukov, linux-riscv, LKML, kasan-dev
On Mon, 24 May 2021 at 03:56, Liu Shixin <liushixin2@huawei.com> wrote:
> On 2021/5/23 10:38, Palmer Dabbelt wrote:
> > On Fri, 14 May 2021 08:20:10 PDT (-0700), elver@google.com wrote:
> >> On Fri, 14 May 2021 at 05:11, Liu Shixin <liushixin2@huawei.com> wrote:
> >>> Add architecture specific implementation details for KFENCE and enable
> >>> KFENCE for the riscv64 architecture. In particular, this implements the
> >>> required interface in <asm/kfence.h>.
> >>>
> >>> KFENCE requires that attributes for pages from its memory pool can
> >>> individually be set. Therefore, force the kfence pool to be mapped at
> >>> page granularity.
> >>>
> >>> I tested this patch using the testcases in kfence_test.c and all passed.
> >>>
> >>> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> >>
> >> Acked-by: Marco Elver <elver@google.com>
> >>
> >>
> >>> ---
> >>> v1->v2: Change kmalloc() to pte_alloc_one_kernel() for allocating pte.
> >>>
> >>> arch/riscv/Kconfig | 1 +
> >>> arch/riscv/include/asm/kfence.h | 51 +++++++++++++++++++++++++++++++++
> >>> arch/riscv/mm/fault.c | 11 ++++++-
> >>> 3 files changed, 62 insertions(+), 1 deletion(-)
> >>> create mode 100644 arch/riscv/include/asm/kfence.h
> >>>
> >>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> >>> index c426e7d20907..000d8aba1030 100644
> >>> --- a/arch/riscv/Kconfig
> >>> +++ b/arch/riscv/Kconfig
> >>> @@ -64,6 +64,7 @@ config RISCV
> >>> select HAVE_ARCH_JUMP_LABEL_RELATIVE
> >>> select HAVE_ARCH_KASAN if MMU && 64BIT
> >>> select HAVE_ARCH_KASAN_VMALLOC if MMU && 64BIT
> >>> + select HAVE_ARCH_KFENCE if MMU && 64BIT
> >>> select HAVE_ARCH_KGDB
> >>> select HAVE_ARCH_KGDB_QXFER_PKT
> >>> select HAVE_ARCH_MMAP_RND_BITS if MMU
> >>> diff --git a/arch/riscv/include/asm/kfence.h b/arch/riscv/include/asm/kfence.h
> >>> new file mode 100644
> >>> index 000000000000..c25d67e0b8ba
> >>> --- /dev/null
> >>> +++ b/arch/riscv/include/asm/kfence.h
> >>> @@ -0,0 +1,51 @@
> >>> +/* SPDX-License-Identifier: GPL-2.0 */
> >>> +
> >>> +#ifndef _ASM_RISCV_KFENCE_H
> >>> +#define _ASM_RISCV_KFENCE_H
> >>> +
> >>> +#include <linux/kfence.h>
> >>> +#include <linux/pfn.h>
> >>> +#include <asm-generic/pgalloc.h>
> >>> +#include <asm/pgtable.h>
> >>> +
> >>> +static inline bool arch_kfence_init_pool(void)
> >>> +{
> >>> + int i;
> >>> + unsigned long addr;
> >>> + pte_t *pte;
> >>> + pmd_t *pmd;
> >>> +
> >>> + for (addr = (unsigned long)__kfence_pool; is_kfence_address((void *)addr);
> >>> + addr += PAGE_SIZE) {
> >>> + pte = virt_to_kpte(addr);
> >>> + pmd = pmd_off_k(addr);
> >>> +
> >>> + if (!pmd_leaf(*pmd) && pte_present(*pte))
> >>> + continue;
> >>> +
> >>> + pte = pte_alloc_one_kernel(&init_mm);
> >>> + for (i = 0; i < PTRS_PER_PTE; i++)
> >>> + set_pte(pte + i, pfn_pte(PFN_DOWN(__pa((addr & PMD_MASK) + i * PAGE_SIZE)), PAGE_KERNEL));
> >>> +
> >>> + set_pmd(pmd, pfn_pmd(PFN_DOWN(__pa(pte)), PAGE_TABLE));
> >>> + flush_tlb_kernel_range(addr, addr + PMD_SIZE);
> >>> + }
> >>> +
> >>> + return true;
> >>> +}
> >
> > I'm not fundamentally opposed to this, but the arm64 approach where pages are split at runtime when they have mis-matched permissions seems cleaner to me. I'm not sure why x86 is doing it during init, though, as IIUC set_memory_4k() will work for both.
> >
> > Upgrading our __set_memory() with the ability to split pages (like arm64 has) seems generally useful, and would let us trivially implement the dynamic version of this. We'll probably end up with the ability to split pages anyway, so that would be the least code in the long run.
> >
> > If there's some reason to prefer statically allocating the pages I'm fine with this, though.
> >
> As I understand，the arm64 approach does not implement dynamic splitting.
> If kfence is enabled in arch arm64, the linear map need to be forcibly mapped
> at page granularity. But x86 does not have such constraints as it only split pages
> in the kfence pool, so I think the x86 approach is better as it has less influence
> on the whole.
Correct.
I think either riscv gains set_memory_4k(), like x86, or we go with
the approach in this patch. Unless you see this is trivially
implementable, I wouldn't want to block this patch. It's better to
have something working, and then incrementally improve later if riscv
ever gets set_memory_4k().
Thanks,
-- Marco
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-25 8:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-14 3:44 [PATCH RFC v2] riscv: Enable KFENCE for riscv64 Liu Shixin
2021-05-14 15:20 ` Marco Elver
2021-05-23 2:38 ` Palmer Dabbelt
2021-05-24 1:56 ` Liu Shixin
2021-05-25 8:52 ` Marco Elver
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).