When userspace does PR_SET_TAGGED_ADDR_CTRL, but Supm extension is not available, the kernel crashes:
Oops - illegal instruction [#1] [snip] epc : set_tagged_addr_ctrl+0x112/0x15a ra : set_tagged_addr_ctrl+0x74/0x15a epc : ffffffff80011ace ra : ffffffff80011a30 sp : ffffffc60039be10 [snip] status: 0000000200000120 badaddr: 0000000010a79073 cause: 0000000000000002 set_tagged_addr_ctrl+0x112/0x15a __riscv_sys_prctl+0x352/0x73c do_trap_ecall_u+0x17c/0x20c andle_exception+0x150/0x15c
Fix it by checking if Supm is available.
Fixes: 09d6775f503b ("riscv: Add support for userspace pointer masking") Signed-off-by: Nam Cao namcao@linutronix.de Cc: stable@vger.kernel.org --- arch/riscv/kernel/process.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c index 7c244de77180..3db2c0c07acd 100644 --- a/arch/riscv/kernel/process.c +++ b/arch/riscv/kernel/process.c @@ -275,6 +275,9 @@ long set_tagged_addr_ctrl(struct task_struct *task, unsigned long arg) unsigned long pmm; u8 pmlen;
+ if (!riscv_has_extension_unlikely(RISCV_ISA_EXT_SUPM)) + return -EINVAL; + if (is_compat_thread(ti)) return -EINVAL;
Hi Nam,
On 04/05/2025 12:19, Nam Cao wrote:
When userspace does PR_SET_TAGGED_ADDR_CTRL, but Supm extension is not available, the kernel crashes:
Oops - illegal instruction [#1] [snip] epc : set_tagged_addr_ctrl+0x112/0x15a ra : set_tagged_addr_ctrl+0x74/0x15a epc : ffffffff80011ace ra : ffffffff80011a30 sp : ffffffc60039be10 [snip] status: 0000000200000120 badaddr: 0000000010a79073 cause: 0000000000000002 set_tagged_addr_ctrl+0x112/0x15a __riscv_sys_prctl+0x352/0x73c do_trap_ecall_u+0x17c/0x20c andle_exception+0x150/0x15c
It seems like the csr write is triggering this illegal instruction, can you confirm it is? If so, I can't find in the specification that an implementation should do that when writing envcfg and I can't reproduce it on qemu. Where did you see this oops?
Thanks,
Alex
Fix it by checking if Supm is available.
Fixes: 09d6775f503b ("riscv: Add support for userspace pointer masking") Signed-off-by: Nam Cao namcao@linutronix.de Cc: stable@vger.kernel.org
arch/riscv/kernel/process.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c index 7c244de77180..3db2c0c07acd 100644 --- a/arch/riscv/kernel/process.c +++ b/arch/riscv/kernel/process.c @@ -275,6 +275,9 @@ long set_tagged_addr_ctrl(struct task_struct *task, unsigned long arg) unsigned long pmm; u8 pmlen;
- if (!riscv_has_extension_unlikely(RISCV_ISA_EXT_SUPM))
return -EINVAL;
- if (is_compat_thread(ti)) return -EINVAL;
Hi Alex,
On Mon, May 05, 2025 at 06:02:26PM +0200, Alexandre Ghiti wrote:
On 04/05/2025 12:19, Nam Cao wrote:
When userspace does PR_SET_TAGGED_ADDR_CTRL, but Supm extension is not available, the kernel crashes:
Oops - illegal instruction [#1] [snip] epc : set_tagged_addr_ctrl+0x112/0x15a ra : set_tagged_addr_ctrl+0x74/0x15a epc : ffffffff80011ace ra : ffffffff80011a30 sp : ffffffc60039be10 [snip] status: 0000000200000120 badaddr: 0000000010a79073 cause: 0000000000000002 set_tagged_addr_ctrl+0x112/0x15a __riscv_sys_prctl+0x352/0x73c do_trap_ecall_u+0x17c/0x20c andle_exception+0x150/0x15c
It seems like the csr write is triggering this illegal instruction, can you confirm it is?
Yes, it is the "csr_write(CSR_ENVCFG, envcfg);" in envcfg_update_bits().
If so, I can't find in the specification that an implementation should do that when writing envcfg and I can't reproduce it on qemu. Where did you see this oops?
I can't find it in the spec either. I think it is up to the implementation.
I got this crash on the MangoPI board: https://mangopi.org/mqpro
Best regards, Nam
On 05/05/2025 18:07, Nam Cao wrote:
Hi Alex,
On Mon, May 05, 2025 at 06:02:26PM +0200, Alexandre Ghiti wrote:
On 04/05/2025 12:19, Nam Cao wrote:
When userspace does PR_SET_TAGGED_ADDR_CTRL, but Supm extension is not available, the kernel crashes:
Oops - illegal instruction [#1] [snip] epc : set_tagged_addr_ctrl+0x112/0x15a ra : set_tagged_addr_ctrl+0x74/0x15a epc : ffffffff80011ace ra : ffffffff80011a30 sp : ffffffc60039be10 [snip] status: 0000000200000120 badaddr: 0000000010a79073 cause: 0000000000000002 set_tagged_addr_ctrl+0x112/0x15a __riscv_sys_prctl+0x352/0x73c do_trap_ecall_u+0x17c/0x20c andle_exception+0x150/0x15c
It seems like the csr write is triggering this illegal instruction, can you confirm it is?
Yes, it is the "csr_write(CSR_ENVCFG, envcfg);" in envcfg_update_bits().
If so, I can't find in the specification that an implementation should do that when writing envcfg and I can't reproduce it on qemu. Where did you see this oops?
I can't find it in the spec either. I think it is up to the implementation.
The reserved fields of senvcfg are WPRI and contrary to WLRL, it does not explicitly "permit" to raise an illegal instruction so I'd say it is not up to the implementation, I'll ask around.
Thanks,
Alex
I got this crash on the MangoPI board: https://mangopi.org/mqpro
Best regards, Nam
linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
On 05/05/2025 21:27, Alexandre Ghiti wrote:
On 05/05/2025 18:07, Nam Cao wrote:
Hi Alex,
On Mon, May 05, 2025 at 06:02:26PM +0200, Alexandre Ghiti wrote:
On 04/05/2025 12:19, Nam Cao wrote:
When userspace does PR_SET_TAGGED_ADDR_CTRL, but Supm extension is not available, the kernel crashes:
Oops - illegal instruction [#1] [snip] epc : set_tagged_addr_ctrl+0x112/0x15a ra : set_tagged_addr_ctrl+0x74/0x15a epc : ffffffff80011ace ra : ffffffff80011a30 sp : ffffffc60039be10 [snip] status: 0000000200000120 badaddr: 0000000010a79073 cause: 0000000000000002 set_tagged_addr_ctrl+0x112/0x15a __riscv_sys_prctl+0x352/0x73c do_trap_ecall_u+0x17c/0x20c andle_exception+0x150/0x15c
It seems like the csr write is triggering this illegal instruction, can you confirm it is?
Yes, it is the "csr_write(CSR_ENVCFG, envcfg);" in envcfg_update_bits().
If so, I can't find in the specification that an implementation should do that when writing envcfg and I can't reproduce it on qemu. Where did you see this oops?
I can't find it in the spec either. I think it is up to the implementation.
The reserved fields of senvcfg are WPRI and contrary to WLRL, it does not explicitly "permit" to raise an illegal instruction so I'd say it is not up to the implementation, I'll ask around.
So I had confirmation that WPRI should not raise an illegal instruction so that's an issue with the platform. Your patch is not wrong but I'd rather have an explicit errata, what do you think?
Thanks,
Alex
Thanks,
Alex
I got this crash on the MangoPI board: https://mangopi.org/mqpro
Best regards, Nam
linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
On 2025-05-06 11:31 AM, Alexandre Ghiti wrote:
On 05/05/2025 21:27, Alexandre Ghiti wrote:
On 05/05/2025 18:07, Nam Cao wrote:
Hi Alex,
On Mon, May 05, 2025 at 06:02:26PM +0200, Alexandre Ghiti wrote:
On 04/05/2025 12:19, Nam Cao wrote:
When userspace does PR_SET_TAGGED_ADDR_CTRL, but Supm extension is not available, the kernel crashes:
Oops - illegal instruction [#1] [snip] epc : set_tagged_addr_ctrl+0x112/0x15a ra : set_tagged_addr_ctrl+0x74/0x15a epc : ffffffff80011ace ra : ffffffff80011a30 sp : ffffffc60039be10 [snip] status: 0000000200000120 badaddr: 0000000010a79073 cause: 0000000000000002 set_tagged_addr_ctrl+0x112/0x15a __riscv_sys_prctl+0x352/0x73c do_trap_ecall_u+0x17c/0x20c andle_exception+0x150/0x15c
It seems like the csr write is triggering this illegal instruction, can you confirm it is?
Yes, it is the "csr_write(CSR_ENVCFG, envcfg);" in envcfg_update_bits().
If so, I can't find in the specification that an implementation should do that when writing envcfg and I can't reproduce it on qemu. Where did you see this oops?
I can't find it in the spec either. I think it is up to the implementation.
The reserved fields of senvcfg are WPRI and contrary to WLRL, it does not explicitly "permit" to raise an illegal instruction so I'd say it is not up to the implementation, I'll ask around.
So I had confirmation that WPRI should not raise an illegal instruction so that's an issue with the platform. Your patch is not wrong but I'd rather have an explicit errata, what do you think?
There is no erratum here. Allwinner D1 / T-HEAD C906 implements Ss1p11, which was before senvcfg was added to the privileged architecture, and does not implement any of the extensions which would imply senvcfg's existence, so the CSR is reserved. We could check for Xlinuxenvcfg to determine if the CSR access will raise an exception, but that does not gain anything over checking for Supm specifically. So the fix is correct.
Reviewed-by: Samuel Holland samuel.holland@sifive.com
That said, I wonder if set_tagged_addr_ctrl(task, 0) should succeed when Supm is not implemented, matching get_tagged_addr_ctrl(). Without Supm, we know that have_user_pmlen_7 and have_user_pmlen_16 will both be false, so pmlen == 0 is the only case where we would call envcfg_update_bits(). And we know it would be a no-op. So an alternative fix would be to return 0 below the pmlen checks:
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c index 7c244de77180..536da9aa690e 100644 --- a/arch/riscv/kernel/process.c +++ b/arch/riscv/kernel/process.c @@ -309,6 +309,9 @@ long set_tagged_addr_ctrl(struct task_struct *task, unsigned long arg) if (!(arg & PR_TAGGED_ADDR_ENABLE)) pmlen = PMLEN_0;
+ if (!riscv_has_extension_unlikely(RISCV_ISA_EXT_SUPM)) + return 0; + if (mmap_write_lock_killable(mm)) return -EINTR;
But I don't know if this better matches what userspace would expect.
Regards, Samuel
I got this crash on the MangoPI board: https://mangopi.org/mqpro
Best regards, Nam
On Tue, May 06, 2025 at 05:29:57PM -0500, Samuel Holland wrote:
That said, I wonder if set_tagged_addr_ctrl(task, 0) should succeed when Supm is not implemented, matching get_tagged_addr_ctrl(). Without Supm, we know that have_user_pmlen_7 and have_user_pmlen_16 will both be false, so pmlen == 0 is the only case where we would call envcfg_update_bits(). And we know it would be a no-op. So an alternative fix would be to return 0 below the pmlen checks:
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c index 7c244de77180..536da9aa690e 100644 --- a/arch/riscv/kernel/process.c +++ b/arch/riscv/kernel/process.c @@ -309,6 +309,9 @@ long set_tagged_addr_ctrl(struct task_struct *task, unsigned long arg) if (!(arg & PR_TAGGED_ADDR_ENABLE)) pmlen = PMLEN_0;
- if (!riscv_has_extension_unlikely(RISCV_ISA_EXT_SUPM))
return 0;
- if (mmap_write_lock_killable(mm)) return -EINTR;
But I don't know if this better matches what userspace would expect.
I'm not sure about this either. The man page says:
|If the arguments are invalid, the mode specified in arg2 is |unrecognized, or if this feature is unsupported by the kernel or disabled |via /proc/sys/abi/tagged_addr_disabled, the call fails with the error |EINVAL. | |In particular, if prctl(PR_SET_TAGGED_ADDR_CTRL, 0, 0, 0, 0) fails with |EINVAL, then all addresses passed to the kernel must be untagged.
So according to the man page, returning -EINVAL is the right thing.
But arm64 returns 0 in this case.
I would say let's follow the man page, and leave it as is.
Best regards, Nam
Hello:
This patch was applied to riscv/linux.git (fixes) by Alexandre Ghiti alexghiti@rivosinc.com:
On Sun, 4 May 2025 12:19:20 +0200 you wrote:
When userspace does PR_SET_TAGGED_ADDR_CTRL, but Supm extension is not available, the kernel crashes:
Oops - illegal instruction [#1] [snip] epc : set_tagged_addr_ctrl+0x112/0x15a ra : set_tagged_addr_ctrl+0x74/0x15a epc : ffffffff80011ace ra : ffffffff80011a30 sp : ffffffc60039be10 [snip] status: 0000000200000120 badaddr: 0000000010a79073 cause: 0000000000000002 set_tagged_addr_ctrl+0x112/0x15a __riscv_sys_prctl+0x352/0x73c do_trap_ecall_u+0x17c/0x20c andle_exception+0x150/0x15c
[...]
Here is the summary with links: - riscv: Fix kernel crash due to PR_SET_TAGGED_ADDR_CTRL https://git.kernel.org/riscv/c/ae08d55807c0
You are awesome, thank you!
linux-stable-mirror@lists.linaro.org