All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeremy Linton <jeremy.linton@arm.com>
To: linux-arm-kernel@lists.infradead.org
Cc: catalin.marinas@arm.com, will@kernel.org, keescook@chromium.org,
	Jason@zx2c4.com, gustavoars@kernel.org, mark.rutland@arm.com,
	rostedt@goodmis.org, arnd@arndb.de, broonie@kernel.org,
	guohui@uniontech.com, Manoj.Iyer@arm.com,
	linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org,
	Jeremy Linton <jeremy.linton@arm.com>,
	James Yang <james.yang@arm.com>,
	Shiyou Huang <shiyou.huang@arm.com>
Subject: [PATCH 1/1] arm64: syscall: Direct PRNG kstack randomization
Date: Tue,  5 Mar 2024 16:18:24 -0600	[thread overview]
Message-ID: <20240305221824.3300322-2-jeremy.linton@arm.com> (raw)
In-Reply-To: <20240305221824.3300322-1-jeremy.linton@arm.com>

The existing arm64 stack randomization uses the kernel rng to acquire
5 bits of address space randomization. This is problematic because it
creates non determinism in the syscall path when the rng needs to be
generated or reseeded. This shows up as large tail latencies in some
benchmarks and directly affects the minimum RT latencies as seen by
cyclictest.

Other architectures are using timers/cycle counters for this function,
which is sketchy from a randomization perspective because it should be
possible to estimate this value from knowledge of the syscall return
time, and from reading the current value of the timer/counters.

So, a poor rng should be better than the cycle counter if it is hard
to extract the stack offsets sufficiently to be able to detect the
PRNG's period. Lets downgrade from get_random_u16() to
prandom_u32_state() under the theory that the danger of someone
guessing the 1 in 32 per call offset, is larger than that of being
able to extract sufficient history to accurately predict future
offsets. Further it should be safer to run with prandom_u32_state than
disabling stack randomization for those subset of applications where the
difference in latency is on the order of ~5X worse.

Reported-by: James Yang <james.yang@arm.com>
Reported-by: Shiyou Huang <shiyou.huang@arm.com>
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 arch/arm64/kernel/syscall.c | 42 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
index 9a70d9746b66..33b3ea4adff8 100644
--- a/arch/arm64/kernel/syscall.c
+++ b/arch/arm64/kernel/syscall.c
@@ -5,6 +5,7 @@
 #include <linux/errno.h>
 #include <linux/nospec.h>
 #include <linux/ptrace.h>
+#include <linux/prandom.h>
 #include <linux/randomize_kstack.h>
 #include <linux/syscalls.h>
 
@@ -37,6 +38,45 @@ static long __invoke_syscall(struct pt_regs *regs, syscall_fn_t syscall_fn)
 	return syscall_fn(regs);
 }
 
+#ifdef CONFIG_RANDOMIZE_KSTACK_OFFSET
+DEFINE_PER_CPU(struct rnd_state, kstackrng);
+
+static u16 kstack_rng(void)
+{
+	u32 rng = prandom_u32_state(this_cpu_ptr(&kstackrng));
+
+	return rng & 0x1ff;
+}
+
+/* Should we reseed? */
+static int kstack_rng_setup(unsigned int cpu)
+{
+	u32 rng_seed;
+
+	/* zero should be avoided as a seed */
+	do {
+		rng_seed = get_random_u32();
+	} while (!rng_seed);
+	prandom_seed_state(this_cpu_ptr(&kstackrng), rng_seed);
+	return 0;
+}
+
+static int kstack_init(void)
+{
+	int ret;
+
+	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "arm64/cpuinfo:kstackrandomize",
+				kstack_rng_setup, NULL);
+	if (ret < 0)
+		pr_err("kstack: failed to register rng callbacks.\n");
+	return 0;
+}
+
+arch_initcall(kstack_init);
+#else
+static u16 kstack_rng(void) { return 0; }
+#endif /* CONFIG_RANDOMIZE_KSTACK_OFFSET */
+
 static void invoke_syscall(struct pt_regs *regs, unsigned int scno,
 			   unsigned int sc_nr,
 			   const syscall_fn_t syscall_table[])
@@ -66,7 +106,7 @@ static void invoke_syscall(struct pt_regs *regs, unsigned int scno,
 	 *
 	 * The resulting 5 bits of entropy is seen in SP[8:4].
 	 */
-	choose_random_kstack_offset(get_random_u16() & 0x1FF);
+	choose_random_kstack_offset(kstack_rng());
 }
 
 static inline bool has_syscall_work(unsigned long flags)
-- 
2.43.0


WARNING: multiple messages have this Message-ID (diff)
From: Jeremy Linton <jeremy.linton@arm.com>
To: linux-arm-kernel@lists.infradead.org
Cc: catalin.marinas@arm.com, will@kernel.org, keescook@chromium.org,
	Jason@zx2c4.com, gustavoars@kernel.org, mark.rutland@arm.com,
	rostedt@goodmis.org, arnd@arndb.de, broonie@kernel.org,
	guohui@uniontech.com, Manoj.Iyer@arm.com,
	linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org,
	Jeremy Linton <jeremy.linton@arm.com>,
	James Yang <james.yang@arm.com>,
	Shiyou Huang <shiyou.huang@arm.com>
Subject: [PATCH 1/1] arm64: syscall: Direct PRNG kstack randomization
Date: Tue,  5 Mar 2024 16:18:24 -0600	[thread overview]
Message-ID: <20240305221824.3300322-2-jeremy.linton@arm.com> (raw)
In-Reply-To: <20240305221824.3300322-1-jeremy.linton@arm.com>

