[PATCH v2 0/6] riscv: vector: misc ptrace fixes for debug use-cases

[PATCH v2 0/6] riscv: vector: misc ptrace fixes for debug use-cases

[Sergey Matyukevich]

This patch series suggests fixes for several corner cases in the RISC-V
vector ptrace implementation:

- follow gdbserver expectations and return ENODATA instead of EINVAL if vector
  extension is supported but not yet activated for a traced process

- force vector context save on the next context switch after ptrace call that
  modified vector CSRs, to avoid reading stale values by the next ptrace calls

- force vector context save on the first context switch after vector context
  initialization, to avoid reading zero vlenb by an early attached debugger

For detailed description see the appropriate commit messages. A new test is
added into the tools/testing/selftests/riscv/vector to verify the fixes.
Each fix is accompanied by its own test case.

Initial version [1] of this series included only the last fix for zero vlenb.

[1] https://lore.kernel.org/linux-riscv/20250821173957.563472-1-geomatsi@gmail.com/T/#u


Ilya Mamay (1):
  riscv: ptrace: return ENODATA for inactive vector extension

Sergey Matyukevich (5):
  selftests: riscv: test ptrace vector interface
  selftests: riscv: set invalid vtype using ptrace
  riscv: vector: allow to force vector context save
  selftests: riscv: verify initial vector state with ptrace
  riscv: vector: initialize vlenb on the first context switch

 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                    |  15 +-
 arch/riscv/kernel/vector.c                    |   4 +
 .../testing/selftests/riscv/vector/.gitignore |   1 +
 tools/testing/selftests/riscv/vector/Makefile |   5 +-
 .../testing/selftests/riscv/vector/v_ptrace.c | 302 ++++++++++++++++++
 8 files changed, 331 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/riscv/vector/v_ptrace.c


base-commit: c746c3b5169831d7fb032a1051d8b45592ae8d78
-- 
2.51.0


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

[PATCH v2 1/6] selftests: riscv: test ptrace vector interface

[Sergey Matyukevich]

Add a test case to check ptrace behavior in the case when vector
extension is supported by the system, but vector context is not
yet enabled for the traced process.

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
---
 .../testing/selftests/riscv/vector/.gitignore |  1 +
 tools/testing/selftests/riscv/vector/Makefile |  5 +-
 .../testing/selftests/riscv/vector/v_ptrace.c | 87 +++++++++++++++++++
 3 files changed, 92 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/riscv/vector/v_ptrace.c

diff --git a/tools/testing/selftests/riscv/vector/.gitignore b/tools/testing/selftests/riscv/vector/.gitignore
index 7d9c87cd0649..d21c03c3ee0e 100644
--- a/tools/testing/selftests/riscv/vector/.gitignore
+++ b/tools/testing/selftests/riscv/vector/.gitignore
@@ -2,3 +2,4 @@ vstate_exec_nolibc
 vstate_prctl
 v_initval
 v_exec_initval_nolibc
+v_ptrace
diff --git a/tools/testing/selftests/riscv/vector/Makefile b/tools/testing/selftests/riscv/vector/Makefile
index 6f7497f4e7b3..c14ad127e7fb 100644
--- a/tools/testing/selftests/riscv/vector/Makefile
+++ b/tools/testing/selftests/riscv/vector/Makefile
@@ -2,7 +2,7 @@
 # Copyright (C) 2021 ARM Limited
 # Originally tools/testing/arm64/abi/Makefile
 
-TEST_GEN_PROGS := v_initval vstate_prctl
+TEST_GEN_PROGS := v_initval vstate_prctl v_ptrace
 TEST_GEN_PROGS_EXTENDED := vstate_exec_nolibc v_exec_initval_nolibc
 
 include ../../lib.mk
@@ -26,3 +26,6 @@ $(OUTPUT)/v_initval: v_initval.c $(OUTPUT)/sys_hwprobe.o $(OUTPUT)/v_helpers.o
 $(OUTPUT)/v_exec_initval_nolibc: v_exec_initval_nolibc.c
 	$(CC) -nostdlib -static -include ../../../../include/nolibc/nolibc.h \
 		-Wall $(CFLAGS) $(LDFLAGS) $^ -o $@ -lgcc
+
+$(OUTPUT)/v_ptrace: v_ptrace.c $(OUTPUT)/sys_hwprobe.o $(OUTPUT)/v_helpers.o
+	$(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
diff --git a/tools/testing/selftests/riscv/vector/v_ptrace.c b/tools/testing/selftests/riscv/vector/v_ptrace.c
new file mode 100644
index 000000000000..6a8d56a5c4f4
--- /dev/null
+++ b/tools/testing/selftests/riscv/vector/v_ptrace.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <sys/ptrace.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <sys/wait.h>
+#include <sys/uio.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include <linux/ptrace.h>
+#include <linux/elf.h>
+
+#include "../../kselftest_harness.h"
+#include "v_helpers.h"
+
+volatile unsigned long chld_lock;
+
+TEST(ptrace_rvv_not_enabled)
+{
+	pid_t pid;
+
+	if (!is_vector_supported())
+		SKIP(return, "Vector not supported");
+
+	chld_lock = 1;
+
+	pid = fork();
+
+	ASSERT_LE(0, pid)
+		TH_LOG("fork: %m");
+
+	if (pid == 0) {
+		while (chld_lock == 1)
+			asm volatile("" : : "g"(chld_lock) : "memory");
+
+		asm volatile ("ebreak" : : : );
+	} else {
+		struct __riscv_v_regset_state *regset_data;
+		unsigned long vlenb;
+		size_t regset_size;
+		struct iovec iov;
+		int status;
+		int ret;
+
+		asm volatile("csrr %[vlenb], vlenb" : [vlenb] "=r"(vlenb));
+
+		ASSERT_GT(vlenb, 0)
+			TH_LOG("vlenb is not valid: %lu\n", vlenb);
+
+		/* attach */
+
+		ASSERT_EQ(0, ptrace(PTRACE_ATTACH, pid, NULL, NULL));
+		ASSERT_EQ(pid, waitpid(pid, &status, 0));
+		ASSERT_TRUE(WIFSTOPPED(status));
+
+		/* unlock */
+
+		ASSERT_EQ(0, ptrace(PTRACE_POKEDATA, pid, &chld_lock, 0));
+
+		/* resume and wait for ebreak */
+
+		ASSERT_EQ(0, ptrace(PTRACE_CONT, pid, NULL, NULL));
+		ASSERT_EQ(pid, waitpid(pid, &status, 0));
+		ASSERT_TRUE(WIFSTOPPED(status));
+
+		/* try to read vector registers from the tracee */
+
+		regset_size = sizeof(*regset_data) + vlenb * 32;
+		regset_data = calloc(1, regset_size);
+
+		iov.iov_base = regset_data;
+		iov.iov_len = regset_size;
+
+		/* V extension is available, but not yet enabled for the tracee */
+
+		errno = 0;
+		ret = ptrace(PTRACE_GETREGSET, pid, NT_RISCV_VECTOR, &iov);
+		ASSERT_EQ(ENODATA, errno);
+		ASSERT_EQ(-1, ret);
+
+		/* cleanup */
+
+		ASSERT_EQ(0, kill(pid, SIGKILL));
+	}
+}
+
+TEST_HARNESS_MAIN
-- 
2.51.0


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

[PATCH v2 2/6] riscv: ptrace: return ENODATA for inactive vector extension

[Sergey Matyukevich]

From: Ilya Mamay <mmamayka01@gmail.com>

Currently, ptrace returns EINVAL when the vector extension is supported
but not yet activated for the traced process. This error code is
inappropriate since all the ptrace arguments are valid.

Debug tools like gdbserver expect ENODATA when the requested register
set is not active, e.g. see [1]. This expectation seems to be more
appropriate, so modify the vector ptrace implementation to return:
- EINVAL when V extension is not supported
- ENODATA when V extension is supported but not active

[1] https://github.com/bminor/binutils-gdb/blob/637f25e88675fa47e47f9cc5e2cf37384836b8a2/gdbserver/linux-low.cc#L5020

Signed-off-by: Ilya Mamay <mmamayka01@gmail.com>
---
 arch/riscv/kernel/ptrace.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
index 8e86305831ea..906cf1197edc 100644
--- a/arch/riscv/kernel/ptrace.c
+++ b/arch/riscv/kernel/ptrace.c
@@ -95,9 +95,12 @@ static int riscv_vr_get(struct task_struct *target,
 	struct __riscv_v_ext_state *vstate = &target->thread.vstate;
 	struct __riscv_v_regset_state ptrace_vstate;
 
-	if (!riscv_v_vstate_query(task_pt_regs(target)))
+	if (!has_vector())
 		return -EINVAL;
 
+	if (!riscv_v_vstate_query(task_pt_regs(target)))
+		return -ENODATA;
+
 	/*
 	 * Ensure the vector registers have been saved to the memory before
 	 * copying them to membuf.
@@ -130,9 +133,12 @@ static int riscv_vr_set(struct task_struct *target,
 	struct __riscv_v_ext_state *vstate = &target->thread.vstate;
 	struct __riscv_v_regset_state ptrace_vstate;
 
-	if (!riscv_v_vstate_query(task_pt_regs(target)))
+	if (!has_vector())
 		return -EINVAL;
 
+	if (!riscv_v_vstate_query(task_pt_regs(target)))
+		return -ENODATA;
+
 	/* Copy rest of the vstate except datap */
 	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &ptrace_vstate, 0,
 				 sizeof(struct __riscv_v_regset_state));
-- 
2.51.0


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

[PATCH v2 3/6] selftests: riscv: set invalid vtype using ptrace

[Sergey Matyukevich]

Add a test case that attempts to set invalid vtype value using ptrace
and verifies that the 'vill' bit is set as required by the RISC-V
Vector specification v1.0, Section 3.4.1.

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
---
 .../testing/selftests/riscv/vector/v_ptrace.c | 112 ++++++++++++++++++
 1 file changed, 112 insertions(+)

diff --git a/tools/testing/selftests/riscv/vector/v_ptrace.c b/tools/testing/selftests/riscv/vector/v_ptrace.c
index 6a8d56a5c4f4..ccda8a4dc49b 100644
--- a/tools/testing/selftests/riscv/vector/v_ptrace.c
+++ b/tools/testing/selftests/riscv/vector/v_ptrace.c
@@ -84,4 +84,116 @@ TEST(ptrace_rvv_not_enabled)
 	}
 }
 
+TEST(ptrace_rvv_invalid_vtype)
+{
+	static volatile unsigned long vtype;
+	unsigned long vlenb;
+	unsigned long reg;
+	pid_t pid;
+
+	if (!is_vector_supported())
+		SKIP(return, "Vector not supported");
+
+	asm volatile("csrr %[vlenb], vlenb" : [vlenb] "=r"(vlenb));
+
+	if (vlenb > 16)
+		SKIP(return, "This test does not support VLEN > 128");
+
+	chld_lock = 1;
+
+	pid = fork();
+
+	ASSERT_LE(0, pid)
+		TH_LOG("fork: %m");
+
+	if (pid == 0) {
+		while (chld_lock == 1)
+			asm volatile("" : : "g"(chld_lock) : "memory");
+
+		asm(".option arch, +v\n");
+		asm(".option arch, +c\n");
+		asm volatile("vsetvli x0, x0, e8, m8, tu, mu\n");
+
+		while (1) {
+			asm volatile ("c.ebreak");
+			asm volatile("csrr %[vtype], vtype" : [vtype] "=r"(vtype) : :);
+			asm volatile ("c.ebreak");
+		}
+	} else {
+		struct __riscv_v_regset_state *regset_data;
+		struct user_regs_struct regs;
+		size_t regset_size;
+		struct iovec iov;
+		int status;
+
+		/* attach */
+
+		ASSERT_EQ(0, ptrace(PTRACE_ATTACH, pid, NULL, NULL));
+		ASSERT_EQ(pid, waitpid(pid, &status, 0));
+		ASSERT_TRUE(WIFSTOPPED(status));
+
+		/* unlock */
+
+		ASSERT_EQ(0, ptrace(PTRACE_POKEDATA, pid, &chld_lock, 0));
+
+		/* resume and wait for the 1st c.ebreak */
+
+		ASSERT_EQ(0, ptrace(PTRACE_CONT, pid, NULL, NULL));
+		ASSERT_EQ(pid, waitpid(pid, &status, 0));
+		ASSERT_TRUE(WIFSTOPPED(status));
+
+		/* read tracee vector csr regs using ptrace GETREGSET */
+
+		regset_size = sizeof(*regset_data) + vlenb * 32;
+		regset_data = calloc(1, regset_size);
+
+		iov.iov_base = regset_data;
+		iov.iov_len = regset_size;
+
+		ASSERT_EQ(0, ptrace(PTRACE_GETREGSET, pid, NT_RISCV_VECTOR, &iov));
+
+		/* set invalid vtype 0x1d = (5 | 3 << 3):
+		 * - LMUL: 1/8
+		 * - SEW:  64
+		 *   - invalid configuration for VLENB <= 128
+		 */
+		regset_data->vtype = 0x1d;
+		ASSERT_EQ(0, ptrace(PTRACE_SETREGSET, pid, NT_RISCV_VECTOR, &iov));
+
+		/* skip 1st c.ebreak, then resume and wait for the 2nd c.ebreak */
+
+		iov.iov_base = &regs;
+		iov.iov_len = sizeof(regs);
+
+		ASSERT_EQ(0, ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, &iov));
+		regs.pc += 2;
+		ASSERT_EQ(0, ptrace(PTRACE_SETREGSET, pid, NT_PRSTATUS, &iov));
+
+		ASSERT_EQ(0, ptrace(PTRACE_CONT, pid, NULL, NULL));
+		ASSERT_EQ(pid, waitpid(pid, &status, 0));
+		ASSERT_TRUE(WIFSTOPPED(status));
+
+		/* read tracee vtype using ptrace GETREGSET */
+
+		iov.iov_base = regset_data;
+		iov.iov_len = regset_size;
+		ASSERT_EQ(0, ptrace(PTRACE_GETREGSET, pid, NT_RISCV_VECTOR, &iov));
+
+		/* read tracee vtype ptrace PEEKDATA */
+
+		errno = 0;
+		reg = ptrace(PTRACE_PEEKDATA, pid, &vtype, NULL);
+		ASSERT_FALSE((errno != 0) && (reg == -1));
+
+		/* verify that V state is illegal */
+
+		EXPECT_EQ(reg, regset_data->vtype);
+		EXPECT_EQ(1UL, (regset_data->vtype >> (__riscv_xlen - 1)));
+
+		/* cleanup */
+
+		ASSERT_EQ(0, kill(pid, SIGKILL));
+	}
+}
+
 TEST_HARNESS_MAIN
-- 
2.51.0


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

[PATCH v2 4/6] riscv: vector: allow to force vector context save

[Sergey Matyukevich]

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.

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@gmail.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


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

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

[Andy Chiu]

On Tue, Oct 7, 2025 at 6:58 AM Sergey Matyukevich <geomatsi@gmail.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?

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@gmail.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
>

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

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

[Andy Chiu]

On Wed, Oct 15, 2025 at 3:18 PM Andy Chiu <andybnac@gmail.com> wrote:
>
> On Tue, Oct 7, 2025 at 6:58 AM Sergey Matyukevich <geomatsi@gmail.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@gmail.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
> >

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

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

[Sergey Matyukevich]

On Wed, Oct 15, 2025 at 04:32:05PM -0500, Andy Chiu wrote:
> On Wed, Oct 15, 2025 at 3:18 PM Andy Chiu <andybnac@gmail.com> wrote:
> >
> > On Tue, Oct 7, 2025 at 6:58 AM Sergey Matyukevich <geomatsi@gmail.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?

Just to clarify, the suggestion is to drop the TIF_RISCV_V_FORCE_SAVE
entirely and use only careful validation of input parameter in riscv_vr_set,
rather than using both checks. Is that correct?

If that is correct, then I assume we can rely on the simple rule ELEN == XLEN
to validate vsew/vlmul supported combinations. Additionally, reserved vsew
values (see 3.4.1 in spec) should also be rejected.

Thanks,
Sergey

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

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

[Andy Chiu]

On Sun, Oct 19, 2025 at 4:29 PM Sergey Matyukevich <geomatsi@gmail.com> wrote:
>
> On Wed, Oct 15, 2025 at 04:32:05PM -0500, Andy Chiu wrote:
> > On Wed, Oct 15, 2025 at 3:18 PM Andy Chiu <andybnac@gmail.com> wrote:
> > >
> > > On Tue, Oct 7, 2025 at 6:58 AM Sergey Matyukevich <geomatsi@gmail.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?
>
> Just to clarify, the suggestion is to drop the TIF_RISCV_V_FORCE_SAVE
> entirely and use only careful validation of input parameter in riscv_vr_set,
> rather than using both checks. Is that correct?

Yes, exactly

>
> If that is correct, then I assume we can rely on the simple rule ELEN == XLEN
> to validate vsew/vlmul supported combinations. Additionally, reserved vsew
> values (see 3.4.1 in spec) should also be rejected.

I am sorry but this assumption may not be correct. The spec does not
restrict a 32b machine from supporting ELEN=64, according to my
search. There is a way to infer ELEN though, by inspecting if zve64x
is present on isa.

Thanks,
Andy

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

[PATCH v2 5/6] selftests: riscv: verify initial vector state with ptrace

[Sergey Matyukevich]

Add a test case that attaches to a traced process immediately after its
first vector instructions to verify the initial vector context state.

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
---
 .../testing/selftests/riscv/vector/v_ptrace.c | 103 ++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/tools/testing/selftests/riscv/vector/v_ptrace.c b/tools/testing/selftests/riscv/vector/v_ptrace.c
index ccda8a4dc49b..f452e04629ea 100644
--- a/tools/testing/selftests/riscv/vector/v_ptrace.c
+++ b/tools/testing/selftests/riscv/vector/v_ptrace.c
@@ -196,4 +196,107 @@ TEST(ptrace_rvv_invalid_vtype)
 	}
 }
 
+TEST(ptrace_rvv_early_access)
+{
+	static volatile unsigned long vstart;
+	static volatile unsigned long vtype;
+	static volatile unsigned long vlenb;
+	static volatile unsigned long vcsr;
+	static volatile unsigned long vl;
+	pid_t pid;
+
+	if (!is_vector_supported())
+		SKIP(return, "Vector not supported");
+
+	chld_lock = 1;
+
+	pid = fork();
+
+	ASSERT_LE(0, pid)
+		TH_LOG("fork: %m");
+
+	if (pid == 0) {
+		while (chld_lock == 1)
+			asm volatile("" : : "g"(chld_lock) : "memory");
+
+		asm volatile("csrr %[vstart], vstart" : [vstart] "=r"(vstart));
+		asm volatile("csrr %[vl], vl" : [vl] "=r"(vl));
+		asm volatile("csrr %[vtype], vtype" : [vtype] "=r"(vtype));
+		asm volatile("csrr %[vcsr], vcsr" : [vcsr] "=r"(vcsr));
+		asm volatile("csrr %[vlenb], vlenb" : [vlenb] "=r"(vlenb));
+
+		asm volatile ("ebreak" : : : );
+	} else {
+		struct __riscv_v_regset_state *regset_data;
+		unsigned long vstart_csr;
+		unsigned long vl_csr;
+		unsigned long vtype_csr;
+		unsigned long vcsr_csr;
+		unsigned long vlenb_csr;
+		size_t regset_size;
+		struct iovec iov;
+		int status;
+
+		/* attach */
+
+		ASSERT_EQ(0, ptrace(PTRACE_ATTACH, pid, NULL, NULL));
+		ASSERT_EQ(pid, waitpid(pid, &status, 0));
+		ASSERT_TRUE(WIFSTOPPED(status));
+
+		/* unlock */
+
+		ASSERT_EQ(0, ptrace(PTRACE_POKEDATA, pid, &chld_lock, 0));
+
+		/* resume and wait for ebreak */
+
+		ASSERT_EQ(0, ptrace(PTRACE_CONT, pid, NULL, NULL));
+		ASSERT_EQ(pid, waitpid(pid, &status, 0));
+		ASSERT_TRUE(WIFSTOPPED(status));
+
+		/* read tracee vector csr regs using ptrace PEEKDATA */
+
+		errno = 0;
+		vstart_csr = ptrace(PTRACE_PEEKDATA, pid, &vstart, NULL);
+		ASSERT_FALSE((errno != 0) && (vstart_csr == -1));
+
+		errno = 0;
+		vl_csr = ptrace(PTRACE_PEEKDATA, pid, &vl, NULL);
+		ASSERT_FALSE((errno != 0) && (vl_csr == -1));
+
+		errno = 0;
+		vtype_csr = ptrace(PTRACE_PEEKDATA, pid, &vtype, NULL);
+		ASSERT_FALSE((errno != 0) && (vtype_csr == -1));
+
+		errno = 0;
+		vcsr_csr = ptrace(PTRACE_PEEKDATA, pid, &vcsr, NULL);
+		ASSERT_FALSE((errno != 0) && (vcsr_csr == -1));
+
+		errno = 0;
+		vlenb_csr = ptrace(PTRACE_PEEKDATA, pid, &vlenb, NULL);
+		ASSERT_FALSE((errno != 0) && (vlenb_csr == -1));
+
+		/* read tracee csr regs using ptrace GETREGSET */
+
+		regset_size = sizeof(*regset_data) + vlenb_csr * 32;
+		regset_data = calloc(1, regset_size);
+
+		iov.iov_base = regset_data;
+		iov.iov_len = regset_size;
+
+		ASSERT_EQ(0, ptrace(PTRACE_GETREGSET, pid, NT_RISCV_VECTOR, &iov));
+
+		/* compare */
+
+		EXPECT_EQ(vstart_csr, regset_data->vstart);
+		EXPECT_EQ(vtype_csr, regset_data->vtype);
+		EXPECT_EQ(vlenb_csr, regset_data->vlenb);
+		EXPECT_EQ(vcsr_csr, regset_data->vcsr);
+		EXPECT_EQ(vl_csr, regset_data->vl);
+
+		/* cleanup */
+
+		ASSERT_EQ(0, kill(pid, SIGKILL));
+	}
+}
+
 TEST_HARNESS_MAIN
-- 
2.51.0


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

[PATCH v2 6/6] riscv: vector: initialize vlenb on the first context switch

[Sergey Matyukevich]

The vstate in thread_struct is zeroed when the vector context is
initialized. That includes read-only register vlenb, which holds
the vector register length in bytes. This zeroed state persists
until mstatus.VS becomes 'dirty' and a context switch saves the
actual hardware values.

This can expose the zero vlenb value to the user-space in early
debug scenarios, e.g. when ptrace attaches to a traced process
early, before any vector instruction except the first one was
executed.

Fix this by forcing the vector context save on the first context switch.

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
---
 arch/riscv/kernel/vector.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
index 901e67adf576..3dd22a71aa18 100644
--- a/arch/riscv/kernel/vector.c
+++ b/arch/riscv/kernel/vector.c
@@ -120,6 +120,7 @@ static int riscv_v_thread_zalloc(struct kmem_cache *cache,
 
 	ctx->datap = datap;
 	memset(ctx, 0, offsetof(struct __riscv_v_ext_state, datap));
+
 	return 0;
 }
 
@@ -216,8 +217,11 @@ bool riscv_v_first_use_handler(struct pt_regs *regs)
 		force_sig(SIGBUS);
 		return true;
 	}
+
 	riscv_v_vstate_on(regs);
 	riscv_v_vstate_set_restore(current, regs);
+	set_tsk_thread_flag(current, TIF_RISCV_V_FORCE_SAVE);
+
 	return true;
 }
 
-- 
2.51.0


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

Re: [PATCH v2 6/6] riscv: vector: initialize vlenb on the first context switch

[Andy Chiu]

Hi Sergey,

On Tue, Oct 7, 2025 at 6:58 AM Sergey Matyukevich <geomatsi@gmail.com> wrote:
>
> The vstate in thread_struct is zeroed when the vector context is
> initialized. That includes read-only register vlenb, which holds
> the vector register length in bytes. This zeroed state persists
> until mstatus.VS becomes 'dirty' and a context switch saves the
> actual hardware values.
>
> This can expose the zero vlenb value to the user-space in early
> debug scenarios, e.g. when ptrace attaches to a traced process
> early, before any vector instruction except the first one was
> executed.
>
> Fix this by forcing the vector context save on the first context switch.
>
> Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
> ---
>  arch/riscv/kernel/vector.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
> index 901e67adf576..3dd22a71aa18 100644
> --- a/arch/riscv/kernel/vector.c
> +++ b/arch/riscv/kernel/vector.c
> @@ -120,6 +120,7 @@ static int riscv_v_thread_zalloc(struct kmem_cache *cache,
>
>         ctx->datap = datap;
>         memset(ctx, 0, offsetof(struct __riscv_v_ext_state, datap));
> +
>         return 0;
>  }
>
> @@ -216,8 +217,11 @@ bool riscv_v_first_use_handler(struct pt_regs *regs)
>                 force_sig(SIGBUS);
>                 return true;
>         }
> +
>         riscv_v_vstate_on(regs);
>         riscv_v_vstate_set_restore(current, regs);
> +       set_tsk_thread_flag(current, TIF_RISCV_V_FORCE_SAVE);
> +

I am afraid that this approach can result in a security issue where a
context switch happens before the v-restore part of the current
process, cheating the kernel to store stale v-regs onto the current
context memory. Please note that this handler is run with irq enabled
so preemption is allowed.

I would expect simply initializing the vleb in riscv_v_thread_zalloc,
perhaps dropping the "z" in the name to prevent confusion.

>         return true;
>  }
>
> --
> 2.51.0
>

Thanks,
Andy

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

Re: [PATCH v2 6/6] riscv: vector: initialize vlenb on the first context switch

[Sergey Matyukevich]

On Wed, Oct 15, 2025 at 02:54:39PM -0500, Andy Chiu wrote:
> Hi Sergey,
> 
> On Tue, Oct 7, 2025 at 6:58 AM Sergey Matyukevich <geomatsi@gmail.com> wrote:
> >
> > The vstate in thread_struct is zeroed when the vector context is
> > initialized. That includes read-only register vlenb, which holds
> > the vector register length in bytes. This zeroed state persists
> > until mstatus.VS becomes 'dirty' and a context switch saves the
> > actual hardware values.
> >
> > This can expose the zero vlenb value to the user-space in early
> > debug scenarios, e.g. when ptrace attaches to a traced process
> > early, before any vector instruction except the first one was
> > executed.
> >
> > Fix this by forcing the vector context save on the first context switch.
> >
> > Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
> > ---
> >  arch/riscv/kernel/vector.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
> > index 901e67adf576..3dd22a71aa18 100644
> > --- a/arch/riscv/kernel/vector.c
> > +++ b/arch/riscv/kernel/vector.c
> > @@ -120,6 +120,7 @@ static int riscv_v_thread_zalloc(struct kmem_cache *cache,
> >
> >         ctx->datap = datap;
> >         memset(ctx, 0, offsetof(struct __riscv_v_ext_state, datap));
> > +
> >         return 0;
> >  }
> >
> > @@ -216,8 +217,11 @@ bool riscv_v_first_use_handler(struct pt_regs *regs)
> >                 force_sig(SIGBUS);
> >                 return true;
> >         }
> > +
> >         riscv_v_vstate_on(regs);
> >         riscv_v_vstate_set_restore(current, regs);
> > +       set_tsk_thread_flag(current, TIF_RISCV_V_FORCE_SAVE);
> > +
> 
> I am afraid that this approach can result in a security issue where a
> context switch happens before the v-restore part of the current
> process, cheating the kernel to store stale v-regs onto the current
> context memory. Please note that this handler is run with irq enabled
> so preemption is allowed.
> 
> I would expect simply initializing the vleb in riscv_v_thread_zalloc,
> perhaps dropping the "z" in the name to prevent confusion.

Ok, so we can just set 'ctx->vlenb = riscv_v_vsize / 32' in the renamed
riscv_v_thread_alloc function. But note, that w/o forced context save
we implicitly reset the vector configuration to 'all zeros', overwriting
the hardware defaults.

By the way, could you please elaborate a little bit more about your security
concerns with the TIF_RISCV_V_FORCE_SAVE approach ? The atomic and per-process
flag modification looks safe to me, so I'd like to understand what I am
missing.

Thanks,
Sergey

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

Re: [PATCH v2 6/6] riscv: vector: initialize vlenb on the first context switch

[Andy Chiu]

On Sun, Oct 19, 2025 at 4:43 PM Sergey Matyukevich <geomatsi@gmail.com> wrote:
>
> On Wed, Oct 15, 2025 at 02:54:39PM -0500, Andy Chiu wrote:
> > Hi Sergey,
> >
> > On Tue, Oct 7, 2025 at 6:58 AM Sergey Matyukevich <geomatsi@gmail.com> wrote:
> > >
> > > The vstate in thread_struct is zeroed when the vector context is
> > > initialized. That includes read-only register vlenb, which holds
> > > the vector register length in bytes. This zeroed state persists
> > > until mstatus.VS becomes 'dirty' and a context switch saves the
> > > actual hardware values.
> > >
> > > This can expose the zero vlenb value to the user-space in early
> > > debug scenarios, e.g. when ptrace attaches to a traced process
> > > early, before any vector instruction except the first one was
> > > executed.
> > >
> > > Fix this by forcing the vector context save on the first context switch.
> > >
> > > Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
> > > ---
> > >  arch/riscv/kernel/vector.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
> > > index 901e67adf576..3dd22a71aa18 100644
> > > --- a/arch/riscv/kernel/vector.c
> > > +++ b/arch/riscv/kernel/vector.c
> > > @@ -120,6 +120,7 @@ static int riscv_v_thread_zalloc(struct kmem_cache *cache,
> > >
> > >         ctx->datap = datap;
> > >         memset(ctx, 0, offsetof(struct __riscv_v_ext_state, datap));
> > > +
> > >         return 0;
> > >  }
> > >
> > > @@ -216,8 +217,11 @@ bool riscv_v_first_use_handler(struct pt_regs *regs)
> > >                 force_sig(SIGBUS);
> > >                 return true;
> > >         }
> > > +
> > >         riscv_v_vstate_on(regs);
> > >         riscv_v_vstate_set_restore(current, regs);
> > > +       set_tsk_thread_flag(current, TIF_RISCV_V_FORCE_SAVE);
> > > +
> >
> > I am afraid that this approach can result in a security issue where a
> > context switch happens before the v-restore part of the current
> > process, cheating the kernel to store stale v-regs onto the current
> > context memory. Please note that this handler is run with irq enabled
> > so preemption is allowed.
> >
> > I would expect simply initializing the vleb in riscv_v_thread_zalloc,
> > perhaps dropping the "z" in the name to prevent confusion.
>
> Ok, so we can just set 'ctx->vlenb = riscv_v_vsize / 32' in the renamed
> riscv_v_thread_alloc function. But note, that w/o forced context save
> we implicitly reset the vector configuration to 'all zeros', overwriting
> the hardware defaults.

Resetting all vregs to zero is desired as otherwise we may
unintentionally leak stale states from other users or the kernel to
the user process.

>
> By the way, could you please elaborate a little bit more about your security
> concerns with the TIF_RISCV_V_FORCE_SAVE approach ? The atomic and per-process
> flag modification looks safe to me, so I'd like to understand what I am
> missing.
>

The concern is information leak. A context switch can happen right
after the FORCE_SAVE bit is set. At this point the kernel saves live
vregs on the machine to the context memory (vstate) of that process.
The content of live registers may come from another process, or stale
value of in-kernel Vector uses, since we don't flush registers at
every ownership change. When we switch back to the original process
and return to the user space, the saved stale content is restored back
to registers. As a result, the user space can read Vector registers
from other contexts.

Thanks,
Andy

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