linux-kernel - Re: [PATCH] riscv: Disallow PR_GET_TAGGED_ADDR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <mhng-4654dddb-920a-456b-8fc9-2caaa80c4781@palmer-ri-x1c9a>
Date: Fri, 09 May 2025 09:09:54 -0700 (PDT)
From: Palmer Dabbelt <palmer@...belt.com>
To: namcao@...utronix.de
CC: samuel.holland@...ive.com, alex@...ti.fr, linux-riscv@...ts.infradead.org,
  aou@...s.berkeley.edu, bodonnel@...hat.com, Charlie Jenkins <charlie@...osinc.com>,
  Conor Dooley <conor.dooley@...rochip.com>, joel.granados@...nel.org, Paul Walmsley <paul.walmsley@...ive.com>,
  cuiyunhui@...edance.com, linux-kernel@...r.kernel.org
Subject:     Re: [PATCH] riscv: Disallow PR_GET_TAGGED_ADDR_CTRL without Supm

On Wed, 07 May 2025 11:08:11 PDT (-0700), namcao@...utronix.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@...ive.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@...utronix.de>

Thanks!