arch/x86/kernel/shstk.c:295:55: sparse: sparse: cast removes address space '__user' of expression

arch/x86/kernel/shstk.c:295:55: sparse: sparse: cast removes address space '__user' of expression

[kernel test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   52b1853b080a082ec3749c3a9577f6c71b1d4a90
commit: 05e36022c0543ba55a3de55af455b00cb3eb4fcc x86/shstk: Handle signals for shadow stack
date:   5 months ago
config: x86_64-randconfig-123-20240106 (https://download.01.org/0day-ci/archive/20240108/202401080003.duO4RmjK-lkp@intel.com/config)
compiler: ClangBuiltLinux clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240108/202401080003.duO4RmjK-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/202401080003.duO4RmjK-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   arch/x86/kernel/shstk.c:244:29: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected unsigned long long [noderef] [usertype] __user *addr @@     got void *[noderef] __user @@
   arch/x86/kernel/shstk.c:244:29: sparse:     expected unsigned long long [noderef] [usertype] __user *addr
   arch/x86/kernel/shstk.c:244:29: sparse:     got void *[noderef] __user
>> arch/x86/kernel/shstk.c:295:55: sparse: sparse: cast removes address space '__user' of expression

vim +/__user +295 arch/x86/kernel/shstk.c

   271	
   272	int setup_signal_shadow_stack(struct ksignal *ksig)
   273	{
   274		void __user *restorer = ksig->ka.sa.sa_restorer;
   275		unsigned long ssp;
   276		int err;
   277	
   278		if (!cpu_feature_enabled(X86_FEATURE_USER_SHSTK) ||
   279		    !features_enabled(ARCH_SHSTK_SHSTK))
   280			return 0;
   281	
   282		if (!restorer)
   283			return -EINVAL;
   284	
   285		ssp = get_user_shstk_addr();
   286		if (unlikely(!ssp))
   287			return -EINVAL;
   288	
   289		err = shstk_push_sigframe(&ssp);
   290		if (unlikely(err))
   291			return err;
   292	
   293		/* Push restorer address */
   294		ssp -= SS_FRAME_SIZE;
 > 295		err = write_user_shstk_64((u64 __user *)ssp, (u64)restorer);
   296		if (unlikely(err))
   297			return -EFAULT;
   298	
   299		fpregs_lock_and_load();
   300		wrmsrl(MSR_IA32_PL3_SSP, ssp);
   301		fpregs_unlock();
   302	
   303		return 0;
   304	}
   305	

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

[PATCH] x86/shstk: Use __force when casting away __user

[Rick Edgecombe]

In setup_signal_shadow_stack() the kernel needs to push the restorer
address to the shadow stack. This involves writing the value of the
restorer pointer to the shadow stack. Since the restorer is defined as a
__user in struct k_sigaction, the __user needs to be casted away to read
the value. It is safe to do, because nothing is being dereferenced, and
the de-__user-ed value is not stashed in an accessible local variable
where it might accidentally be used for another purpose.

However, sparse warns about casting away __user. So use __force to
silence sparse and add a comment to explain why it is ok.

Fixes: 05e36022c054 ("x86/shstk: Handle signals for shadow stack")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202401080003.duO4RmjK-lkp@intel.com/
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
 arch/x86/kernel/shstk.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/shstk.c b/arch/x86/kernel/shstk.c
index 59e15dd8d0f8..7cc294482011 100644
--- a/arch/x86/kernel/shstk.c
+++ b/arch/x86/kernel/shstk.c
@@ -367,7 +367,11 @@ int setup_signal_shadow_stack(struct ksignal *ksig)
 
 	/* Push restorer address */
 	ssp -= SS_FRAME_SIZE;
-	err = write_user_shstk_64((u64 __user *)ssp, (u64)restorer);
+	/*
+	 * Use __force because this is just writing the address of the pointer
+	 * to the shadow stack, not dereferencing it.
+	 */
+	err = write_user_shstk_64((u64 __user *)ssp, (u64 __force)restorer);
 	if (unlikely(err))
 		return -EFAULT;
 
-- 
2.34.1

Re: [PATCH] x86/shstk: Use __force when casting away __user

[Kees Cook]

On Tue, Jan 09, 2024 at 02:46:19PM -0800, Rick Edgecombe wrote:
> In setup_signal_shadow_stack() the kernel needs to push the restorer
> address to the shadow stack. This involves writing the value of the
> restorer pointer to the shadow stack. Since the restorer is defined as a
> __user in struct k_sigaction, the __user needs to be casted away to read
> the value. It is safe to do, because nothing is being dereferenced, and
> the de-__user-ed value is not stashed in an accessible local variable
> where it might accidentally be used for another purpose.
> 
> However, sparse warns about casting away __user. So use __force to
> silence sparse and add a comment to explain why it is ok.
> 
> Fixes: 05e36022c054 ("x86/shstk: Handle signals for shadow stack")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202401080003.duO4RmjK-lkp@intel.com/
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>

Seems fine to me. Thanks!

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook