linux-kernel - Re: [PATCH v2 4/6] riscv: vector: allow to force vector context save

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: <CAFTtA3NoOZEMqYD6+vjP=09T15GiThjVy1LeDX0U8CC-4HMKOA@mail.gmail.com>
Date: Wed, 15 Oct 2025 16:32:05 -0500
From: Andy Chiu <andybnac@...il.com>
To: Sergey Matyukevich <geomatsi@...il.com>
Cc: linux-riscv@...ts.infradead.org, linux-kselftest@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Paul Walmsley <pjw@...nel.org>, 
	Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>, 
	Alexandre Ghiti <alex@...ti.fr>, Oleg Nesterov <oleg@...hat.com>, Shuah Khan <shuah@...nel.org>, 
	Jisheng Zhang <jszhang@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, Thomas Huth <thuth@...hat.com>, 
	Charlie Jenkins <charlie@...osinc.com>, Han Gao <rabenda.cn@...il.com>, 
	Samuel Holland <samuel.holland@...ive.com>, Nam Cao <namcao@...utronix.de>, 
	Joel Granados <joel.granados@...nel.org>, Clément Léger <cleger@...osinc.com>, 
	Conor Dooley <conor.dooley@...rochip.com>
Subject: Re: [PATCH v2 4/6] riscv: vector: allow to force vector context save

On Wed, Oct 15, 2025 at 3:18 PM Andy Chiu <andybnac@...il.com> wrote:
>
> On Tue, Oct 7, 2025 at 6:58 AM Sergey Matyukevich <geomatsi@...il.com> wrote:
> >
> > When ptrace updates vector CSR registers for a traced process, the
> > changes may not be immediately visible to the next ptrace operations
> > due to vector context switch optimizations.
> >
> > The function 'riscv_v_vstate_save' saves context only if mstatus.VS is
> > 'dirty'. However mstatus.VS of the traced process context may remain
> > 'clean' between two breakpoints, if no vector instructions were executed
> > between those two breakpoints. In this case the vector context will not
> > be saved at the second breakpoint. As a result, the second ptrace may
> > read stale vector CSR values.
>
> IIUC, the second ptrace should not get the stale vector CSR values.
> The second riscv_vr_get() should be reading from the context memory
> (vstate), which is updated from the last riscv_vr_set(). The user's
> vstate should remain the same since last riscv_vr_set(). Could you
> explain more on how this bug is observed and why only CSRs are
> affected but not v-regs as well?

>From looking into your test, I can see that you were trying to set an
invalid configuration to Vetor CSRs and expect vill to be reflected
upon next read. Yes, this is not happening on the current
implementation as it was not expecting invalid input from the user,
which should be taken into consideration. Thanks for spotting the
case!

According to the spec, "The use of vtype encodings with LMUL <
SEWMIN/ELEN is reserved, implementations can set vill if they do not
support these configurations." This mean the implementation may
actually support this configuration. If that is the case, I think we
should not allow this to be configured through the vector ptrace
interface, which is designed to support 1.0 (and 0.7) specs. That
means, we should not allow this problematic configuration to pass
through riscv_vr_set(), reach user space, then the forced save.

I would opt for validating all CSR configurations in the first place.
Could you also help enforce checks on other reserved bits as well?

Thanks,
Andy

>
> Thanks,
> Andy
>
> >
> > Fix this by introducing a TIF flag that forces vector context save on
> > the next context switch, regardless of mstatus.VS state. Set this
> > flag on ptrace oprations that modify vector CSR registers.
> >
> > Signed-off-by: Sergey Matyukevich <geomatsi@...il.com>
> > ---
> >  arch/riscv/include/asm/thread_info.h | 2 ++
> >  arch/riscv/include/asm/vector.h      | 3 +++
> >  arch/riscv/kernel/process.c          | 2 ++
> >  arch/riscv/kernel/ptrace.c           | 5 +++++
> >  4 files changed, 12 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
> > index 836d80dd2921..e05e9aa89c43 100644
> > --- a/arch/riscv/include/asm/thread_info.h
> > +++ b/arch/riscv/include/asm/thread_info.h
> > @@ -118,7 +118,9 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
> >
> >  #define TIF_32BIT                      16      /* compat-mode 32bit process */
> >  #define TIF_RISCV_V_DEFER_RESTORE      17      /* restore Vector before returing to user */
> > +#define TIF_RISCV_V_FORCE_SAVE         13      /* force Vector context save */
> >
> >  #define _TIF_RISCV_V_DEFER_RESTORE     BIT(TIF_RISCV_V_DEFER_RESTORE)
> > +#define _TIF_RISCV_V_FORCE_SAVE                BIT(TIF_RISCV_V_FORCE_SAVE)
> >
> >  #endif /* _ASM_RISCV_THREAD_INFO_H */
> > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> > index b61786d43c20..d3770e13da93 100644
> > --- a/arch/riscv/include/asm/vector.h
> > +++ b/arch/riscv/include/asm/vector.h
> > @@ -370,6 +370,9 @@ static inline void __switch_to_vector(struct task_struct *prev,
> >  {
> >         struct pt_regs *regs;
> >
> > +       if (test_and_clear_tsk_thread_flag(prev, TIF_RISCV_V_FORCE_SAVE))
> > +               __riscv_v_vstate_dirty(task_pt_regs(prev));
> > +
> >         if (riscv_preempt_v_started(prev)) {
> >                 if (riscv_v_is_on()) {
> >                         WARN_ON(prev->thread.riscv_v_flags & RISCV_V_CTX_DEPTH_MASK);
> > diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
> > index 31a392993cb4..47959c55cefb 100644
> > --- a/arch/riscv/kernel/process.c
> > +++ b/arch/riscv/kernel/process.c
> > @@ -183,6 +183,7 @@ void flush_thread(void)
> >         kfree(current->thread.vstate.datap);
> >         memset(&current->thread.vstate, 0, sizeof(struct __riscv_v_ext_state));
> >         clear_tsk_thread_flag(current, TIF_RISCV_V_DEFER_RESTORE);
> > +       clear_tsk_thread_flag(current, TIF_RISCV_V_FORCE_SAVE);
> >  #endif
> >  #ifdef CONFIG_RISCV_ISA_SUPM
> >         if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SUPM))
> > @@ -205,6 +206,7 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
> >         memset(&dst->thread.vstate, 0, sizeof(struct __riscv_v_ext_state));
> >         memset(&dst->thread.kernel_vstate, 0, sizeof(struct __riscv_v_ext_state));
> >         clear_tsk_thread_flag(dst, TIF_RISCV_V_DEFER_RESTORE);
> > +       clear_tsk_thread_flag(dst, TIF_RISCV_V_FORCE_SAVE);
> >
> >         return 0;
> >  }
> > diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
> > index 906cf1197edc..569f756bef23 100644
> > --- a/arch/riscv/kernel/ptrace.c
> > +++ b/arch/riscv/kernel/ptrace.c
> > @@ -148,6 +148,11 @@ static int riscv_vr_set(struct task_struct *target,
> >         if (vstate->vlenb != ptrace_vstate.vlenb)
> >                 return -EINVAL;
> >
> > +       if (vstate->vtype != ptrace_vstate.vtype ||
> > +           vstate->vcsr != ptrace_vstate.vcsr ||
> > +           vstate->vl != ptrace_vstate.vl)
> > +               set_tsk_thread_flag(target, TIF_RISCV_V_FORCE_SAVE);
> > +
> >         vstate->vstart = ptrace_vstate.vstart;
> >         vstate->vl = ptrace_vstate.vl;
> >         vstate->vtype = ptrace_vstate.vtype;
> > --
> > 2.51.0
> >