[v3] riscv: Discard vector state on syscalls

  From: Björn Töpel <bjorn@rivosinc.com>

The RISC-V vector specification states:
  Executing a system call causes all caller-saved vector registers
  (v0-v31, vl, vtype) and vstart to become unspecified.

The vector registers are set to all 1s, vill is set (invalid), and the
vector status is set to Dirty.

That way we can prevent userspace from accidentally relying on the
stated save.

Rémi pointed out [1] that writing to the registers might be
superfluous, and setting vill is sufficient.

Link: https://lore.kernel.org/linux-riscv/12784326.9UPPK3MAeB@basile.remlab.net/ # [1]
Suggested-by: Darius Rad <darius@bluespec.com>
Suggested-by: Palmer Dabbelt <palmer@rivosinc.com>
Suggested-by: Rémi Denis-Courmont <remi@remlab.net>
Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
---

v2->v3:
  Set state to Dirty after discard, for proper ptrace() handling
  (Andy)

v1->v2:
  Proper register restore for initial state (Andy)
  Set registers to 1s, and not 0s (Darius)

---
 arch/riscv/include/asm/vector.h | 33 +++++++++++++++++++++++++++++++++
 arch/riscv/kernel/traps.c       |  2 ++
 2 files changed, 35 insertions(+)


base-commit: 488833ccdcac118da16701f4ee0673b20ba47fe3
  

Hey,

On Thu, Jun 29, 2023 at 08:27:30AM +0200, Björn Töpel wrote:
> From: Björn Töpel <bjorn@rivosinc.com>
> 
> The RISC-V vector specification states:
>   Executing a system call causes all caller-saved vector registers
>   (v0-v31, vl, vtype) and vstart to become unspecified.
> 
> The vector registers are set to all 1s, vill is set (invalid), and the
> vector status is set to Dirty.
> 
> That way we can prevent userspace from accidentally relying on the
> stated save.
> 
> Rémi pointed out [1] that writing to the registers might be
> superfluous, and setting vill is sufficient.
> 
> Link: https://lore.kernel.org/linux-riscv/12784326.9UPPK3MAeB@basile.remlab.net/ # [1]
> Suggested-by: Darius Rad <darius@bluespec.com>
> Suggested-by: Palmer Dabbelt <palmer@rivosinc.com>
> Suggested-by: Rémi Denis-Courmont <remi@remlab.net>
> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>

clang allmodconfig and rv32_defconfig fail to build with this patch,
according to patchwork:
../arch/riscv/kernel/traps.c:299:3: error: call to undeclared function 'riscv_v_vstate_discard'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]

Cheers,
Conor.

> ---
> 
> v2->v3:
>   Set state to Dirty after discard, for proper ptrace() handling
>   (Andy)
> 
> v1->v2:
>   Proper register restore for initial state (Andy)
>   Set registers to 1s, and not 0s (Darius)
> 
> ---
>  arch/riscv/include/asm/vector.h | 33 +++++++++++++++++++++++++++++++++
>  arch/riscv/kernel/traps.c       |  2 ++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> index 04c0b07bf6cd..0b23056503c5 100644
> --- a/arch/riscv/include/asm/vector.h
> +++ b/arch/riscv/include/asm/vector.h
> @@ -33,6 +33,11 @@ static inline void __riscv_v_vstate_clean(struct pt_regs *regs)
>  	regs->status = (regs->status & ~SR_VS) | SR_VS_CLEAN;
>  }
>  
> +static inline void __riscv_v_vstate_dirty(struct pt_regs *regs)
> +{
> +	regs->status = (regs->status & ~SR_VS) | SR_VS_DIRTY;
> +}
> +
>  static inline void riscv_v_vstate_off(struct pt_regs *regs)
>  {
>  	regs->status = (regs->status & ~SR_VS) | SR_VS_OFF;
> @@ -128,6 +133,34 @@ static inline void __riscv_v_vstate_restore(struct __riscv_v_ext_state *restore_
>  	riscv_v_disable();
>  }
>  
> +static inline void __riscv_v_vstate_discard(void)
> +{
> +	unsigned long vl, vtype_inval = 1UL << (BITS_PER_LONG - 1);
> +
> +	riscv_v_enable();
> +	asm volatile (
> +		".option push\n\t"
> +		".option arch, +v\n\t"
> +		"vsetvli	%0, x0, e8, m8, ta, ma\n\t"
> +		"vmv.v.i	v0, -1\n\t"
> +		"vmv.v.i	v8, -1\n\t"
> +		"vmv.v.i	v16, -1\n\t"
> +		"vmv.v.i	v24, -1\n\t"
> +		"vsetvl		%0, x0, %1\n\t"
> +		".option pop\n\t"
> +		: "=&r" (vl) : "r" (vtype_inval) : "memory");
> +	riscv_v_disable();
> +}
> +
> +static inline void riscv_v_vstate_discard(struct pt_regs *regs)
> +{
> +	if ((regs->status & SR_VS) == SR_VS_OFF)
> +		return;
> +
> +	__riscv_v_vstate_discard();
> +	__riscv_v_vstate_dirty(regs);
> +}
> +
>  static inline void riscv_v_vstate_save(struct task_struct *task,
>  				       struct pt_regs *regs)
>  {
> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> index 5158961ea977..5ff63a784a6d 100644
> --- a/arch/riscv/kernel/traps.c
> +++ b/arch/riscv/kernel/traps.c
> @@ -296,6 +296,8 @@ asmlinkage __visible __trap_section void do_trap_ecall_u(struct pt_regs *regs)
>  		regs->epc += 4;
>  		regs->orig_a0 = regs->a0;
>  
> +		riscv_v_vstate_discard(regs);
> +
>  		syscall = syscall_enter_from_user_mode(regs, syscall);
>  
>  		if (syscall < NR_syscalls)
> 
> base-commit: 488833ccdcac118da16701f4ee0673b20ba47fe3
> -- 
> 2.39.2
>
  

Hi Björn,

kernel test robot noticed the following build errors:

[auto build test ERROR on 488833ccdcac118da16701f4ee0673b20ba47fe3]

url:    https://github.com/intel-lab-lkp/linux/commits/Bj-rn-T-pel/riscv-Discard-vector-state-on-syscalls/20230629-142852
base:   488833ccdcac118da16701f4ee0673b20ba47fe3
patch link:    https://lore.kernel.org/r/20230629062730.985184-1-bjorn%40kernel.org
patch subject: [PATCH v3] riscv: Discard vector state on syscalls
config: riscv-randconfig-r042-20230629 (https://download.01.org/0day-ci/archive/20230629/202306291513.DwaMo6k7-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce: (https://download.01.org/0day-ci/archive/20230629/202306291513.DwaMo6k7-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306291513.DwaMo6k7-lkp@intel.com/

All errors (new ones prefixed by >>):

         |                                          ~~~~~~~~~~ ^
   In file included from arch/riscv/kernel/traps.c:15:
   In file included from include/linux/kprobes.h:28:
   In file included from include/linux/ftrace.h:10:
   In file included from include/linux/trace_recursion.h:5:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:751:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     751 |         insw(addr, buffer, count);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw'
     105 | #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count)
         |                                          ~~~~~~~~~~ ^
   In file included from arch/riscv/kernel/traps.c:15:
   In file included from include/linux/kprobes.h:28:
   In file included from include/linux/ftrace.h:10:
   In file included from include/linux/trace_recursion.h:5:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:759:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     759 |         insl(addr, buffer, count);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/io.h:106:53: note: expanded from macro 'insl'
     106 | #define insl(addr, buffer, count) __insl(PCI_IOBASE + (addr), buffer, count)
         |                                          ~~~~~~~~~~ ^
   In file included from arch/riscv/kernel/traps.c:15:
   In file included from include/linux/kprobes.h:28:
   In file included from include/linux/ftrace.h:10:
   In file included from include/linux/trace_recursion.h:5:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:768:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     768 |         outsb(addr, buffer, count);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/io.h:118:55: note: expanded from macro 'outsb'
     118 | #define outsb(addr, buffer, count) __outsb(PCI_IOBASE + (addr), buffer, count)
         |                                            ~~~~~~~~~~ ^
   In file included from arch/riscv/kernel/traps.c:15:
   In file included from include/linux/kprobes.h:28:
   In file included from include/linux/ftrace.h:10:
   In file included from include/linux/trace_recursion.h:5:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:777:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     777 |         outsw(addr, buffer, count);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/io.h:119:55: note: expanded from macro 'outsw'
     119 | #define outsw(addr, buffer, count) __outsw(PCI_IOBASE + (addr), buffer, count)
         |                                            ~~~~~~~~~~ ^
   In file included from arch/riscv/kernel/traps.c:15:
   In file included from include/linux/kprobes.h:28:
   In file included from include/linux/ftrace.h:10:
   In file included from include/linux/trace_recursion.h:5:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:786:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     786 |         outsl(addr, buffer, count);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/io.h:120:55: note: expanded from macro 'outsl'
     120 | #define outsl(addr, buffer, count) __outsl(PCI_IOBASE + (addr), buffer, count)
         |                                            ~~~~~~~~~~ ^
   In file included from arch/riscv/kernel/traps.c:15:
   In file included from include/linux/kprobes.h:28:
   In file included from include/linux/ftrace.h:10:
   In file included from include/linux/trace_recursion.h:5:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:1134:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    1134 |         return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
         |                                                   ~~~~~~~~~~ ^
>> arch/riscv/kernel/traps.c:299:3: error: call to undeclared function 'riscv_v_vstate_discard'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     299 |                 riscv_v_vstate_discard(regs);
         |                 ^
   arch/riscv/kernel/traps.c:299:3: note: did you mean 'riscv_v_vstate_query'?
   arch/riscv/include/asm/vector.h:206:20: note: 'riscv_v_vstate_query' declared here
     206 | static inline bool riscv_v_vstate_query(struct pt_regs *regs) { return false; }
         |                    ^
   13 warnings and 1 error generated.


vim +/riscv_v_vstate_discard +299 arch/riscv/kernel/traps.c

   290	
   291	asmlinkage __visible __trap_section void do_trap_ecall_u(struct pt_regs *regs)
   292	{
   293		if (user_mode(regs)) {
   294			ulong syscall = regs->a7;
   295	
   296			regs->epc += 4;
   297			regs->orig_a0 = regs->a0;
   298	
 > 299			riscv_v_vstate_discard(regs);
   300	
   301			syscall = syscall_enter_from_user_mode(regs, syscall);
   302	
   303			if (syscall < NR_syscalls)
   304				syscall_handler(regs, syscall);
   305			else
   306				regs->a0 = -ENOSYS;
   307	
   308			syscall_exit_to_user_mode(regs);
   309		} else {
   310			irqentry_state_t state = irqentry_nmi_enter(regs);
   311	
   312			do_trap_error(regs, SIGILL, ILL_ILLTRP, regs->epc,
   313				"Oops - environment call from U-mode");
   314	
   315			irqentry_nmi_exit(regs, state);
   316		}
   317
  

Hi Björn,

kernel test robot noticed the following build errors:

[auto build test ERROR on 488833ccdcac118da16701f4ee0673b20ba47fe3]

url:    https://github.com/intel-lab-lkp/linux/commits/Bj-rn-T-pel/riscv-Discard-vector-state-on-syscalls/20230629-142852
base:   488833ccdcac118da16701f4ee0673b20ba47fe3
patch link:    https://lore.kernel.org/r/20230629062730.985184-1-bjorn%40kernel.org
patch subject: [PATCH v3] riscv: Discard vector state on syscalls
config: riscv-allnoconfig (https://download.01.org/0day-ci/archive/20230629/202306292011.OGfLGBam-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230629/202306292011.OGfLGBam-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306292011.OGfLGBam-lkp@intel.com/

All errors (new ones prefixed by >>):

   arch/riscv/kernel/traps.c: In function 'do_trap_ecall_u':
>> arch/riscv/kernel/traps.c:299:17: error: implicit declaration of function 'riscv_v_vstate_discard'; did you mean 'riscv_v_vstate_restore'? [-Werror=implicit-function-declaration]
     299 |                 riscv_v_vstate_discard(regs);
         |                 ^~~~~~~~~~~~~~~~~~~~~~
         |                 riscv_v_vstate_restore
   cc1: some warnings being treated as errors


vim +299 arch/riscv/kernel/traps.c

   290	
   291	asmlinkage __visible __trap_section void do_trap_ecall_u(struct pt_regs *regs)
   292	{
   293		if (user_mode(regs)) {
   294			ulong syscall = regs->a7;
   295	
   296			regs->epc += 4;
   297			regs->orig_a0 = regs->a0;
   298	
 > 299			riscv_v_vstate_discard(regs);
   300	
   301			syscall = syscall_enter_from_user_mode(regs, syscall);
   302	
   303			if (syscall < NR_syscalls)
   304				syscall_handler(regs, syscall);
   305			else
   306				regs->a0 = -ENOSYS;
   307	
   308			syscall_exit_to_user_mode(regs);
   309		} else {
   310			irqentry_state_t state = irqentry_nmi_enter(regs);
   311	
   312			do_trap_error(regs, SIGILL, ILL_ILLTRP, regs->epc,
   313				"Oops - environment call from U-mode");
   314	
   315			irqentry_nmi_exit(regs, state);
   316		}
   317
  

Conor Dooley <conor.dooley@microchip.com> writes:

> clang allmodconfig and rv32_defconfig fail to build with this patch,
> according to patchwork:
> ../arch/riscv/kernel/traps.c:299:3: error: call to undeclared function 'riscv_v_vstate_discard'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]

Ugh. Sloppy. :-(

Thank you!
Björn