The existing arm64 stack randomization uses the kernel rng to acquire
5 bits of address space randomization. This is problematic because it
creates non determinism in the syscall path when the rng needs to be
generated or reseeded. This shows up as large tail latencies in some
benchmarks and directly affects the minimum RT latencies as seen by
cyclictest.

Other architectures are using timers/cycle counters for this function,
which is sketchy from a randomization perspective because it should be
possible to estimate this value from knowledge of the syscall return
time, and from reading the current value of the timer/counters.

So, a poor rng should be better than the cycle counter if it is hard
to extract the stack offsets sufficiently to be able to detect the
PRNG's period. Lets downgrade from get_random_u16() to
prandom_u32_state() under the theory that the danger of someone
guessing the 1 in 32 per call offset, is larger than that of being
able to extract sufficient history to accurately predict future
offsets. Further it should be safer to run with prandom_u32_state than
disabling stack randomization for those subset of applications where the
difference in latency is on the order of ~5X worse.

Reported-by: James Yang <james.yang@arm.com>
Reported-by: Shiyou Huang <shiyou.huang@arm.com>
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 arch/arm64/kernel/syscall.c | 42 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
index 9a70d9746b66..33b3ea4adff8 100644
--- a/arch/arm64/kernel/syscall.c
+++ b/arch/arm64/kernel/syscall.c
@@ -5,6 +5,7 @@
 #include <linux/errno.h>
 #include <linux/nospec.h>
 #include <linux/ptrace.h>
+#include <linux/prandom.h>
 #include <linux/randomize_kstack.h>
 #include <linux/syscalls.h>
 
@@ -37,6 +38,45 @@ static long __invoke_syscall(struct pt_regs *regs, syscall_fn_t syscall_fn)
 	return syscall_fn(regs);
 }
 
+#ifdef CONFIG_RANDOMIZE_KSTACK_OFFSET
+DEFINE_PER_CPU(struct rnd_state, kstackrng);
+
+static u16 kstack_rng(void)
+{
+	u32 rng = prandom_u32_state(this_cpu_ptr(&kstackrng));
+
+	return rng & 0x1ff;
+}
+
+/* Should we reseed? */
+static int kstack_rng_setup(unsigned int cpu)
+{
+	u32 rng_seed;
+
+	/* zero should be avoided as a seed */
+	do {
+		rng_seed = get_random_u32();
+	} while (!rng_seed);
+	prandom_seed_state(this_cpu_ptr(&kstackrng), rng_seed);
+	return 0;
+}
+
+static int kstack_init(void)
+{
+	int ret;
+
+	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "arm64/cpuinfo:kstackrandomize",
+				kstack_rng_setup, NULL);
+	if (ret < 0)
+		pr_err("kstack: failed to register rng callbacks.\n");
+	return 0;
+}
+
+arch_initcall(kstack_init);
+#else
+static u16 kstack_rng(void) { return 0; }
+#endif /* CONFIG_RANDOMIZE_KSTACK_OFFSET */
+
 static void invoke_syscall(struct pt_regs *regs, unsigned int scno,
 			   unsigned int sc_nr,
 			   const syscall_fn_t syscall_table[])
@@ -66,7 +106,7 @@ static void invoke_syscall(struct pt_regs *regs, unsigned int scno,
 	 *
 	 * The resulting 5 bits of entropy is seen in SP[8:4].
 	 */
-	choose_random_kstack_offset(get_random_u16() & 0x1FF);
+	choose_random_kstack_offset(kstack_rng());
 }
 
 static inline bool has_syscall_work(unsigned long flags)
-- 
2.43.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2024-03-05 22:18 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-05 22:18 [PATCH 0/1] Bring kstack randomized perf closer to unrandomized Jeremy Linton
2024-03-05 22:18 ` Jeremy Linton
2024-03-05 22:18 ` Jeremy Linton [this message]
2024-03-05 22:18   ` [PATCH 1/1] arm64: syscall: Direct PRNG kstack randomization Jeremy Linton
2024-03-05 23:33   ` Kees Cook
2024-03-05 23:33     ` Kees Cook
2024-03-06 20:46     ` Arnd Bergmann
2024-03-06 20:46       ` Arnd Bergmann
2024-03-06 21:54       ` Jeremy Linton
2024-03-06 21:54         ` Jeremy Linton
2024-03-07 11:10         ` Arnd Bergmann
2024-03-07 11:10           ` Arnd Bergmann
2024-03-07 19:10           ` Kees Cook
2024-03-07 19:10             ` Kees Cook
2024-03-07 21:56             ` Arnd Bergmann
2024-03-07 21:56               ` Arnd Bergmann
2024-03-07 19:15           ` Kees Cook
2024-03-07 19:15             ` Kees Cook
2024-03-07 22:02             ` Arnd Bergmann
2024-03-07 22:02               ` Arnd Bergmann
2024-03-08 16:49           ` Jeremy Linton
2024-03-08 16:49             ` Jeremy Linton
2024-03-08 20:29             ` Arnd Bergmann
2024-03-08 20:29               ` Arnd Bergmann
2024-03-22 23:40               ` Jeremy Linton
2024-03-22 23:40                 ` Jeremy Linton
2024-03-23 12:47                 ` Arnd Bergmann
2024-03-23 12:47                   ` Arnd Bergmann
2024-03-07 19:05   ` kernel test robot
2024-03-07 19:05     ` kernel test robot

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=20240305221824.3300322-2-jeremy.linton@arm.com \
    --to=jeremy.linton@arm.com \
    --cc=Jason@zx2c4.com \
    --cc=Manoj.Iyer@arm.com \
    --cc=arnd@arndb.de \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=guohui@uniontech.com \
    --cc=gustavoars@kernel.org \
    --cc=james.yang@arm.com \
    --cc=keescook@chromium.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=rostedt@goodmis.org \
    --cc=shiyou.huang@arm.com \
    --cc=will@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.