linux-kernel - Re: [PATCH] riscv: Add sysctl to control discard of vstate during syscall

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aH6uPi+UOP6GzNjv@x1>
Date: Mon, 21 Jul 2025 14:16:46 -0700
From: Drew Fustini <fustini@...nel.org>
To: Radim Krčmář <rkrcmar@...tanamicro.com>
Cc: Palmer Dabbelt <palmer@...belt.com>,
	Björn Töpel <bjorn@...osinc.com>,
	Alexandre Ghiti <alex@...ti.fr>,
	Paul Walmsley <paul.walmsley@...ive.com>,
	Samuel Holland <samuel.holland@...ive.com>,
	Drew Fustini <dfustini@...storrent.com>,
	Andy Chiu <andybnac@...il.com>,
	Conor Dooley <conor.dooley@...rochip.com>,
	linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linux-riscv <linux-riscv-bounces@...ts.infradead.org>
Subject: Re: [PATCH] riscv: Add sysctl to control discard of vstate during
 syscall

On Mon, Jul 21, 2025 at 02:35:38PM +0200, Radim Krčmář wrote:
> 2025-07-18T20:39:13-07:00, Drew Fustini <fustini@...nel.org>:
> > From: Drew Fustini <dfustini@...storrent.com>
> >
> > Clobbering the vector registers can significantly increase system call
> > latency for some implementations. To mitigate this performance impact, a
> > policy mechanism is provided to administrators, distro maintainers, and
> > developers to control vector state discard in the form of a sysctl knob:
> >
> > /proc/sys/abi/riscv_v_vstate_discard
> >
> > Valid values are:
> >
> > 0: Do not discard vector state during syscall
> > 1: Discard vector state during syscall
> >
> > The initial state is controlled by CONFIG_RISCV_ISA_V_VSTATE_DISCARD.
> 
> I think it is a bit more complicated to do this nicely...
> Programs don't have to save/restore vector registers around syscalls
> when compiled for riscv_v_vstate_discard=0, so running under
> riscv_v_vstate_discard=1 would break them.

Thanks for your comments. You raise a good point that this sysctl can
lead to the case where a program might be compiled to not save/restore
vector registers around syscalls. That same program would not work
correctly if the sysadmin changes riscv_v_vstate_discard to 1.

> Shouldn't we have a way to prevent riscv_v_vstate_discard=0 executable
> from running with riscv_v_vstate_discard=1?

Yes, this does make me concerned that a program could crash as a result
of this sysctl which would be confusing for the user as they may not
even be aware of this sysctl. I'll have to think more about how such a
protection could work.

> 
> > Fixes: 9657e9b7d253 ("riscv: Discard vector state on syscalls")
> 
> Programs compiled for riscv_v_vstate_discard=1 are compatible with 0, so
> I think it would be simplest to revert that patch, and pretended it
> never happened... (The issues will eventually go away.)

I agree that reverting the existing discard behavior would be the
simplest solution to the peformance issue observed on some
implementations. However, I believe there is also the desire to have a
way to enforce strict clobbering across syscalls to catch any incorrect
behavior while testing. I was hoping a syscall could allow both use
cases to be handled, but you raise good points about compatibility.

> Shouldn't the RISC-V Linux syscall ABI be defined somewhere?
> How come we could have broken it with 9657e9b7d253?

I may have been wrong to use a Fixes tag for 9657e9b7d253. I was trying
to highlight the original discussion that I was trying to address with
this sysctl patch.

> 
> Thanks.
> 
> ---
> I don't think it makes much sense to clobber vector registers on a
> syscall -- a kernel might not even touch vector registers, so they are
> efforlessly preserved in that case.
> If kernel needs to use vector registers in the syscall, then the kernel
> needs to prevent any register leaks to userspace anyway by restoring
> some state into them -- and why not restore the original one?
> 
> I think that main point of clobbering would be to optimize
> context-switches after the userspace is not using vector registers
> anymore, but it's terribly inefficient if the ratio of syscalls to
> context switches is high.
> Linux can also try to detect the situation, and turn to lazy vector
> context-switch, with sstatus.VS=off, instead of eagerly restoring
> clobbered state.
> (A good indicator might be that the userspace hasn't dirtied the vectors
>  since the last context-switch -- kernel didn't need to save the state,
>  so it will restore lazily.)

I think this is an interesting discussion to have. I was hoping this
patch would get people discussing if mandatory vector state cloberring
is really something that should be do in syscalls.

Thanks,
Drew