[PATCH v2 0/2] riscv: fix kprobes on minimal kernel configs

[PATCH v2 0/2] riscv: fix kprobes on minimal kernel configs

[Xiaofeng Yuan]

When building with an allnoconfig-derived minimal configuration
(without CONFIG_STRICT_MODULE_RWX), kprobes fails because
instruction slot pages from EXECMEM_KPROBES are read-only and
patch_map() bypasses the fixmap writable alias.

These two patches fix the infrastructure.

v2:
  - use CONFIG_STRICT_MODULE_RWX instead of CONFIG_ARCH_HAS_EXECMEM_ROX
    in PATCH 1/2 (per Nam Cao's review)

Xiaofeng Yuan (2):
  riscv: mm: make EXECMEM_KPROBES writable without
    CONFIG_STRICT_MODULE_RWX
  riscv: patch: skip fixmap mapping when kernel text is already
    writable

 arch/riscv/kernel/patch.c | 3 ++-
 arch/riscv/mm/init.c      | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

[PATCH v2 0/2] riscv: fix kprobes on minimal kernel configs

[Xiaofeng Yuan]

When building with an allnoconfig-derived minimal configuration
(without CONFIG_STRICT_MODULE_RWX), kprobes fails because
instruction slot pages from EXECMEM_KPROBES are read-only and
patch_map() bypasses the fixmap writable alias.

These two patches fix the infrastructure.

v2:
  - use CONFIG_STRICT_MODULE_RWX instead of CONFIG_ARCH_HAS_EXECMEM_ROX
    in PATCH 1/2 (per Nam Cao's review)

Xiaofeng Yuan (2):
  riscv: mm: make EXECMEM_KPROBES writable without
    CONFIG_STRICT_MODULE_RWX
  riscv: patch: skip fixmap mapping when kernel text is already
    writable

 arch/riscv/kernel/patch.c | 3 ++-
 arch/riscv/mm/init.c      | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 1/2] riscv: mm: make EXECMEM_KPROBES writable without CONFIG_STRICT_MODULE_RWX

[Xiaofeng Yuan]

...

Signed-off-by: Xiaofeng Yuan <xiaofengmian@163.com>
---
v2: use CONFIG_STRICT_MODULE_RWX instead of CONFIG_ARCH_HAS_EXECMEM_ROX (per Nam Cao's review)

 arch/riscv/mm/init.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 5b1b3c88b4..7951d72a12 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -1457,7 +1457,9 @@ struct execmem_info __init *execmem_arch_setup(void)
 			[EXECMEM_KPROBES] = {
 				.start	= VMALLOC_START,
 				.end	= VMALLOC_END,
-				.pgprot	= PAGE_KERNEL_READ_EXEC,
+				.pgprot	= IS_ENABLED(CONFIG_STRICT_MODULE_RWX) ?
+					  PAGE_KERNEL_READ_EXEC :
+					  PAGE_KERNEL_EXEC,
 				.alignment = 1,
 			},
 			[EXECMEM_BPF] = {
-- 
2.43.0

[PATCH v2 1/2] riscv: mm: make EXECMEM_KPROBES writable without CONFIG_STRICT_MODULE_RWX

[Xiaofeng Yuan]

...

Signed-off-by: Xiaofeng Yuan <xiaofengmian@163.com>
---
v2: use CONFIG_STRICT_MODULE_RWX instead of CONFIG_ARCH_HAS_EXECMEM_ROX (per Nam Cao's review)

 arch/riscv/mm/init.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 5b1b3c88b4..7951d72a12 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -1457,7 +1457,9 @@ struct execmem_info __init *execmem_arch_setup(void)
 			[EXECMEM_KPROBES] = {
 				.start	= VMALLOC_START,
 				.end	= VMALLOC_END,
-				.pgprot	= PAGE_KERNEL_READ_EXEC,
+				.pgprot	= IS_ENABLED(CONFIG_STRICT_MODULE_RWX) ?
+					  PAGE_KERNEL_READ_EXEC :
+					  PAGE_KERNEL_EXEC,
 				.alignment = 1,
 			},
 			[EXECMEM_BPF] = {
-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 1/2] riscv: mm: make EXECMEM_KPROBES writable without CONFIG_STRICT_MODULE_RWX

[Nam Cao]

Xiaofeng Yuan <xiaofengmian@163.com> writes:
> ...
>
> Signed-off-by: Xiaofeng Yuan <xiaofengmian@163.com>

Commit description is missing.

For the diff:
Reviewed-by: Nam Cao <namcao@linutronix.de>

Re: [PATCH v2 1/2] riscv: mm: make EXECMEM_KPROBES writable without CONFIG_STRICT_MODULE_RWX

[Nam Cao]

Xiaofeng Yuan <xiaofengmian@163.com> writes:
> ...
>
> Signed-off-by: Xiaofeng Yuan <xiaofengmian@163.com>

Commit description is missing.

For the diff:
Reviewed-by: Nam Cao <namcao@linutronix.de>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 2/2] riscv: patch: skip fixmap mapping when kernel text is already writable

[Xiaofeng Yuan]

...

Signed-off-by: Xiaofeng Yuan <xiaofengmian@163.com>
---
 arch/riscv/kernel/patch.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
index 16b243376f..b6e8e1e83e 100644
--- a/arch/riscv/kernel/patch.c
+++ b/arch/riscv/kernel/patch.c
@@ -44,7 +44,8 @@ static __always_inline void *patch_map(void *addr, const unsigned int fixmap)
 	uintptr_t uintaddr = (uintptr_t) addr;
 	phys_addr_t phys;
 
-	if (core_kernel_text(uintaddr) || is_kernel_exittext(uintaddr)) {
+	if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) &&
+	    (core_kernel_text(uintaddr) || is_kernel_exittext(uintaddr))) {
 		phys = __pa_symbol(addr);
 	} else if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) {
 		struct page *page = vmalloc_to_page(addr);
-- 
2.43.0

[PATCH v2 2/2] riscv: patch: skip fixmap mapping when kernel text is already writable

[Xiaofeng Yuan]

...

Signed-off-by: Xiaofeng Yuan <xiaofengmian@163.com>
---
 arch/riscv/kernel/patch.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
index 16b243376f..b6e8e1e83e 100644
--- a/arch/riscv/kernel/patch.c
+++ b/arch/riscv/kernel/patch.c
@@ -44,7 +44,8 @@ static __always_inline void *patch_map(void *addr, const unsigned int fixmap)
 	uintptr_t uintaddr = (uintptr_t) addr;
 	phys_addr_t phys;
 
-	if (core_kernel_text(uintaddr) || is_kernel_exittext(uintaddr)) {
+	if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) &&
+	    (core_kernel_text(uintaddr) || is_kernel_exittext(uintaddr))) {
 		phys = __pa_symbol(addr);
 	} else if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) {
 		struct page *page = vmalloc_to_page(addr);
-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 2/2] riscv: patch: skip fixmap mapping when kernel text is already writable

