Re: [PATCH] riscv: Fix kernel crash due to PR_SET_TAGGED_ADDR_CTRL

[Nam Cao <namcao@linutronix.de>]

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

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