* [PATCH v2 1/3] lib/vsprintf: Prepare for more general use of ptr_to_id()
2018-10-11 8:42 [PATCH v2 0/3] lib/vsprintf: Hash remaining raw addresses Geert Uytterhoeven
@ 2018-10-11 8:42 ` Geert Uytterhoeven
2018-10-12 10:39 ` Petr Mladek
2018-10-11 8:42 ` [PATCH v2 2/3] lib/vsprintf: Hash legacy clock addresses Geert Uytterhoeven
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Geert Uytterhoeven @ 2018-10-11 8:42 UTC (permalink / raw)
To: Petr Mladek, Andy Shevchenko, Tobin C . Harding, Andrew Morton,
Jonathan Corbet
Cc: linux-doc, linux-kernel, Geert Uytterhoeven
- Make the ptr argument const, to avoid adding casts in future
callers,
- Move the function and its dependencies up, so it can be called from
special pointer type formatting routines.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
- Get rid of the forward declaration for ptr_to_id().
---
lib/vsprintf.c | 205 +++++++++++++++++++++++++------------------------
1 file changed, 103 insertions(+), 102 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 095a677f89c02442..b4e4ab274bde2397 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -612,6 +612,109 @@ char *string(char *buf, char *end, const char *s, struct printf_spec spec)
return widen_string(buf, len, end, spec);
}
+static noinline_for_stack
+char *pointer_string(char *buf, char *end, const void *ptr,
+ struct printf_spec spec)
+{
+ spec.base = 16;
+ spec.flags |= SMALL;
+ if (spec.field_width == -1) {
+ spec.field_width = 2 * sizeof(ptr);
+ spec.flags |= ZEROPAD;
+ }
+
+ return number(buf, end, (unsigned long int)ptr, spec);
+}
+
+/* Make pointers available for printing early in the boot sequence. */
+static int debug_boot_weak_hash __ro_after_init;
+
+static int __init debug_boot_weak_hash_enable(char *str)
+{
+ debug_boot_weak_hash = 1;
+ pr_info("debug_boot_weak_hash enabled\n");
+ return 0;
+}
+early_param("debug_boot_weak_hash", debug_boot_weak_hash_enable);
+
+static DEFINE_STATIC_KEY_TRUE(not_filled_random_ptr_key);
+static siphash_key_t ptr_key __read_mostly;
+
+static void enable_ptr_key_workfn(struct work_struct *work)
+{
+ get_random_bytes(&ptr_key, sizeof(ptr_key));
+ /* Needs to run from preemptible context */
+ static_branch_disable(¬_filled_random_ptr_key);
+}
+
+static DECLARE_WORK(enable_ptr_key_work, enable_ptr_key_workfn);
+
+static void fill_random_ptr_key(struct random_ready_callback *unused)
+{
+ /* This may be in an interrupt handler. */
+ queue_work(system_unbound_wq, &enable_ptr_key_work);
+}
+
+static struct random_ready_callback random_ready = {
+ .func = fill_random_ptr_key
+};
+
+static int __init initialize_ptr_random(void)
+{
+ int key_size = sizeof(ptr_key);
+ int ret;
+
+ /* Use hw RNG if available. */
+ if (get_random_bytes_arch(&ptr_key, key_size) == key_size) {
+ static_branch_disable(¬_filled_random_ptr_key);
+ return 0;
+ }
+
+ ret = add_random_ready_callback(&random_ready);
+ if (!ret) {
+ return 0;
+ } else if (ret == -EALREADY) {
+ /* This is in preemptible context */
+ enable_ptr_key_workfn(&enable_ptr_key_work);
+ 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, const void *ptr,
+ struct printf_spec spec)
+{
+ const char *str = sizeof(ptr) == 8 ? "(____ptrval____)" : "(ptrval)";
+ unsigned long hashval;
+
+ /* When debugging early boot use non-cryptographically secure hash. */
+ if (unlikely(debug_boot_weak_hash)) {
+ hashval = hash_long((unsigned long)ptr, 32);
+ return pointer_string(buf, end, (const void *)hashval, spec);
+ }
+
+ if (static_branch_unlikely(¬_filled_random_ptr_key)) {
+ spec.field_width = 2 * sizeof(ptr);
+ /* string length must be less than default_width */
+ return string(buf, end, str, spec);
+ }
+
+#ifdef CONFIG_64BIT
+ hashval = (unsigned long)siphash_1u64((u64)ptr, &ptr_key);
+ /*
+ * Mask off the first 32 bits, this makes explicit that we have
+ * modified the address (and 32 bits is plenty for a unique ID).
+ */
+ hashval = hashval & 0xffffffff;
+#else
+ hashval = (unsigned long)siphash_1u32((u32)ptr, &ptr_key);
+#endif
+ return pointer_string(buf, end, (const void *)hashval, spec);
+}
+
static noinline_for_stack
char *dentry_name(char *buf, char *end, const struct dentry *d, struct printf_spec spec,
const char *fmt)
@@ -1357,20 +1460,6 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
return string(buf, end, uuid, spec);
}
-static noinline_for_stack
-char *pointer_string(char *buf, char *end, const void *ptr,
- struct printf_spec spec)
-{
- spec.base = 16;
- spec.flags |= SMALL;
- if (spec.field_width == -1) {
- spec.field_width = 2 * sizeof(ptr);
- spec.flags |= ZEROPAD;
- }
-
- return number(buf, end, (unsigned long int)ptr, spec);
-}
-
int kptr_restrict __read_mostly;
static noinline_for_stack
@@ -1656,94 +1745,6 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
return widen_string(buf, buf - buf_start, end, spec);
}
-/* Make pointers available for printing early in the boot sequence. */
-static int debug_boot_weak_hash __ro_after_init;
-
-static int __init debug_boot_weak_hash_enable(char *str)
-{
- debug_boot_weak_hash = 1;
- pr_info("debug_boot_weak_hash enabled\n");
- return 0;
-}
-early_param("debug_boot_weak_hash", debug_boot_weak_hash_enable);
-
-static DEFINE_STATIC_KEY_TRUE(not_filled_random_ptr_key);
-static siphash_key_t ptr_key __read_mostly;
-
-static void enable_ptr_key_workfn(struct work_struct *work)
-{
- get_random_bytes(&ptr_key, sizeof(ptr_key));
- /* Needs to run from preemptible context */
- static_branch_disable(¬_filled_random_ptr_key);
-}
-
-static DECLARE_WORK(enable_ptr_key_work, enable_ptr_key_workfn);
-
-static void fill_random_ptr_key(struct random_ready_callback *unused)
-{
- /* This may be in an interrupt handler. */
- queue_work(system_unbound_wq, &enable_ptr_key_work);
-}
-
-static struct random_ready_callback random_ready = {
- .func = fill_random_ptr_key
-};
-
-static int __init initialize_ptr_random(void)
-{
- int key_size = sizeof(ptr_key);
- int ret;
-
- /* Use hw RNG if available. */
- if (get_random_bytes_arch(&ptr_key, key_size) == key_size) {
- static_branch_disable(¬_filled_random_ptr_key);
- return 0;
- }
-
- ret = add_random_ready_callback(&random_ready);
- if (!ret) {
- return 0;
- } else if (ret == -EALREADY) {
- /* This is in preemptible context */
- enable_ptr_key_workfn(&enable_ptr_key_work);
- 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)
-{
- const char *str = sizeof(ptr) == 8 ? "(____ptrval____)" : "(ptrval)";
- unsigned long hashval;
-
- /* When debugging early boot use non-cryptographically secure hash. */
- if (unlikely(debug_boot_weak_hash)) {
- hashval = hash_long((unsigned long)ptr, 32);
- return pointer_string(buf, end, (const void *)hashval, spec);
- }
-
- if (static_branch_unlikely(¬_filled_random_ptr_key)) {
- spec.field_width = 2 * sizeof(ptr);
- /* string length must be less than default_width */
- return string(buf, end, str, spec);
- }
-
-#ifdef CONFIG_64BIT
- hashval = (unsigned long)siphash_1u64((u64)ptr, &ptr_key);
- /*
- * Mask off the first 32 bits, this makes explicit that we have
- * modified the address (and 32 bits is plenty for a unique ID).
- */
- hashval = hashval & 0xffffffff;
-#else
- hashval = (unsigned long)siphash_1u32((u32)ptr, &ptr_key);
-#endif
- return pointer_string(buf, end, (const void *)hashval, spec);
-}
-
/*
* Show a '%p' thing. A kernel extension is that the '%p' is followed
* by an extra set of alphanumeric characters that are extended format
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] lib/vsprintf: Hash legacy clock addresses
2018-10-11 8:42 [PATCH v2 0/3] lib/vsprintf: Hash remaining raw addresses Geert Uytterhoeven
2018-10-11 8:42 ` [PATCH v2 1/3] lib/vsprintf: Prepare for more general use of ptr_to_id() Geert Uytterhoeven
@ 2018-10-11 8:42 ` Geert Uytterhoeven
2018-10-12 10:39 ` Petr Mladek
2018-10-11 8:42 ` [PATCH v2 3/3] lib/vsprintf: Hash printed address for netdev bits fallback Geert Uytterhoeven
2018-10-11 17:01 ` [PATCH v2 0/3] lib/vsprintf: Hash remaining raw addresses Andy Shevchenko
3 siblings, 1 reply; 9+ messages in thread
From: Geert Uytterhoeven @ 2018-10-11 8:42 UTC (permalink / raw)
To: Petr Mladek, Andy Shevchenko, Tobin C . Harding, Andrew Morton,
Jonathan Corbet
Cc: linux-doc, linux-kernel, Geert Uytterhoeven
On platforms using the Common Clock Framework, "%pC" prints the clock's
name. On legacy platforms, it prints the unhashed clock's address,
potentially leaking sensitive information regarding the kernel layout in
memory.
Avoid this leak by printing the hashed address instead. To distinguish
between clocks, a 32-bit unique identifier is as good as an actual
pointer value.
Fixes: ad67b74d2469d9b8 ("printk: hash addresses printed with %p")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
v2:
- Add Reviewed-by.
---
Documentation/core-api/printk-formats.rst | 5 ++---
lib/vsprintf.c | 2 +-
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 25dc591cb1108790..d39798c2558500d5 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -420,9 +420,8 @@ struct clk
%pC pll1
%pCn pll1
-For printing struct clk structures. %pC and %pCn print the name
-(Common Clock Framework) or address (legacy clock framework) of the
-structure.
+For printing struct clk structures. %pC and %pCn print the name of the clock
+(Common Clock Framework) or a unique 32-bit ID (legacy clock framework).
Passed by reference.
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index b4e4ab274bde2397..3ea2119b85ac2e4f 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1563,7 +1563,7 @@ char *clock(char *buf, char *end, struct clk *clk, struct printf_spec spec,
#ifdef CONFIG_COMMON_CLK
return string(buf, end, __clk_get_name(clk), spec);
#else
- return special_hex_number(buf, end, (unsigned long)clk, sizeof(unsigned long));
+ return ptr_to_id(buf, end, clk, spec);
#endif
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] lib/vsprintf: Hash printed address for netdev bits fallback
2018-10-11 8:42 [PATCH v2 0/3] lib/vsprintf: Hash remaining raw addresses Geert Uytterhoeven
2018-10-11 8:42 ` [PATCH v2 1/3] lib/vsprintf: Prepare for more general use of ptr_to_id() Geert Uytterhoeven
2018-10-11 8:42 ` [PATCH v2 2/3] lib/vsprintf: Hash legacy clock addresses Geert Uytterhoeven
@ 2018-10-11 8:42 ` Geert Uytterhoeven
2018-10-12 10:40 ` Petr Mladek
2018-10-11 17:01 ` [PATCH v2 0/3] lib/vsprintf: Hash remaining raw addresses Andy Shevchenko
3 siblings, 1 reply; 9+ messages in thread
From: Geert Uytterhoeven @ 2018-10-11 8:42 UTC (permalink / raw)
To: Petr Mladek, Andy Shevchenko, Tobin C . Harding, Andrew Morton,
Jonathan Corbet
Cc: linux-doc, linux-kernel, Geert Uytterhoeven
The handler for "%pN" falls back to printing the raw pointer value when
using a different format than the (sole supported) special format
"%pNF", potentially leaking sensitive information regarding the kernel
layout in memory.
Avoid this leak by printing the hashed address instead.
Note that there are no in-tree users of the fallback.
Fixes: ad67b74d2469d9b8 ("printk: hash addresses printed with %p")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
v2:
- Add Reviewed-by.
---
lib/vsprintf.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 3ea2119b85ac2e4f..4c75d847c1453ca4 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1510,7 +1510,8 @@ char *restricted_pointer(char *buf, char *end, const void *ptr,
}
static noinline_for_stack
-char *netdev_bits(char *buf, char *end, const void *addr, const char *fmt)
+char *netdev_bits(char *buf, char *end, const void *addr,
+ struct printf_spec spec, const char *fmt)
{
unsigned long long num;
int size;
@@ -1521,9 +1522,7 @@ char *netdev_bits(char *buf, char *end, const void *addr, const char *fmt)
size = sizeof(netdev_features_t);
break;
default:
- num = (unsigned long)addr;
- size = sizeof(unsigned long);
- break;
+ return ptr_to_id(buf, end, addr, spec);
}
return special_hex_number(buf, end, num, size);
@@ -1949,7 +1948,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
break;
return restricted_pointer(buf, end, ptr, spec);
case 'N':
- return netdev_bits(buf, end, ptr, fmt);
+ return netdev_bits(buf, end, ptr, spec, fmt);
case 'a':
return address_val(buf, end, ptr, fmt);
case 'd':
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread