arch/riscv/kernel/signal.c:196 __restore_cfiss_state() warn: maybe return -EFAULT instead of the bytes remaining?

arch/riscv/kernel/signal.c:196 __restore_cfiss_state() warn: maybe return -EFAULT instead of the bytes remaining?

[Dan Carpenter]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   26a4cfaff82a2dcb810f6bfd5f4842f9b6046c8a
commit: 66c9c713de597f9b40a319ebda4d3466ce2cdff0 riscv/signal: save and restore the shadow stack on a signal
config: riscv-randconfig-r073-20260216 (https://download.01.org/0day-ci/archive/20260216/202602160730.QsdCT9Zp-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 13.4.0
smatch version: v0.5.0-8994-gd50c5a4c

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>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202602160730.QsdCT9Zp-lkp@intel.com/

New smatch warnings:
arch/riscv/kernel/signal.c:196 __restore_cfiss_state() warn: maybe return -EFAULT instead of the bytes remaining?

Old smatch warnings:
arch/riscv/kernel/signal.c:52 restore_fp_state() warn: maybe return -EFAULT instead of the bytes remaining?
arch/riscv/kernel/signal.c:66 save_fp_state() warn: maybe return -EFAULT instead of the bytes remaining?
arch/riscv/kernel/signal.c:132 __restore_v_state() warn: maybe return -EFAULT instead of the bytes remaining?
arch/riscv/kernel/signal.c:142 __restore_v_state() warn: maybe return -EFAULT instead of the bytes remaining?
arch/riscv/kernel/signal.c:228 restore_sigcontext() warn: maybe return -EFAULT instead of the bytes remaining?

vim +196 arch/riscv/kernel/signal.c

66c9c713de597f Deepak Gupta 2026-01-25  174  static long __restore_cfiss_state(struct pt_regs *regs, void __user *sc_cfi)
66c9c713de597f Deepak Gupta 2026-01-25  175  {
66c9c713de597f Deepak Gupta 2026-01-25  176  	struct __sc_riscv_cfi_state __user *state = sc_cfi;
66c9c713de597f Deepak Gupta 2026-01-25  177  	unsigned long ss_ptr = 0;
66c9c713de597f Deepak Gupta 2026-01-25  178  	long err;
66c9c713de597f Deepak Gupta 2026-01-25  179  
66c9c713de597f Deepak Gupta 2026-01-25  180  	/*
66c9c713de597f Deepak Gupta 2026-01-25  181  	 * Restore shadow stack as a form of token stored on the shadow stack itself as a safe
66c9c713de597f Deepak Gupta 2026-01-25  182  	 * way to restore.
66c9c713de597f Deepak Gupta 2026-01-25  183  	 * A token on the shadow stack gives the following properties:
66c9c713de597f Deepak Gupta 2026-01-25  184  	 * - Safe save and restore for shadow stack switching. Any save of shadow stack
66c9c713de597f Deepak Gupta 2026-01-25  185  	 *   must have saved a token on shadow stack. Similarly any restore of shadow
66c9c713de597f Deepak Gupta 2026-01-25  186  	 *   stack must check the token before restore. Since writing to a shadow stack with
66c9c713de597f Deepak Gupta 2026-01-25  187  	 *   the address of shadow stack itself is not easily allowed, a restore without a save
66c9c713de597f Deepak Gupta 2026-01-25  188  	 *   is quite difficult for an attacker to perform.
66c9c713de597f Deepak Gupta 2026-01-25  189  	 * - A natural break. A token in the shadow stack provides a natural break in shadow stack
66c9c713de597f Deepak Gupta 2026-01-25  190  	 *   So a single linear range can be bucketed into different shadow stack segments.
66c9c713de597f Deepak Gupta 2026-01-25  191  	 *   sspopchk will detect the condition and fault to kernel as a sw check exception.
66c9c713de597f Deepak Gupta 2026-01-25  192  	 */
66c9c713de597f Deepak Gupta 2026-01-25  193  	err = __copy_from_user(&ss_ptr, &state->ss_ptr, sizeof(unsigned long));
66c9c713de597f Deepak Gupta 2026-01-25  194  
66c9c713de597f Deepak Gupta 2026-01-25  195  	if (unlikely(err))
66c9c713de597f Deepak Gupta 2026-01-25 @196  		return err;

A lot of code in signal.c used to return nonsense values because it's
highly optimized and the caller handles it.  As in:

	return __copy_from_user();

But this code doesn't feel very optimized, honestly, and I bet
returning -EFAULT doesn't hurt anything.  I think it would be faster
to get rid of the ret variable and do:

	if (__copy_from_user())
		return -EFAULT;

My brief and bogus testing seems to show a slight speed up
probably based on the smaller stack size.

66c9c713de597f Deepak Gupta 2026-01-25  197  
66c9c713de597f Deepak Gupta 2026-01-25  198  	return restore_user_shstk(current, ss_ptr);
66c9c713de597f Deepak Gupta 2026-01-25  199  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki