The kprobe page is allocated by execmem allocator with ROX permission. It needs to call set_memory_rox() to set proper permission for the direct map too. It was missed.
Fixes: 10d5e97c1bf8 ("arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page") Cc: stable@vger.kernel.org Signed-off-by: Yang Shi yang@os.amperecomputing.com Reviewed-by: Catalin Marinas catalin.marinas@arm.com --- v2: Separated the patch from BBML2 series since it is an orthogonal bug fix per Ryan. Fixed the variable name nit per Catalin. Collected R-bs from Catalin.
arch/arm64/kernel/probes/kprobes.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c index 0c5d408afd95..8ab6104a4883 100644 --- a/arch/arm64/kernel/probes/kprobes.c +++ b/arch/arm64/kernel/probes/kprobes.c @@ -10,6 +10,7 @@
#define pr_fmt(fmt) "kprobes: " fmt
+#include <linux/execmem.h> #include <linux/extable.h> #include <linux/kasan.h> #include <linux/kernel.h> @@ -41,6 +42,17 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk); static void __kprobes post_kprobe_handler(struct kprobe *, struct kprobe_ctlblk *, struct pt_regs *);
+void *alloc_insn_page(void) +{ + void *addr; + + addr = execmem_alloc(EXECMEM_KPROBES, PAGE_SIZE); + if (!addr) + return NULL; + set_memory_rox((unsigned long)addr, 1); + return addr; +} + static void __kprobes arch_prepare_ss_slot(struct kprobe *p) { kprobe_opcode_t *addr = p->ainsn.xol_insn;
On Thu, Sep 18, 2025 at 09:23:49AM -0700, Yang Shi wrote:
The kprobe page is allocated by execmem allocator with ROX permission. It needs to call set_memory_rox() to set proper permission for the direct map too. It was missed.
Fixes: 10d5e97c1bf8 ("arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page") Cc: stable@vger.kernel.org Signed-off-by: Yang Shi yang@os.amperecomputing.com Reviewed-by: Catalin Marinas catalin.marinas@arm.com
v2: Separated the patch from BBML2 series since it is an orthogonal bug fix per Ryan. Fixed the variable name nit per Catalin. Collected R-bs from Catalin.
arch/arm64/kernel/probes/kprobes.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c index 0c5d408afd95..8ab6104a4883 100644 --- a/arch/arm64/kernel/probes/kprobes.c +++ b/arch/arm64/kernel/probes/kprobes.c @@ -10,6 +10,7 @@ #define pr_fmt(fmt) "kprobes: " fmt +#include <linux/execmem.h> #include <linux/extable.h> #include <linux/kasan.h> #include <linux/kernel.h> @@ -41,6 +42,17 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk); static void __kprobes post_kprobe_handler(struct kprobe *, struct kprobe_ctlblk *, struct pt_regs *); +void *alloc_insn_page(void) +{
- void *addr;
- addr = execmem_alloc(EXECMEM_KPROBES, PAGE_SIZE);
- if (!addr)
return NULL;- set_memory_rox((unsigned long)addr, 1);
- return addr;
+}
Why isn't execmem taking care of this? It looks to me like the execmem_cache_alloc() path calls set_memory_rox() but the execmem_vmalloc() path doesn't?
It feels a bit bizarre to me that we have to provide our own wrapper (which is identical to what s390 does). Also, how does alloc_insn_page() handle the direct map alias on x86?
Will
On 9/18/25 10:26 AM, Will Deacon wrote:
On Thu, Sep 18, 2025 at 09:23:49AM -0700, Yang Shi wrote:
The kprobe page is allocated by execmem allocator with ROX permission. It needs to call set_memory_rox() to set proper permission for the direct map too. It was missed.
Fixes: 10d5e97c1bf8 ("arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page") Cc: stable@vger.kernel.org Signed-off-by: Yang Shi yang@os.amperecomputing.com Reviewed-by: Catalin Marinas catalin.marinas@arm.com
v2: Separated the patch from BBML2 series since it is an orthogonal bug fix per Ryan. Fixed the variable name nit per Catalin. Collected R-bs from Catalin.
arch/arm64/kernel/probes/kprobes.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c index 0c5d408afd95..8ab6104a4883 100644 --- a/arch/arm64/kernel/probes/kprobes.c +++ b/arch/arm64/kernel/probes/kprobes.c @@ -10,6 +10,7 @@ #define pr_fmt(fmt) "kprobes: " fmt +#include <linux/execmem.h> #include <linux/extable.h> #include <linux/kasan.h> #include <linux/kernel.h> @@ -41,6 +42,17 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk); static void __kprobes post_kprobe_handler(struct kprobe *, struct kprobe_ctlblk *, struct pt_regs *); +void *alloc_insn_page(void) +{
- void *addr;
- addr = execmem_alloc(EXECMEM_KPROBES, PAGE_SIZE);
- if (!addr)
return NULL;- set_memory_rox((unsigned long)addr, 1);
- return addr;
+}
Why isn't execmem taking care of this? It looks to me like the execmem_cache_alloc() path calls set_memory_rox() but the execmem_vmalloc() path doesn't?
execmem_cache_alloc() is just called if execmem ROX cache is enabled, but it currently just supported by x86. Included Mike to this thread who is the author of execmem ROX cache.
It feels a bit bizarre to me that we have to provide our own wrapper (which is identical to what s390 does). Also, how does alloc_insn_page() handle the direct map alias on x86?
x86 handles it via execmem ROX cache.
Thanks, Yang
Will
On Thu, Sep 18, 2025 at 10:33:26AM -0700, Yang Shi wrote:
On 9/18/25 10:26 AM, Will Deacon wrote:
On Thu, Sep 18, 2025 at 09:23:49AM -0700, Yang Shi wrote:
The kprobe page is allocated by execmem allocator with ROX permission. It needs to call set_memory_rox() to set proper permission for the direct map too. It was missed.
Fixes: 10d5e97c1bf8 ("arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page") Cc: stable@vger.kernel.org Signed-off-by: Yang Shi yang@os.amperecomputing.com Reviewed-by: Catalin Marinas catalin.marinas@arm.com
v2: Separated the patch from BBML2 series since it is an orthogonal bug fix per Ryan. Fixed the variable name nit per Catalin. Collected R-bs from Catalin.
arch/arm64/kernel/probes/kprobes.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c index 0c5d408afd95..8ab6104a4883 100644 --- a/arch/arm64/kernel/probes/kprobes.c +++ b/arch/arm64/kernel/probes/kprobes.c @@ -10,6 +10,7 @@ #define pr_fmt(fmt) "kprobes: " fmt +#include <linux/execmem.h> #include <linux/extable.h> #include <linux/kasan.h> #include <linux/kernel.h> @@ -41,6 +42,17 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk); static void __kprobes post_kprobe_handler(struct kprobe *, struct kprobe_ctlblk *, struct pt_regs *); +void *alloc_insn_page(void) +{
- void *addr;
- addr = execmem_alloc(EXECMEM_KPROBES, PAGE_SIZE);
- if (!addr)
return NULL;- set_memory_rox((unsigned long)addr, 1);
- return addr;
+}
Why isn't execmem taking care of this? It looks to me like the execmem_cache_alloc() path calls set_memory_rox() but the execmem_vmalloc() path doesn't?
execmem_alloc() -> execmem_vmalloc() consolidated __vmalloc_node_range() for executable allocations. Those also didn't update the linear map alias.
It could be added to execmem_vmalloc(), but as of now we don't have a way for generic code to tell which set_memory method to call based on pgprot, so making execmem_vmalloc() to deal with direct map alias is quite involved.
It would be easier to just remove the direct map alias. It works on x86 so I don't see what can possibly go wrong :)
execmem_cache_alloc() is just called if execmem ROX cache is enabled, but it currently just supported by x86. Included Mike to this thread who is the author of execmem ROX cache.
It feels a bit bizarre to me that we have to provide our own wrapper (which is identical to what s390 does). Also, how does alloc_insn_page() handle the direct map alias on x86?
s390 had its version of alloc_insn_page() long before execmem so there I just replaced module_alloc() with exemem_alloc().
arm64 version of alloc_insn_page() didn't update the direct map before execmem, so I overlooked this issue when I was converting arm64 to execmem.
x86 handles it via execmem ROX cache.
Thanks, Yang
Will
On 18/09/25 10:56 pm, Will Deacon wrote:
On Thu, Sep 18, 2025 at 09:23:49AM -0700, Yang Shi wrote:
The kprobe page is allocated by execmem allocator with ROX permission. It needs to call set_memory_rox() to set proper permission for the direct map too. It was missed.
Fixes: 10d5e97c1bf8 ("arm64: use PAGE_KERNEL_ROX directly in alloc_insn_page") Cc: stable@vger.kernel.org Signed-off-by: Yang Shi yang@os.amperecomputing.com Reviewed-by: Catalin Marinas catalin.marinas@arm.com
v2: Separated the patch from BBML2 series since it is an orthogonal bug fix per Ryan. Fixed the variable name nit per Catalin. Collected R-bs from Catalin.
arch/arm64/kernel/probes/kprobes.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c index 0c5d408afd95..8ab6104a4883 100644 --- a/arch/arm64/kernel/probes/kprobes.c +++ b/arch/arm64/kernel/probes/kprobes.c @@ -10,6 +10,7 @@ #define pr_fmt(fmt) "kprobes: " fmt +#include <linux/execmem.h> #include <linux/extable.h> #include <linux/kasan.h> #include <linux/kernel.h> @@ -41,6 +42,17 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk); static void __kprobes post_kprobe_handler(struct kprobe *, struct kprobe_ctlblk *, struct pt_regs *); +void *alloc_insn_page(void) +{
- void *addr;
- addr = execmem_alloc(EXECMEM_KPROBES, PAGE_SIZE);
- if (!addr)
return NULL;- set_memory_rox((unsigned long)addr, 1);
- return addr;
+}
Why isn't execmem taking care of this? It looks to me like the execmem_cache_alloc() path calls set_memory_rox() but the execmem_vmalloc() path doesn't?
Ryan has raised this issue here - https://lore.kernel.org/all/d4019be7-e24c-4715-a42a-4f1fc39a9bd4@arm.com/
It feels a bit bizarre to me that we have to provide our own wrapper (which is identical to what s390 does). Also, how does alloc_insn_page() handle the direct map alias on x86?
Will
linux-stable-mirror@lists.linaro.org
-
Dev Jain -
Mike Rapoport -
Will Deacon -
Yang Shi