[bug report] x86: Add PTRACE interface for shadow stack

From: Dan Carpenter <error27@gmail.com>
To: rick.p.edgecombe@intel.com
Cc: kernel-janitors@vger.kernel.org
Subject: [bug report] x86: Add PTRACE interface for shadow stack
Date: Sat, 25 Mar 2023 14:08:10 +0300	[thread overview]
Message-ID: <90af27cc-6c9d-4fb9-be3b-fc4ef378766d@kili.mountain> (raw)

Hello Rick Edgecombe,

The patch d84e6ee122e5: "x86: Add PTRACE interface for shadow stack"
from Mar 18, 2023, leads to the following Smatch static checker
warning:

	arch/x86/kernel/fpu/regset.c:234 ssp_set()
	error: uninitialized symbol 'user_ssp'.

arch/x86/kernel/fpu/regset.c
    212 int ssp_set(struct task_struct *target, const struct user_regset *regset,
    213             unsigned int pos, unsigned int count,
    214             const void *kbuf, const void __user *ubuf)
    215 {
    216         struct fpu *fpu = &target->thread.fpu;
    217         struct xregs_state *xsave = &fpu->fpstate->regs.xsave;
    218         struct cet_user_state *cetregs;
    219         unsigned long user_ssp;
    220         int r;
    221 
    222         if (!cpu_feature_enabled(X86_FEATURE_USER_SHSTK) ||
    223             !ssp_active(target, regset))
    224                 return -ENODEV;
    225 
    226         r = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &user_ssp, 0, -1);
                                              ^^^^^
"count" can be zero.  It comes from the user in ptrace_request().

ptrace_request()
-> ptrace_regset()
   -> copy_regset_from_user()
      -> ssp_set()

    227         if (r)
    228                 return r;
    229 
    230         /*
    231          * Some kernel instructions (IRET, etc) can cause exceptions in the case
    232          * of disallowed CET register values. Just prevent invalid values.
    233          */
--> 234         if (user_ssp >= TASK_SIZE_MAX || !IS_ALIGNED(user_ssp, 8))
                    ^^^^^^^^
Leading to an uninitialized variable here.

    235                 return -EINVAL;
    236 
    237         fpu_force_restore(fpu);
    238 
    239         cetregs = get_xsave_addr(xsave, XFEATURE_CET_USER);
    240         if (WARN_ON(!cetregs)) {
    241                 /*
    242                  * This shouldn't ever be NULL because shadow stack was
    243                  * verified to be enabled above. This means
    244                  * MSR_IA32_U_CET.CET_SHSTK_EN should be 1 and so
    245                  * XFEATURE_CET_USER should not be in the init state.
    246                  */
    247                 return -ENODEV;
    248         }
    249 
    250         cetregs->user_ssp = user_ssp;
    251         return 0;
    252 }

regards,
dan carpenter