[Nam Cao]

Xiaofeng Yuan <xiaofengmian@163.com> writes:

> ...
>
> Signed-off-by: Xiaofeng Yuan <xiaofengmian@163.com>

Also missing commit description.

> ---
>  arch/riscv/kernel/patch.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
> index 16b243376f..b6e8e1e83e 100644
> --- a/arch/riscv/kernel/patch.c
> +++ b/arch/riscv/kernel/patch.c
> @@ -44,7 +44,8 @@ static __always_inline void *patch_map(void *addr, const unsigned int fixmap)
>  	uintptr_t uintaddr = (uintptr_t) addr;
>  	phys_addr_t phys;
>  
> -	if (core_kernel_text(uintaddr) || is_kernel_exittext(uintaddr)) {
> +	if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) &&
> +	    (core_kernel_text(uintaddr) || is_kernel_exittext(uintaddr))) {
>  		phys = __pa_symbol(addr);
>  	} else if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) {
>  		struct page *page = vmalloc_to_page(addr);

Probably cleaner to have

if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX))
	return addr;

at the beginning of this function. And get rid of
CONFIG_STRICT_MODULE_RWX for the rest.

But looks functionally correct to me.

Nam

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 2/2] riscv: patch: skip fixmap mapping when kernel text is already writable

[Nam Cao]

Xiaofeng Yuan <xiaofengmian@163.com> writes:

> ...
>
> Signed-off-by: Xiaofeng Yuan <xiaofengmian@163.com>

Also missing commit description.

> ---
>  arch/riscv/kernel/patch.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
> index 16b243376f..b6e8e1e83e 100644
> --- a/arch/riscv/kernel/patch.c
> +++ b/arch/riscv/kernel/patch.c
> @@ -44,7 +44,8 @@ static __always_inline void *patch_map(void *addr, const unsigned int fixmap)
>  	uintptr_t uintaddr = (uintptr_t) addr;
>  	phys_addr_t phys;
>  
> -	if (core_kernel_text(uintaddr) || is_kernel_exittext(uintaddr)) {
> +	if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) &&
> +	    (core_kernel_text(uintaddr) || is_kernel_exittext(uintaddr))) {
>  		phys = __pa_symbol(addr);
>  	} else if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) {
>  		struct page *page = vmalloc_to_page(addr);

Probably cleaner to have

if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX))
	return addr;

at the beginning of this function. And get rid of
CONFIG_STRICT_MODULE_RWX for the rest.

But looks functionally correct to me.

Nam