[<prev] [next>] [day] [month] [year] [list]
Message-Id: <20230325193349.31893-1-rick.p.edgecombe@intel.com>
Date: Sat, 25 Mar 2023 12:33:49 -0700
From: Rick Edgecombe <rick.p.edgecombe@...el.com>
To: x86@...nel.org, Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...el.com>,
Andy Lutomirski <luto@...nel.org>,
"H . Peter Anvin" <hpa@...or.com>,
Peter Zijlstra <peterz@...radead.org>,
Dan Carpenter <error27@...il.com>, linux-kernel@...r.kernel.org
Cc: rick.p.edgecombe@...el.com
Subject: [PATCH] x86: Enforce only whole copies for ssp_set()
The regset set interface takes pos and count arguments to allow for
partial copies. No callers use a non-zero pos, but ptrace allows for the
count to be specified. It limits count to be a multiple of regset size, so
this still allows for a zero size to be passed to ssp_set().
In ssp_set(), user_regset_copyin() returns success for copying zero bytes,
which means user_ssp can later be accessed uninitialized. So add
enforcement for this case. The other regset's also enforce pos == 0, so do
that as well even though there is no caller today.
In the case of partial copies, some regsets return -EINVAL and some
return -EFAULT. -EINVAL seems more appropriate, so use that error code.
Fixes: d84e6ee122e5 ("x86: Add PTRACE interface for shadow stack")
Reported-by: Dan Carpenter <error27@...il.com>
Link: https://lore.kernel.org/all/90af27cc-6c9d-4fb9-be3b-fc4ef378766d@kili.mountain/
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@...el.com>
---
Hi x86 maintainers,
While debugging this I wrote a shadow stack ptrace selftest that tries
a bunch of invalid values. I thought to save it for a future series
with selftests enhancements, unless you'd like to see it sooner.
Thanks,
Rick
---
arch/x86/kernel/fpu/regset.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index f0a8eaf7c52e..6bc1eb2a21bd 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -223,6 +223,9 @@ int ssp_set(struct task_struct *target, const struct user_regset *regset,
!ssp_active(target, regset))
return -ENODEV;
+ if (pos != 0 || count != sizeof(user_ssp))
+ return -EINVAL;
+
r = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &user_ssp, 0, -1);
if (r)
return r;
base-commit: b642e9e5f0dc797f543b431d4ba910a3da72a074
--
2.17.1
Powered by blists - more mailing lists