In most recent linux-6.6.y tree, `arch/riscv/kernel/probes/kprobes.c::arch_prepare_ss_slot` still has the obsolete code:
u32 insn = __BUG_INSN_32; unsigned long offset = GET_INSN_LENGTH(p->opcode); p->ainsn.api.restore = (unsigned long)p->addr + offset; patch_text_nosync(p->ainsn.api.insn, &p->opcode, 1); patch_text_nosync((void *)p->ainsn.api.insn + offset, &insn, 1);
The last two 1s are wrong size of written instructions , which would lead to kernel crash, like `insmod kprobe_example.ko` gives:
[ 509.812815][ T2734] kprobe_init: Planted kprobe at 00000000c5c46130 [ 509.837606][ C5] handler_pre: <kernel_clone> p->addr = 0x00000000c5c46130, pc = 0xffffffff80032ee2, status = 0x200000120 [ 509.839315][ C5] Oops - illegal instruction [#1]
I've tried two patchs from torvalds tree and it didn't crash again:
51781ce8f448 riscv: Pass patch_text() the length in bytes (rebased) 13134cc94914 riscv: kprobes: Fix incorrect address calculation
Regards, laokz
On Tue, Apr 22, 2025 at 10:58:42AM +0800, Kai Zhang wrote:
In most recent linux-6.6.y tree, `arch/riscv/kernel/probes/kprobes.c::arch_prepare_ss_slot` still has the obsolete code:
u32 insn = __BUG_INSN_32; unsigned long offset = GET_INSN_LENGTH(p->opcode); p->ainsn.api.restore = (unsigned long)p->addr + offset; patch_text_nosync(p->ainsn.api.insn, &p->opcode, 1); patch_text_nosync((void *)p->ainsn.api.insn + offset, &insn, 1);
The last two 1s are wrong size of written instructions , which would lead to kernel crash, like `insmod kprobe_example.ko` gives:
[ 509.812815][ T2734] kprobe_init: Planted kprobe at 00000000c5c46130 [ 509.837606][ C5] handler_pre: <kernel_clone> p->addr = 0x00000000c5c46130, pc = 0xffffffff80032ee2, status = 0x200000120 [ 509.839315][ C5] Oops - illegal instruction [#1]
I've tried two patchs from torvalds tree and it didn't crash again:
51781ce8f448 riscv: Pass patch_text() the length in bytes (rebased) 13134cc94914 riscv: kprobes: Fix incorrect address calculation
Neither of these apply cleanly. Please provide working backports if you wish to see them added to the tree.
thanks,
greg k-h
On 4/22/2025 4:46 PM, Greg Kroah-Hartman wrote:
On Tue, Apr 22, 2025 at 10:58:42AM +0800, Kai Zhang wrote:
In most recent linux-6.6.y tree, `arch/riscv/kernel/probes/kprobes.c::arch_prepare_ss_slot` still has the obsolete code:
u32 insn = __BUG_INSN_32; unsigned long offset = GET_INSN_LENGTH(p->opcode); p->ainsn.api.restore = (unsigned long)p->addr + offset; patch_text_nosync(p->ainsn.api.insn, &p->opcode, 1); patch_text_nosync((void *)p->ainsn.api.insn + offset, &insn, 1);The last two 1s are wrong size of written instructions , which would lead to kernel crash, like `insmod kprobe_example.ko` gives:
[ 509.812815][ T2734] kprobe_init: Planted kprobe at 00000000c5c46130 [ 509.837606][ C5] handler_pre: <kernel_clone> p->addr = 0x00000000c5c46130, pc = 0xffffffff80032ee2, status = 0x200000120 [ 509.839315][ C5] Oops - illegal instruction [#1]
I've tried two patchs from torvalds tree and it didn't crash again:
51781ce8f448 riscv: Pass patch_text() the length in bytes (rebased) 13134cc94914 riscv: kprobes: Fix incorrect address calculation
Neither of these apply cleanly. Please provide working backports if you wish to see them added to the tree.
thanks,
greg k-h
revert 03753bfacbc6 apply 51781ce8f448 apply 13134cc94914
Regards, laokz
On Fri, Apr 25, 2025 at 04:03:41PM +0800, Kai Zhang wrote:
On 4/22/2025 4:46 PM, Greg Kroah-Hartman wrote:
On Tue, Apr 22, 2025 at 10:58:42AM +0800, Kai Zhang wrote:
In most recent linux-6.6.y tree, `arch/riscv/kernel/probes/kprobes.c::arch_prepare_ss_slot` still has the obsolete code:
u32 insn = __BUG_INSN_32; unsigned long offset = GET_INSN_LENGTH(p->opcode); p->ainsn.api.restore = (unsigned long)p->addr + offset; patch_text_nosync(p->ainsn.api.insn, &p->opcode, 1); patch_text_nosync((void *)p->ainsn.api.insn + offset, &insn, 1);The last two 1s are wrong size of written instructions , which would lead to kernel crash, like `insmod kprobe_example.ko` gives:
[ 509.812815][ T2734] kprobe_init: Planted kprobe at 00000000c5c46130 [ 509.837606][ C5] handler_pre: <kernel_clone> p->addr = 0x00000000c5c46130, pc = 0xffffffff80032ee2, status = 0x200000120 [ 509.839315][ C5] Oops - illegal instruction [#1]
I've tried two patchs from torvalds tree and it didn't crash again:
51781ce8f448 riscv: Pass patch_text() the length in bytes (rebased) 13134cc94914 riscv: kprobes: Fix incorrect address calculation
Neither of these apply cleanly. Please provide working backports if you wish to see them added to the tree.
thanks,
greg k-h
revert 03753bfacbc6 apply 51781ce8f448 apply 13134cc94914
Thanks, but that's not how we take patches for the stable tree. Please submit these all in a tested patch series and we will be glad to queue them up.
greg k-h
Hi Nam,
I reported a riscv kprobe bug of linux-6.6.y. It seems that 03753bfacbc6(riscv: kprobes: Fix incorrect address calculation) should be reverted. There are a lot of changes of riscv kprobe in upstream. I'm not all in sure of my suggested fix. Will you kind to help?
Thanks, laokz
On 4/25/2025 4:07 PM, Greg Kroah-Hartman wrote:
On Fri, Apr 25, 2025 at 04:03:41PM +0800, Kai Zhang wrote:
On 4/22/2025 4:46 PM, Greg Kroah-Hartman wrote:
On Tue, Apr 22, 2025 at 10:58:42AM +0800, Kai Zhang wrote:
In most recent linux-6.6.y tree, `arch/riscv/kernel/probes/kprobes.c::arch_prepare_ss_slot` still has the obsolete code:
u32 insn = __BUG_INSN_32; unsigned long offset = GET_INSN_LENGTH(p->opcode); p->ainsn.api.restore = (unsigned long)p->addr + offset; patch_text_nosync(p->ainsn.api.insn, &p->opcode, 1); patch_text_nosync((void *)p->ainsn.api.insn + offset, &insn, 1);The last two 1s are wrong size of written instructions , which would lead to kernel crash, like `insmod kprobe_example.ko` gives:
[ 509.812815][ T2734] kprobe_init: Planted kprobe at 00000000c5c46130 [ 509.837606][ C5] handler_pre: <kernel_clone> p->addr = 0x00000000c5c46130, pc = 0xffffffff80032ee2, status = 0x200000120 [ 509.839315][ C5] Oops - illegal instruction [#1]
I've tried two patchs from torvalds tree and it didn't crash again:
51781ce8f448 riscv: Pass patch_text() the length in bytes (rebased) 13134cc94914 riscv: kprobes: Fix incorrect address calculation
Neither of these apply cleanly. Please provide working backports if you wish to see them added to the tree.
thanks,
greg k-h
revert 03753bfacbc6 apply 51781ce8f448 apply 13134cc94914
Thanks, but that's not how we take patches for the stable tree. Please submit these all in a tested patch series and we will be glad to queue them up.
greg k-h
On Fri, Apr 25, 2025 at 08:09:21PM +0800, Kai Zhang wrote:
Hi Nam,
I reported a riscv kprobe bug of linux-6.6.y. It seems that 03753bfacbc6(riscv: kprobes: Fix incorrect address calculation) should be reverted. There are a lot of changes of riscv kprobe in upstream. I'm not all in sure of my suggested fix. Will you kind to help?
Certainly.
Thanks, laokz
On 4/25/2025 4:07 PM, Greg Kroah-Hartman wrote:
On Fri, Apr 25, 2025 at 04:03:41PM +0800, Kai Zhang wrote:
On 4/22/2025 4:46 PM, Greg Kroah-Hartman wrote:
On Tue, Apr 22, 2025 at 10:58:42AM +0800, Kai Zhang wrote:
In most recent linux-6.6.y tree, `arch/riscv/kernel/probes/kprobes.c::arch_prepare_ss_slot` still has the obsolete code:
u32 insn = __BUG_INSN_32; unsigned long offset = GET_INSN_LENGTH(p->opcode); p->ainsn.api.restore = (unsigned long)p->addr + offset; patch_text_nosync(p->ainsn.api.insn, &p->opcode, 1); patch_text_nosync((void *)p->ainsn.api.insn + offset, &insn, 1);The last two 1s are wrong size of written instructions , which would lead to kernel crash, like `insmod kprobe_example.ko` gives:
[ 509.812815][ T2734] kprobe_init: Planted kprobe at 00000000c5c46130 [ 509.837606][ C5] handler_pre: <kernel_clone> p->addr = 0x00000000c5c46130, pc = 0xffffffff80032ee2, status = 0x200000120 [ 509.839315][ C5] Oops - illegal instruction [#1]
I've tried two patchs from torvalds tree and it didn't crash again:
51781ce8f448 riscv: Pass patch_text() the length in bytes (rebased) 13134cc94914 riscv: kprobes: Fix incorrect address calculation
Please don't revert this patch. It fixes another issue.
You are correct that the sizes of the instructions are wrong. It can still happen to work if only one instruction is patched.
This bug is not specific to v6.6. It is in mainline as well. Therefore fix patch should be sent to mainline, and then backport to v6.6.
Can you please verify if the below patch fixes your crash?
Best regards, Nam
diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c index 4fbc70e823f0..dc431b965bc3 100644 --- a/arch/riscv/kernel/probes/kprobes.c +++ b/arch/riscv/kernel/probes/kprobes.c @@ -28,8 +28,8 @@ static void __kprobes arch_prepare_ss_slot(struct kprobe *p)
p->ainsn.api.restore = (unsigned long)p->addr + offset;
- patch_text_nosync(p->ainsn.api.insn, &p->opcode, 1); - patch_text_nosync((void *)p->ainsn.api.insn + offset, &insn, 1); + patch_text_nosync(p->ainsn.api.insn, &p->opcode, offset); + patch_text_nosync((void *)p->ainsn.api.insn + offset, &insn, sizeof(insn)); }
static void __kprobes arch_prepare_simulate(struct kprobe *p)
On Fri, Apr 25, 2025 at 02:49:52PM +0200, Nam Cao wrote:
On Fri, Apr 25, 2025 at 08:09:21PM +0800, Kai Zhang wrote:
On 4/25/2025 4:07 PM, Greg Kroah-Hartman wrote:
On Fri, Apr 25, 2025 at 04:03:41PM +0800, Kai Zhang wrote:
On 4/22/2025 4:46 PM, Greg Kroah-Hartman wrote:
On Tue, Apr 22, 2025 at 10:58:42AM +0800, Kai Zhang wrote:
In most recent linux-6.6.y tree, `arch/riscv/kernel/probes/kprobes.c::arch_prepare_ss_slot` still has the obsolete code:
u32 insn = __BUG_INSN_32; unsigned long offset = GET_INSN_LENGTH(p->opcode); p->ainsn.api.restore = (unsigned long)p->addr + offset; patch_text_nosync(p->ainsn.api.insn, &p->opcode, 1); patch_text_nosync((void *)p->ainsn.api.insn + offset, &insn, 1);The last two 1s are wrong size of written instructions , which would lead to kernel crash, like `insmod kprobe_example.ko` gives:
[ 509.812815][ T2734] kprobe_init: Planted kprobe at 00000000c5c46130 [ 509.837606][ C5] handler_pre: <kernel_clone> p->addr = 0x00000000c5c46130, pc = 0xffffffff80032ee2, status = 0x200000120 [ 509.839315][ C5] Oops - illegal instruction [#1]
I've tried two patchs from torvalds tree and it didn't crash again:
51781ce8f448 riscv: Pass patch_text() the length in bytes (rebased) 13134cc94914 riscv: kprobes: Fix incorrect address calculation
Please don't revert this patch. It fixes another issue.
You are correct that the sizes of the instructions are wrong. It can still happen to work if only one instruction is patched.
This bug is not specific to v6.6. It is in mainline as well. Therefore fix patch should be sent to mainline, and then backport to v6.6.
Sorry, I was confused. This is not in mainline. It has already been fixed by 51781ce8f448 ("riscv: Pass patch_text() the length in bytes")
But I wouldn't backport that patch, it is bigger than necessary. The patch I sent in the previous email should be enough.
Nam
On 4/25/2025 8:59 PM, Nam Cao wrote:
On Fri, Apr 25, 2025 at 02:49:52PM +0200, Nam Cao wrote:
On Fri, Apr 25, 2025 at 08:09:21PM +0800, Kai Zhang wrote:
On 4/25/2025 4:07 PM, Greg Kroah-Hartman wrote:
On Fri, Apr 25, 2025 at 04:03:41PM +0800, Kai Zhang wrote:
On 4/22/2025 4:46 PM, Greg Kroah-Hartman wrote:
On Tue, Apr 22, 2025 at 10:58:42AM +0800, Kai Zhang wrote: > In most recent linux-6.6.y tree, > `arch/riscv/kernel/probes/kprobes.c::arch_prepare_ss_slot` still has the > obsolete code: > > u32 insn = __BUG_INSN_32; > unsigned long offset = GET_INSN_LENGTH(p->opcode); > p->ainsn.api.restore = (unsigned long)p->addr + offset; > patch_text_nosync(p->ainsn.api.insn, &p->opcode, 1); > patch_text_nosync((void *)p->ainsn.api.insn + offset, &insn, 1); > > The last two 1s are wrong size of written instructions , which would lead to > kernel crash, like `insmod kprobe_example.ko` gives: > > [ 509.812815][ T2734] kprobe_init: Planted kprobe at 00000000c5c46130 > [ 509.837606][ C5] handler_pre: <kernel_clone> p->addr = > 0x00000000c5c46130, pc = 0xffffffff80032ee2, status = 0x200000120 > [ 509.839315][ C5] Oops - illegal instruction [#1] > > > I've tried two patchs from torvalds tree and it didn't crash again: > > 51781ce8f448 riscv: Pass patch_text() the length in bytes (rebased) > 13134cc94914 riscv: kprobes: Fix incorrect address calculation
Please don't revert this patch. It fixes another issue.
You are correct that the sizes of the instructions are wrong. It can still happen to work if only one instruction is patched.
This bug is not specific to v6.6. It is in mainline as well. Therefore fix patch should be sent to mainline, and then backport to v6.6.
Sorry, I was confused. This is not in mainline. It has already been fixed by 51781ce8f448 ("riscv: Pass patch_text() the length in bytes")
Indeed.
But I wouldn't backport that patch, it is bigger than necessary. The patch I sent in the previous email should be enough.
My suggested fixes are:
revert 03753bfacbc6(riscv: kprobes: Fix incorrect address calculation) apply 51781ce8f448(riscv: Pass patch_text() the length in bytes) apply 13134cc94914(riscv: kprobes: Fix incorrect address calculation)
Because stable-only commit 03753bfacbc6 is actually rebased upstream 13134cc94914, and 13134cc94914 relied on 51781ce8f448. So I gave the above suggestion. But I'm ok with your previous email patch.
Thanks a lot, laokz
On 4/25/2025 8:49 PM, Nam Cao wrote:
On Fri, Apr 25, 2025 at 08:09:21PM +0800, Kai Zhang wrote:
Hi Nam,
I reported a riscv kprobe bug of linux-6.6.y. It seems that 03753bfacbc6(riscv: kprobes: Fix incorrect address calculation) should be reverted. There are a lot of changes of riscv kprobe in upstream. I'm not all in sure of my suggested fix. Will you kind to help?
Certainly.
Thank you very much!
Thanks, laokz
On 4/25/2025 4:07 PM, Greg Kroah-Hartman wrote:
On Fri, Apr 25, 2025 at 04:03:41PM +0800, Kai Zhang wrote:
On 4/22/2025 4:46 PM, Greg Kroah-Hartman wrote:
On Tue, Apr 22, 2025 at 10:58:42AM +0800, Kai Zhang wrote:
In most recent linux-6.6.y tree, `arch/riscv/kernel/probes/kprobes.c::arch_prepare_ss_slot` still has the obsolete code:
u32 insn = __BUG_INSN_32; unsigned long offset = GET_INSN_LENGTH(p->opcode); p->ainsn.api.restore = (unsigned long)p->addr + offset; patch_text_nosync(p->ainsn.api.insn, &p->opcode, 1); patch_text_nosync((void *)p->ainsn.api.insn + offset, &insn, 1);The last two 1s are wrong size of written instructions , which would lead to kernel crash, like `insmod kprobe_example.ko` gives:
[ 509.812815][ T2734] kprobe_init: Planted kprobe at 00000000c5c46130 [ 509.837606][ C5] handler_pre: <kernel_clone> p->addr = 0x00000000c5c46130, pc = 0xffffffff80032ee2, status = 0x200000120 [ 509.839315][ C5] Oops - illegal instruction [#1]
I've tried two patchs from torvalds tree and it didn't crash again:
51781ce8f448 riscv: Pass patch_text() the length in bytes (rebased) 13134cc94914 riscv: kprobes: Fix incorrect address calculation
Please don't revert this patch. It fixes another issue.
You are correct that the sizes of the instructions are wrong. It can still happen to work if only one instruction is patched.
No. Because patch_text_nosync will write only one byte instead of one instruction(2 or 4 bytes).
This bug is not specific to v6.6. It is in mainline as well. Therefore fix patch should be sent to mainline, and then backport to v6.6.
Can you please verify if the below patch fixes your crash?
Sure. I already confirmed that the following patch indeed fixed my crash. But there is still something else I'll talk about in the next mail.
Best regards, Nam
diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c index 4fbc70e823f0..dc431b965bc3 100644 --- a/arch/riscv/kernel/probes/kprobes.c +++ b/arch/riscv/kernel/probes/kprobes.c @@ -28,8 +28,8 @@ static void __kprobes arch_prepare_ss_slot(struct kprobe *p) p->ainsn.api.restore = (unsigned long)p->addr + offset;
- patch_text_nosync(p->ainsn.api.insn, &p->opcode, 1);
- patch_text_nosync((void *)p->ainsn.api.insn + offset, &insn, 1);
- patch_text_nosync(p->ainsn.api.insn, &p->opcode, offset);
- patch_text_nosync((void *)p->ainsn.api.insn + offset, &insn, sizeof(insn)); }
static void __kprobes arch_prepare_simulate(struct kprobe *p)
Regards, laokz
linux-stable-mirror@lists.linaro.org
-
Greg Kroah-Hartman -
Kai Zhang -
Nam Cao