[SRU][X][PATCH 1/1] UBUNTU: SAUCE: Revert "printk: hash addresses printed with %p"
Kleber Sacilotto de Souza
kleber.souza at canonical.com
Thu Apr 1 09:13:51 UTC 2021
BugLink: https://bugs.launchpad.net/bugs/1922200
This reverts commit 5d742149ceb112c61ee576f371b574da32532c43 (commit
ad67b74d2469d9b82aaa572d76474c95bc484d57 upstream).
The backport of this upstream commit, applied to fix CVEs
CVE-2018-5953/CVE-2018-5995/CVE-2018-7754 on xenial/linux, introduced a
regression on the addresses exported via /proc interfaces (mainly
/proc/kallsyms). The patch leaks what the address 0x0 hashes to for
regular users instead of the expected zeroed out values. It also mangles
the default address for 'startup_64' expected to be 'ffffffff81000000'
for non-kaslr kernels (<4.15).
Signed-off-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>
---
Documentation/printk-formats.txt | 11 ----
lib/test_printf.c | 108 +++++++++++--------------------
lib/vsprintf.c | 81 ++---------------------
3 files changed, 45 insertions(+), 155 deletions(-)
diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
index fedb13fdb050..ed6f6abaad57 100644
--- a/Documentation/printk-formats.txt
+++ b/Documentation/printk-formats.txt
@@ -31,17 +31,6 @@ return from vsnprintf.
Raw pointer value SHOULD be printed with %p. The kernel supports
the following extended format specifiers for pointer types:
-Pointer Types
-=============
-
-Pointers printed without a specifier extension (i.e unadorned %p) are
-hashed to give a unique identifier without leaking kernel addresses to user
-space. On 64 bit machines the first 32 bits are zeroed.
-
-::
-
- %p abcdef12 or 00000000abcdef12
-
Symbols/Function Pointers:
%pF versatile_init+0x0/0x110
diff --git a/lib/test_printf.c b/lib/test_printf.c
index e2200f06f168..c5a666af9ba5 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -18,6 +18,24 @@
#define BUF_SIZE 256
#define FILL_CHAR '$'
+#define PTR1 ((void*)0x01234567)
+#define PTR2 ((void*)(long)(int)0xfedcba98)
+
+#if BITS_PER_LONG == 64
+#define PTR1_ZEROES "000000000"
+#define PTR1_SPACES " "
+#define PTR1_STR "1234567"
+#define PTR2_STR "fffffffffedcba98"
+#define PTR_WIDTH 16
+#else
+#define PTR1_ZEROES "0"
+#define PTR1_SPACES " "
+#define PTR1_STR "1234567"
+#define PTR2_STR "fedcba98"
+#define PTR_WIDTH 8
+#endif
+#define PTR_WIDTH_STR stringify(PTR_WIDTH)
+
static unsigned total_tests __initdata;
static unsigned failed_tests __initdata;
static char *test_buffer __initdata;
@@ -142,79 +160,30 @@ test_string(void)
test("a | | ", "%-3.s|%-3.0s|%-3.*s", "a", "b", 0, "c");
}
-#define PLAIN_BUF_SIZE 64 /* leave some space so we don't oops */
-
-#if BITS_PER_LONG == 64
-
-#define PTR_WIDTH 16
-#define PTR ((void *)0xffff0123456789ab)
-#define PTR_STR "ffff0123456789ab"
-#define ZEROS "00000000" /* hex 32 zero bits */
-
-static int __init
-plain_format(void)
-{
- char buf[PLAIN_BUF_SIZE];
- int nchars;
-
- nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR);
-
- if (nchars != PTR_WIDTH || strncmp(buf, ZEROS, strlen(ZEROS)) != 0)
- return -1;
-
- return 0;
-}
-
-#else
-
-#define PTR_WIDTH 8
-#define PTR ((void *)0x456789ab)
-#define PTR_STR "456789ab"
-
-static int __init
-plain_format(void)
-{
- /* Format is implicitly tested for 32 bit machines by plain_hash() */
- return 0;
-}
-
-#endif /* BITS_PER_LONG == 64 */
-
-static int __init
-plain_hash(void)
-{
- char buf[PLAIN_BUF_SIZE];
- int nchars;
-
- nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR);
-
- if (nchars != PTR_WIDTH || strncmp(buf, PTR_STR, PTR_WIDTH) == 0)
- return -1;
-
- return 0;
-}
-
-/*
- * We can't use test() to test %p because we don't know what output to expect
- * after an address is hashed.
- */
static void __init
plain(void)
{
- int err;
-
- err = plain_hash();
- if (err) {
- pr_warn("plain 'p' does not appear to be hashed\n");
- failed_tests++;
- return;
- }
+ test(PTR1_ZEROES PTR1_STR " " PTR2_STR, "%p %p", PTR1, PTR2);
+ /*
+ * The field width is overloaded for some %p extensions to
+ * pass another piece of information. For plain pointers, the
+ * behaviour is slightly odd: One cannot pass either the 0
+ * flag nor a precision to %p without gcc complaining, and if
+ * one explicitly gives a field width, the number is no longer
+ * zero-padded.
+ */
+ test("|" PTR1_STR PTR1_SPACES " | " PTR1_SPACES PTR1_STR "|",
+ "|%-*p|%*p|", PTR_WIDTH+2, PTR1, PTR_WIDTH+2, PTR1);
+ test("|" PTR2_STR " | " PTR2_STR "|",
+ "|%-*p|%*p|", PTR_WIDTH+2, PTR2, PTR_WIDTH+2, PTR2);
- err = plain_format();
- if (err) {
- pr_warn("hashing plain 'p' has unexpected format\n");
- failed_tests++;
- }
+ /*
+ * Unrecognized %p extensions are treated as plain %p, but the
+ * alphanumeric suffix is ignored (that is, does not occur in
+ * the output.)
+ */
+ test("|"PTR1_ZEROES PTR1_STR"|", "|%p0y|", PTR1);
+ test("|"PTR2_STR"|", "|%p0y|", PTR2);
}
static void __init
@@ -225,7 +194,6 @@ symbol_ptr(void)
static void __init
kernel_ptr(void)
{
- /* We can't test this without access to kptr_restrict. */
}
static void __init
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 698beaccbc37..646009db4198 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -31,8 +31,6 @@
#include <linux/dcache.h>
#include <linux/cred.h>
#include <net/addrconf.h>
-#include <linux/siphash.h>
-#include <linux/compiler.h>
#include <asm/page.h> /* for PAGE_SIZE */
#include <asm/sections.h> /* for dereference_function_descriptor() */
@@ -1362,73 +1360,6 @@ char *clock(char *buf, char *end, struct clk *clk, struct printf_spec spec,
int kptr_restrict __read_mostly;
-static bool have_filled_random_ptr_key __read_mostly;
-static siphash_key_t ptr_key __read_mostly;
-
-static void fill_random_ptr_key(struct random_ready_callback *unused)
-{
- get_random_bytes(&ptr_key, sizeof(ptr_key));
- /*
- * have_filled_random_ptr_key==true is dependent on get_random_bytes().
- * ptr_to_id() needs to see have_filled_random_ptr_key==true
- * after get_random_bytes() returns.
- */
- smp_mb();
- WRITE_ONCE(have_filled_random_ptr_key, true);
-}
-
-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 long hashval;
- const int default_width = 2 * sizeof(ptr);
-
- if (unlikely(!have_filled_random_ptr_key)) {
- spec.field_width = default_width;
- /* string length must be less than default_width */
- return string(buf, end, "(ptrval)", 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
-
- spec.flags |= SMALL;
- if (spec.field_width == -1) {
- spec.field_width = default_width;
- spec.flags |= ZEROPAD;
- }
- spec.base = 16;
-
- return number(buf, end, 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
@@ -1520,9 +1451,6 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
* 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,
@@ -1670,9 +1598,14 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
((const struct file *)ptr)->f_path.dentry,
spec, fmt);
}
+ spec.flags |= SMALL;
+ if (spec.field_width == -1) {
+ spec.field_width = default_width;
+ spec.flags |= ZEROPAD;
+ }
+ spec.base = 16;
- /* default is to _not_ leak addresses, hash before printing */
- return ptr_to_id(buf, end, ptr, spec);
+ return number(buf, end, (unsigned long) ptr, spec);
}
/*
--
2.27.0
More information about the kernel-team
mailing list