[PATCH] riscv: Disallow PR_GET_TAGGED_ADDR_CTRL without Supm

[PATCH] riscv: Disallow PR_GET_TAGGED_ADDR_CTRL without Supm

[Samuel Holland]

When the prctl() interface for pointer masking was added, it did not
check that the pointer masking ISA extension was supported, only the
individual submodes. Userspace could still attempt to disable pointer
masking and query the pointer masking state. commit 81de1afb2dd1
("riscv: Fix kernel crash due to PR_SET_TAGGED_ADDR_CTRL") disallowed
the former, as the senvcfg write could crash on older systems.
PR_GET_TAGGED_ADDR_CTRL state does not crash, because it reads only
kernel-internal state and not senvcfg, but it should still be disallowed
for consistency.

Fixes: 09d6775f503b ("riscv: Add support for userspace pointer masking")
Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

 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..f7a1a887ae68 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -330,6 +330,9 @@ long get_tagged_addr_ctrl(struct task_struct *task)
 	struct thread_info *ti = task_thread_info(task);
 	long ret = 0;
 
+	if (!riscv_has_extension_unlikely(RISCV_ISA_EXT_SUPM))
+		return -EINVAL;
+
 	if (is_compat_thread(ti))
 		return -EINVAL;
 
-- 
2.47.2

base-commit: 5bc1018675ec28a8a60d83b378d8c3991faa5a27
branch: up/get-tagged-addr

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

Re: [PATCH] riscv: Disallow PR_GET_TAGGED_ADDR_CTRL without Supm

[Nam Cao]

On Wed, May 07, 2025 at 07:52:18AM -0700, Samuel Holland wrote:
> When the prctl() interface for pointer masking was added, it did not
> check that the pointer masking ISA extension was supported, only the
> individual submodes. Userspace could still attempt to disable pointer
> masking and query the pointer masking state. commit 81de1afb2dd1
> ("riscv: Fix kernel crash due to PR_SET_TAGGED_ADDR_CTRL") disallowed
> the former, as the senvcfg write could crash on older systems.
> PR_GET_TAGGED_ADDR_CTRL state does not crash, because it reads only
> kernel-internal state and not senvcfg, but it should still be disallowed
> for consistency.
> 
> Fixes: 09d6775f503b ("riscv: Add support for userspace pointer masking")
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
> 
>  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..f7a1a887ae68 100644
> --- a/arch/riscv/kernel/process.c
> +++ b/arch/riscv/kernel/process.c
> @@ -330,6 +330,9 @@ long get_tagged_addr_ctrl(struct task_struct *task)
>  	struct thread_info *ti = task_thread_info(task);
>  	long ret = 0;
>  
> +	if (!riscv_has_extension_unlikely(RISCV_ISA_EXT_SUPM))
> +		return -EINVAL;
> +
>  	if (is_compat_thread(ti))
>  		return -EINVAL;

I think this matches what the man page says:

"If the arguments are invalid or this feature is disabled or unsupported by
the kernel, the call fails with EINVAL"

Reviewed-by: Nam Cao <namcao@linutronix.de>

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

Re: [PATCH] riscv: Disallow PR_GET_TAGGED_ADDR_CTRL without Supm

[patchwork-bot+linux-riscv]

Hello:

This patch was applied to riscv/linux.git (fixes)
by Alexandre Ghiti <alexghiti@rivosinc.com>:

On Wed,  7 May 2025 07:52:18 -0700 you wrote:
> When the prctl() interface for pointer masking was added, it did not
> check that the pointer masking ISA extension was supported, only the
> individual submodes. Userspace could still attempt to disable pointer
> masking and query the pointer masking state. commit 81de1afb2dd1
> ("riscv: Fix kernel crash due to PR_SET_TAGGED_ADDR_CTRL") disallowed
> the former, as the senvcfg write could crash on older systems.
> PR_GET_TAGGED_ADDR_CTRL state does not crash, because it reads only
> kernel-internal state and not senvcfg, but it should still be disallowed
> for consistency.
> 
> [...]

Here is the summary with links:
  - riscv: Disallow PR_GET_TAGGED_ADDR_CTRL without Supm
    https://git.kernel.org/riscv/c/7f1c3de1370b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

Re: [PATCH] riscv: Disallow PR_GET_TAGGED_ADDR_CTRL without Supm

[Palmer Dabbelt]

On Wed, 07 May 2025 11:08:11 PDT (-0700), namcao@linutronix.de wrote:
> On Wed, May 07, 2025 at 07:52:18AM -0700, Samuel Holland wrote:
>> When the prctl() interface for pointer masking was added, it did not
>> check that the pointer masking ISA extension was supported, only the
>> individual submodes. Userspace could still attempt to disable pointer
>> masking and query the pointer masking state. commit 81de1afb2dd1
>> ("riscv: Fix kernel crash due to PR_SET_TAGGED_ADDR_CTRL") disallowed
>> the former, as the senvcfg write could crash on older systems.
>> PR_GET_TAGGED_ADDR_CTRL state does not crash, because it reads only
>> kernel-internal state and not senvcfg, but it should still be disallowed
>> for consistency.

We talked some in the patchwork meeting about returning success for the 
flavors of this that could be emulated in the kernel without hardware 
support (ie, disabling pointer masking on systems that don't have it is 
trivial).  The general consensus is that didn't really help any, as 
userspace still needs to deal with systems where these will fail (old 
kernels, Kconfig disabled, etc).

Happy to reconsider that if someone cares.  Either way this is going up 
as a fix, as it just keeps the interface sane when taking 7f1c3de1370b 
("riscv: Disallow PR_GET_TAGGED_ADDR_CTRL without Supm") into account.

>> Fixes: 09d6775f503b ("riscv: Add support for userspace pointer masking")
>> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
>> ---
>>
>>  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..f7a1a887ae68 100644
>> --- a/arch/riscv/kernel/process.c
>> +++ b/arch/riscv/kernel/process.c
>> @@ -330,6 +330,9 @@ long get_tagged_addr_ctrl(struct task_struct *task)
>>  	struct thread_info *ti = task_thread_info(task);
>>  	long ret = 0;
>>
>> +	if (!riscv_has_extension_unlikely(RISCV_ISA_EXT_SUPM))
>> +		return -EINVAL;
>> +
>>  	if (is_compat_thread(ti))
>>  		return -EINVAL;
>
> I think this matches what the man page says:
>
> "If the arguments are invalid or this feature is disabled or unsupported by
> the kernel, the call fails with EINVAL"

Ya, it's also what we do when the Kconfig is disabled so it seems like 
the way to go (we talked about that some too).

> Reviewed-by: Nam Cao <namcao@linutronix.de>

Thanks!

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