From: "Tobin C. Harding" <me@tobin.cc>
To: kernel-hardening@lists.openwall.com
Cc: "Tobin C. Harding" <me@tobin.cc>,
"Jason A. Donenfeld" <Jason@zx2c4.com>,
Theodore Ts'o <tytso@mit.edu>,
Linus Torvalds <torvalds@linux-foundation.org>,
Kees Cook <keescook@chromium.org>,
Paolo Bonzini <pbonzini@redhat.com>,
Tycho Andersen <tycho@docker.com>,
"Roberts, William C" <william.c.roberts@intel.com>,
Tejun Heo <tj@kernel.org>,
Jordan Glover <Golden_Miller83@protonmail.ch>,
Greg KH <gregkh@linuxfoundation.org>,
Petr Mladek <pmladek@suse.com>, Joe Perches <joe@perches.com>,
Ian Campbell <ijc@hellion.org.uk>,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <wilal.deacon@arm.com>,
Steven Rostedt <rostedt@goodmis.org>,
Chris Fries <cfries@google.com>,
Dave Weinstein <olorin@google.com>,
Daniel Micay <danielmicay@gmail.com>,
Djalal Harouni <tixxdz@gmail.com>,
linux-kernel@vger.kernel.org
Subject: [kernel-hardening] [PATCH v6] printk: hash addresses printed with %p
Date: Mon, 23 Oct 2017 10:28:32 +1100 [thread overview]
Message-ID: <1508714912-2266-1-git-send-email-me@tobin.cc> (raw)
Currently there are many places in the kernel where addresses are being
printed using an unadorned %p. Kernel pointers should be printed using
%pK allowing some control via the kptr_restrict sysctl. Exposing addresses
gives attackers sensitive information about the kernel layout in memory.
We can reduce the attack surface by hashing all addresses printed with
%p. This will of course break some users, forcing code printing needed
addresses to be updated.
For what it's worth, usage of unadorned %p can be broken down as
follows (thanks to Joe Perches).
$ git grep -E '%p[^A-Za-z0-9]' | cut -f1 -d"/" | sort | uniq -c
1084 arch
20 block
10 crypto
32 Documentation
8121 drivers
1221 fs
143 include
101 kernel
69 lib
100 mm
1510 net
40 samples
7 scripts
11 security
166 sound
152 tools
2 virt
Add function ptr_to_id() to map an address to a 32 bit unique identifier.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
V6:
- Use __early_initcall() to fill the SipHash key.
- Use static keys to guard hashing before the key is available.
V5:
- Remove spin lock.
- Add Jason A. Donenfeld to CC list by request.
- Add Theodore Ts'o to CC list due to comment on previous version.
V4:
- Remove changes to siphash.{ch}
- Do word size check, and return value cast, directly in ptr_to_id().
- Use add_ready_random_callback() to guard call to get_random_bytes()
V3:
- Use atomic_xchg() to guard setting [random] key.
- Remove erroneous white space change.
V2:
- Use SipHash to do the hashing.
The discussion related to this patch has been fragmented. There are
three threads associated with this patch. Email threads by subject:
[PATCH] printk: hash addresses printed with %p
[PATCH 0/3] add %pX specifier
[kernel-hardening] [RFC V2 0/6] add more kernel pointer filter options
lib/vsprintf.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 86c3385b9eb3..e2f712666882 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -33,6 +33,7 @@
#include <linux/uuid.h>
#include <linux/of.h>
#include <net/addrconf.h>
+#include <linux/siphash.h>
#ifdef CONFIG_BLOCK
#include <linux/blkdev.h>
#endif
@@ -1591,6 +1592,55 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
return widen_string(buf, buf - buf_start, end, spec);
}
+static siphash_key_t ptr_secret __read_mostly;
+static DEFINE_STATIC_KEY_TRUE(no_ptr_secret);
+
+static void fill_random_ptr_key(struct random_ready_callback *rdy)
+{
+ get_random_bytes(&ptr_secret, sizeof(ptr_secret));
+ static_branch_disable(&no_ptr_secret);
+}
+
+static struct random_ready_callback random_ready = {
+ .func = fill_random_ptr_key
+};
+
+static int __init initialize_ptr_random(void)
+{
+ int ret = add_random_ready_callback(&random_ready);
+
+ if (!ret)
+ return 0;
+ else if (ret == -EALREADY) {
+ fill_random_ptr_key(&random_ready);
+ return 0;
+ }
+
+ return ret;
+}
+early_initcall(initialize_ptr_random);
+
+/* Maps a pointer to a 32 bit unique identifier. */
+static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
+{
+ unsigned int hashval;
+
+ if (static_branch_unlikely(&no_ptr_secret))
+ return "(pointer value)";
+
+#ifdef CONFIG_64BIT
+ hashval = (unsigned int)siphash_1u64((u64)ptr, &ptr_secret);
+#else
+ hashval = (unsigned int)siphash_1u32((u32)ptr, &ptr_secret);
+#endif
+
+ spec.field_width = 2 * sizeof(unsigned int);
+ spec.flags = SMALL;
+ spec.base = 16;
+
+ return number(buf, end, hashval, spec);
+}
+
int kptr_restrict __read_mostly;
/*
@@ -1703,6 +1753,9 @@ int kptr_restrict __read_mostly;
* Note: The difference between 'S' and 'F' is that on ia64 and ppc64
* function pointers are really function descriptors, which contain a
* pointer to the real address.
+ *
+ * Note: The default behaviour (unadorned %p) is to hash the address,
+ * rendering it useful as a unique identifier.
*/
static noinline_for_stack
char *pointer(const char *fmt, char *buf, char *end, void *ptr,
@@ -1857,7 +1910,11 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
case 'F':
return device_node_string(buf, end, ptr, spec, fmt + 1);
}
+ default: /* default is to _not_ leak addresses, hash before printing */
+ return ptr_to_id(buf, end, ptr, spec);
}
+
+ /* OK, let's print the address */
spec.flags |= SMALL;
if (spec.field_width == -1) {
spec.field_width = default_width;
--
2.7.4
WARNING: multiple messages have this Message-ID (diff)
From: "Tobin C. Harding" <me@tobin.cc>
To: kernel-hardening@lists.openwall.com
Cc: "Tobin C. Harding" <me@tobin.cc>,
"Jason A. Donenfeld" <Jason@zx2c4.com>,
"Theodore Ts'o" <tytso@mit.edu>,
Linus Torvalds <torvalds@linux-foundation.org>,
Kees Cook <keescook@chromium.org>,
Paolo Bonzini <pbonzini@redhat.com>,
Tycho Andersen <tycho@docker.com>,
"Roberts, William C" <william.c.roberts@intel.com>,
Tejun Heo <tj@kernel.org>,
Jordan Glover <Golden_Miller83@protonmail.ch>,
Greg KH <gregkh@linuxfoundation.org>,
Petr Mladek <pmladek@suse.com>, Joe Perches <joe@perches.com>,
Ian Campbell <ijc@hellion.org.uk>,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <wilal.deacon@arm.com>,
Steven Rostedt <rostedt@goodmis.org>,
Chris Fries <cfries@google.com>,
Dave Weinstein <olorin@google.com>,
Daniel Micay <danielmicay@gmail.com>,
Djalal Harouni <tixxdz@gmail.com>,
linux-kernel@vger.kernel.org
Subject: [PATCH v6] printk: hash addresses printed with %p
Date: Mon, 23 Oct 2017 10:28:32 +1100 [thread overview]
Message-ID: <1508714912-2266-1-git-send-email-me@tobin.cc> (raw)
Currently there are many places in the kernel where addresses are being
printed using an unadorned %p. Kernel pointers should be printed using
%pK allowing some control via the kptr_restrict sysctl. Exposing addresses
gives attackers sensitive information about the kernel layout in memory.
We can reduce the attack surface by hashing all addresses printed with
%p. This will of course break some users, forcing code printing needed
addresses to be updated.
For what it's worth, usage of unadorned %p can be broken down as
follows (thanks to Joe Perches).
$ git grep -E '%p[^A-Za-z0-9]' | cut -f1 -d"/" | sort | uniq -c
1084 arch
20 block
10 crypto
32 Documentation
8121 drivers
1221 fs
143 include
101 kernel
69 lib
100 mm
1510 net
40 samples
7 scripts
11 security
166 sound
152 tools
2 virt
Add function ptr_to_id() to map an address to a 32 bit unique identifier.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
V6:
- Use __early_initcall() to fill the SipHash key.
- Use static keys to guard hashing before the key is available.
V5:
- Remove spin lock.
- Add Jason A. Donenfeld to CC list by request.
- Add Theodore Ts'o to CC list due to comment on previous version.
V4:
- Remove changes to siphash.{ch}
- Do word size check, and return value cast, directly in ptr_to_id().
- Use add_ready_random_callback() to guard call to get_random_bytes()
V3:
- Use atomic_xchg() to guard setting [random] key.
- Remove erroneous white space change.
V2:
- Use SipHash to do the hashing.
The discussion related to this patch has been fragmented. There are
three threads associated with this patch. Email threads by subject:
[PATCH] printk: hash addresses printed with %p
[PATCH 0/3] add %pX specifier
[kernel-hardening] [RFC V2 0/6] add more kernel pointer filter options
lib/vsprintf.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 86c3385b9eb3..e2f712666882 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -33,6 +33,7 @@
#include <linux/uuid.h>
#include <linux/of.h>
#include <net/addrconf.h>
+#include <linux/siphash.h>
#ifdef CONFIG_BLOCK
#include <linux/blkdev.h>
#endif
@@ -1591,6 +1592,55 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
return widen_string(buf, buf - buf_start, end, spec);
}
+static siphash_key_t ptr_secret __read_mostly;
+static DEFINE_STATIC_KEY_TRUE(no_ptr_secret);
+
+static void fill_random_ptr_key(struct random_ready_callback *rdy)
+{
+ get_random_bytes(&ptr_secret, sizeof(ptr_secret));
+ static_branch_disable(&no_ptr_secret);
+}
+
+static struct random_ready_callback random_ready = {
+ .func = fill_random_ptr_key
+};
+
+static int __init initialize_ptr_random(void)
+{
+ int ret = add_random_ready_callback(&random_ready);
+
+ if (!ret)
+ return 0;
+ else if (ret == -EALREADY) {
+ fill_random_ptr_key(&random_ready);
+ return 0;
+ }
+
+ return ret;
+}
+early_initcall(initialize_ptr_random);
+
+/* Maps a pointer to a 32 bit unique identifier. */
+static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
+{
+ unsigned int hashval;
+
+ if (static_branch_unlikely(&no_ptr_secret))
+ return "(pointer value)";
+
+#ifdef CONFIG_64BIT
+ hashval = (unsigned int)siphash_1u64((u64)ptr, &ptr_secret);
+#else
+ hashval = (unsigned int)siphash_1u32((u32)ptr, &ptr_secret);
+#endif
+
+ spec.field_width = 2 * sizeof(unsigned int);
+ spec.flags = SMALL;
+ spec.base = 16;
+
+ return number(buf, end, hashval, spec);
+}
+
int kptr_restrict __read_mostly;
/*
@@ -1703,6 +1753,9 @@ int kptr_restrict __read_mostly;
* Note: The difference between 'S' and 'F' is that on ia64 and ppc64
* function pointers are really function descriptors, which contain a
* pointer to the real address.
+ *
+ * Note: The default behaviour (unadorned %p) is to hash the address,
+ * rendering it useful as a unique identifier.
*/
static noinline_for_stack
char *pointer(const char *fmt, char *buf, char *end, void *ptr,
@@ -1857,7 +1910,11 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
case 'F':
return device_node_string(buf, end, ptr, spec, fmt + 1);
}
+ default: /* default is to _not_ leak addresses, hash before printing */
+ return ptr_to_id(buf, end, ptr, spec);
}
+
+ /* OK, let's print the address */
spec.flags |= SMALL;
if (spec.field_width == -1) {
spec.field_width = default_width;
--
2.7.4
next reply other threads:[~2017-10-22 23:28 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-22 23:28 Tobin C. Harding [this message]
2017-10-22 23:28 ` [PATCH v6] printk: hash addresses printed with %p Tobin C. Harding
2017-10-24 1:12 ` [lkp-robot] [printk] 7d827b898c: BUG:KASAN:slab-out-of-bounds kernel test robot
2017-10-24 1:12 ` 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=1508714912-2266-1-git-send-email-me@tobin.cc \
--to=me@tobin.cc \
--cc=Golden_Miller83@protonmail.ch \
--cc=Jason@zx2c4.com \
--cc=catalin.marinas@arm.com \
--cc=cfries@google.com \
--cc=danielmicay@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=ijc@hellion.org.uk \
--cc=joe@perches.com \
--cc=keescook@chromium.org \
--cc=kernel-hardening@lists.openwall.com \
--cc=linux-kernel@vger.kernel.org \
--cc=olorin@google.com \
--cc=pbonzini@redhat.com \
--cc=pmladek@suse.com \
--cc=rostedt@goodmis.org \
--cc=sergey.senozhatsky@gmail.com \
--cc=tixxdz@gmail.com \
--cc=tj@kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=tycho@docker.com \
--cc=tytso@mit.edu \
--cc=wilal.deacon@arm.com \
--cc=william.c.roberts@intel.com \
/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.