* Re: [PATCH v3] LoongArch: Enable STRICT_MODULE_RWX for stricter modules memory permissions
2026-06-08 6:38 [PATCH v3] LoongArch: Enable STRICT_MODULE_RWX for stricter modules memory permissions haoran.jiang
@ 2026-06-09 8:33 ` Huacai Chen
2026-06-11 6:27 ` Tiezhu Yang
0 siblings, 1 reply; 3+ messages in thread
From: Huacai Chen @ 2026-06-09 8:33 UTC (permalink / raw)
To: haoran.jiang
Cc: loongarch, linux-kernel, kernel, akpm, jbohac, kees, yangtiezhu,
Haoran Jiang
Hi, Haoran,
On Mon, Jun 8, 2026 at 2:39 PM <haoran.jiang@linux.dev> wrote:
>
> From: Haoran Jiang <jianghaoran@kylinos.cn>
>
> Enable STRICT_MODULE_RWX to enforce strict memory permissions
> on modules,making the code region non-writable, the data region
> non-executable, and the read-only data region both non-writable
> and non-executable.Temporarily modify code section read/write
> permissions via set_memory() API.
>
> Signed-off-by: Haoran Jiang <jianghaoran@kylinos.cn>
> ---
> v2:
> Change the method of modifying page table permissions from patch_map to set_memory() API.
>
> v3:
> Modify commit description.
>
> ---
> arch/loongarch/Kconfig | 2 ++
> arch/loongarch/kernel/inst.c | 18 ++++++++++++++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index 606597da46b8..40d748a13c50 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -27,6 +27,7 @@ config LOONGARCH
> select ARCH_HAS_PTE_SPECIAL if 64BIT
> select ARCH_HAS_SET_MEMORY
> select ARCH_HAS_SET_DIRECT_MAP
> + select ARCH_HAS_STRICT_MODULE_RWX
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> select ARCH_HAS_UBSAN
> select ARCH_HAS_VDSO_ARCH_DATA
> @@ -197,6 +198,7 @@ config LOONGARCH
> select NUMA_MEMBLKS if NUMA
> select OF
> select OF_EARLY_FLATTREE
> + select ARCH_OPTIONAL_KERNEL_RWX
Have you compared the performance with and without STRICT_MODULE_RWX?
If no performance decreases, I suggest not select
ARCH_OPTIONAL_KERNEL_RWX, this will cause STRICT_MODULE_RWX to be set
always.
> select PCI
> select PCI_DOMAINS_GENERIC
> select PCI_ECAM if ACPI
> diff --git a/arch/loongarch/kernel/inst.c b/arch/loongarch/kernel/inst.c
> index 0b9228b7c13a..877890c77d0c 100644
> --- a/arch/loongarch/kernel/inst.c
> +++ b/arch/loongarch/kernel/inst.c
> @@ -207,13 +207,31 @@ int larch_insn_read(void *addr, u32 *insnp)
> int larch_insn_write(void *addr, u32 insn)
> {
> int ret;
> + int err = 0;
> + size_t start;
> unsigned long flags = 0;
>
> if ((unsigned long)addr & 3)
> return -EINVAL;
>
> + start = round_down((size_t)addr, PAGE_SIZE);
> +
> raw_spin_lock_irqsave(&patch_lock, flags);
> +
> + err = set_memory_rw(start, 1);
> + if (err) {
> + pr_info("%s: set_memory_rw() failed\n", __func__);
> + return err;
> + }
I don't know how you verified the function, but it is obvious this
should be out of spinlock and irq disabled.
Yes, move it out will cause concurrent problem, so we should use
mutex, and the final state of this function will be like this:
err = set_memory_rw(start, 1);
local_irq_save(flags);
ret = copy_to_kernel_nofault(addr, &insn, LOONGARCH_INSN_SIZE);
local_irq_restore(flags);
err = set_memory_rox(start, 1);
Then the callers should use text_mutex to protect.
For example:
1. For ftrace, define ftrace_arch_code_modify_prepare() and
ftrace_arch_code_modify_post_process() as RISC-V.
2. For jump labels, protect larch_insn_write() with
mutex_lock(&text_mutex) and mutex_unlock(&text_mutex).
3. For kprobes,integrate this patch, and then also protect
larch_insn_patch_text() with mutex_lock(&text_mutex) and
mutex_unlock(&text_mutex).
https://lore.kernel.org/loongarch/CAHirt9gSXZvw0wTv9zupsVRG0_ekza08efW0hwq_yvnU1WbUng@mail.gmail.com/T/#mf0aa514e1f0d0fc9a181fc5aa15b6be737083a29
In addition, it seems that text_mutex and patch_lock are for the same
purpose, so patch_lock can be removed here.
@Tiezhu, what's your opinion?
Huacai
> +
> ret = copy_to_kernel_nofault(addr, &insn, LOONGARCH_INSN_SIZE);
> +
> + err = set_memory_rox(start, 1);
> + if (err) {
> + pr_info("%s: set_memory_rox() failed\n", __func__);
> + return err;
> + }
> +
> raw_spin_unlock_irqrestore(&patch_lock, flags);
>
> return ret;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread