From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751907AbdIXK7e (ORCPT ); Sun, 24 Sep 2017 06:59:34 -0400 Received: from mail-wr0-f195.google.com ([209.85.128.195]:36172 "EHLO mail-wr0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751163AbdIXK7c (ORCPT ); Sun, 24 Sep 2017 06:59:32 -0400 X-Google-Smtp-Source: AOwi7QAo8/9sbqJfiKdFyGlQAh6xs8m3sZrqpuJ+JR6IlQkl1MjRajty9s2fkwGYzrw2BTUEhx2dyA== From: Ingo Molnar To: linux-kernel@vger.kernel.org Cc: Andrew Morton , Eric Biggers , Andy Lutomirski , Borislav Petkov , Dave Hansen , Fenghua Yu , "H . Peter Anvin" , Linus Torvalds , Oleg Nesterov , Peter Zijlstra , Rik van Riel , Thomas Gleixner , Yu-cheng Yu Subject: [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Date: Sun, 24 Sep 2017 12:59:03 +0200 Message-Id: <20170924105913.9157-1-mingo@kernel.org> X-Mailer: git-send-email 2.11.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org As mentioned before, the patch was too big and too complex, and I've split it up into 10 smaller, bisectable patches: Eric Biggers (10): x86/fpu: Introduce validate_xstate_header() x86/fpu: Use validate_xstate_header() to validate the xstate_header in xstateregs_set() x86/fpu: Use validate_xstate_header() to validate the xstate_header in sanitize_restored_xstate() x86/fpu: Copy the full state_header in copy_kernel_to_xstate() x86/fpu: Eliminate the 'xfeatures' local variable in copy_kernel_to_xstate() x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_kernel_to_xstate() x86/fpu: Copy the full header in copy_user_to_xstate() x86/fpu: Eliminate the 'xfeatures' local variable in copy_user_to_xstate() x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_user_to_xstate() x86/fpu: Use using_compacted_format() instead of open coded X86_FEATURE_XSAVES arch/x86/include/asm/fpu/xstate.h | 4 ++++ arch/x86/kernel/fpu/regset.c | 21 ++++++----------- arch/x86/kernel/fpu/signal.c | 17 ++++++------- arch/x86/kernel/fpu/xstate.c | 76 +++++++++++++++++++++++++++++++++-------------------------- 4 files changed, 63 insertions(+), 55 deletions(-) I kept the attribution in place, because the end result is almost the same. Below is the interdiff from the original version, I uninlined validate_xstate_header(), because it's way too large to be inlined everywhere, plus I removed a couple of spurious "!= 0" patterns. The latest x86/fpu bits can be found in -tip: git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git WIP.x86/fpu Thanks, Ingo === include/asm/fpu/xstate.h | 23 +---------------------- kernel/fpu/xstate.c | 28 ++++++++++++++++++++++++++-- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h index 3d79d0ee4d30..83fee2469eb7 100644 --- a/arch/x86/include/asm/fpu/xstate.h +++ b/arch/x86/include/asm/fpu/xstate.h @@ -54,27 +54,6 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf); int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf); /* Validate an xstate header supplied by userspace (ptrace or sigreturn) */ -static inline int validate_xstate_header(const struct xstate_header *hdr) -{ - /* No unknown or supervisor features may be set */ - if (hdr->xfeatures & (~xfeatures_mask | XFEATURE_MASK_SUPERVISOR)) - return -EINVAL; - - /* Userspace must use the uncompacted format */ - if (hdr->xcomp_bv) - return -EINVAL; - - /* - * If 'reserved' is shrunken to add a new field, make sure to validate - * that new field here! - */ - BUILD_BUG_ON(sizeof(hdr->reserved) != 48); - - /* No reserved bits may be set */ - if (memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved))) - return -EINVAL; - - return 0; -} +extern int validate_xstate_header(const struct xstate_header *hdr); #endif diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 7ebd1a0811b6..f1d5476c9022 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -483,6 +483,30 @@ int using_compacted_format(void) return boot_cpu_has(X86_FEATURE_XSAVES); } +/* Validate an xstate header supplied by userspace (ptrace or sigreturn) */ +int validate_xstate_header(const struct xstate_header *hdr) +{ + /* No unknown or supervisor features may be set */ + if (hdr->xfeatures & (~xfeatures_mask | XFEATURE_MASK_SUPERVISOR)) + return -EINVAL; + + /* Userspace must use the uncompacted format */ + if (hdr->xcomp_bv) + return -EINVAL; + + /* + * If 'reserved' is shrunken to add a new field, make sure to validate + * that new field here! + */ + BUILD_BUG_ON(sizeof(hdr->reserved) != 48); + + /* No reserved bits may be set */ + if (memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved))) + return -EINVAL; + + return 0; +} + static void __xstate_dump_leaves(void) { int i; @@ -1127,7 +1151,7 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf) memcpy(&hdr, kbuf + offset, size); - if (validate_xstate_header(&hdr) != 0) + if (validate_xstate_header(&hdr)) return -EINVAL; for (i = 0; i < XFEATURE_MAX; i++) { @@ -1181,7 +1205,7 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf) if (__copy_from_user(&hdr, ubuf + offset, size)) return -EFAULT; - if (validate_xstate_header(&hdr) != 0) + if (validate_xstate_header(&hdr)) return -EINVAL; for (i = 0; i < XFEATURE_MAX; i++) {