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/[email protected]/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/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
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
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 <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Signed-off-by: Rick Edgecombe <[email protected]>
---
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
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 <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> Signed-off-by: Rick Edgecombe <[email protected]>
Seems fine to me. Thanks!
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook