All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <dave.hansen@linux.intel.com>
To: linux-kernel@vger.kernel.org
Cc: Dave Hansen <dave.hansen@linux.intel.com>,
	alex.shi@linux.alibaba.com, tglx@linutronix.de, mingo@redhat.com,
	bp@alien8.de, hpa@zytor.com, x86@kernel.org,
	bigeasy@linutronix.de, gorcunov@gmail.com,
	pankaj.laxminarayan.bharadiya@intel.com,
	aubrey.li@linux.intel.com, dave.hansen@intel.com
Subject: [PATCH] x86/pkeys: add check for pkey "overflow"
Date: Wed, 22 Jan 2020 08:53:46 -0800	[thread overview]
Message-ID: <20200122165346.AD4DA150@viggo.jf.intel.com> (raw)


Alex Shi reported the pkey macros above arch_set_user_pkey_access()
to be unused.  They are unused, and even refer to a nonexistent
CONFIG option.

But, they might have served a good use, which was to ensure that
the code does not try to set values that would not fit in the
PKRU register.  As it stands, a too-large 'pkey' value would
be likely to silently overflow the u32 new_pkru_bits.

Add a check to look for overflows.  Also add a comment to remind
any future developer to closely examine the types used to store
pkey values if arch_max_pkey() ever changes.

This boots and passes the x86 pkey selftests.

Reported-by: Alex Shi <alex.shi@linux.alibaba.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
Cc: Aubrey Li <aubrey.li@linux.intel.com>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Dave Hansen <dave.hansen@intel.com>
---

 b/arch/x86/include/asm/pkeys.h |    5 +++++
 b/arch/x86/kernel/fpu/xstate.c |    9 +++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff -puN arch/x86/kernel/fpu/xstate.c~pkey-check-pkru-shift arch/x86/kernel/fpu/xstate.c
--- a/arch/x86/kernel/fpu/xstate.c~pkey-check-pkru-shift	2020-01-21 09:20:26.542385466 -0800
+++ b/arch/x86/kernel/fpu/xstate.c	2020-01-21 09:28:18.068384290 -0800
@@ -902,8 +902,6 @@ const void *get_xsave_field_ptr(int xfea
 
 #ifdef CONFIG_ARCH_HAS_PKEYS
 
-#define NR_VALID_PKRU_BITS (CONFIG_NR_PROTECTION_KEYS * 2)
-#define PKRU_VALID_MASK (NR_VALID_PKRU_BITS - 1)
 /*
  * This will go out and modify PKRU register to set the access
  * rights for @pkey to @init_val.
@@ -922,6 +920,13 @@ int arch_set_user_pkey_access(struct tas
 	if (!boot_cpu_has(X86_FEATURE_OSPKE))
 		return -EINVAL;
 
+	/*
+	 * This code should only be called with valid 'pkey'
+	 * values originating from in-kernel users.  Complain
+	 * if a bad value is observed.
+	 */
+	WARN_ON_ONCE(pkey >= arch_max_pkey());
+
 	/* Set the bits we need in PKRU:  */
 	if (init_val & PKEY_DISABLE_ACCESS)
 		new_pkru_bits |= PKRU_AD_BIT;
diff -puN arch/x86/include/asm/pkeys.h~pkey-check-pkru-shift arch/x86/include/asm/pkeys.h
--- a/arch/x86/include/asm/pkeys.h~pkey-check-pkru-shift	2020-01-21 09:23:36.733384991 -0800
+++ b/arch/x86/include/asm/pkeys.h	2020-01-21 09:41:44.797382278 -0800
@@ -4,6 +4,11 @@
 
 #define ARCH_DEFAULT_PKEY	0
 
+/*
+ * If more than 16 keys are ever supported, a thorough audit
+ * will be necessary to ensure that the types that store key
+ * numbers and masks have sufficient capacity.
+ */
 #define arch_max_pkey() (boot_cpu_has(X86_FEATURE_OSPKE) ? 16 : 1)
 
 extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
_

             reply	other threads:[~2020-01-22 16:54 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-22 16:53 Dave Hansen [this message]
2020-01-22 18:51 ` [PATCH] x86/pkeys: add check for pkey "overflow" Cyrill Gorcunov
2020-01-22 19:09   ` Dave Hansen
2020-01-22 19:29     ` Cyrill Gorcunov
2020-02-24 19:38 ` [tip: x86/fpu] x86/pkeys: Add " tip-bot2 for Dave Hansen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200122165346.AD4DA150@viggo.jf.intel.com \
    --to=dave.hansen@linux.intel.com \
    --cc=alex.shi@linux.alibaba.com \
    --cc=aubrey.li@linux.intel.com \
    --cc=bigeasy@linutronix.de \
    --cc=bp@alien8.de \
    --cc=dave.hansen@intel.com \
    --cc=gorcunov@gmail.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pankaj.laxminarayan.bharadiya@intel.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.