Commit 35beccf0 authored by Marco Elver's avatar Marco Elver Committed by Linus Torvalds

kfence: report sensitive information based on no_hash_pointers

We cannot rely on CONFIG_DEBUG_KERNEL to decide if we're running a "debug
kernel" where we can safely show potentially sensitive information in the
kernel log.

Instead, simply rely on the newly introduced "no_hash_pointers" to print
unhashed kernel pointers, as well as decide if our reports can include
other potentially sensitive information such as registers and corrupted
bytes.

Link: https://lkml.kernel.org/r/20210223082043.1972742-1-elver@google.comSigned-off-by: default avatarMarco Elver <elver@google.com>
Cc: Timur Tabi <timur@kernel.org>
Cc: Alexander Potapenko <glider@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: Jann Horn <jannh@google.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 0825c1d5
...@@ -88,8 +88,8 @@ A typical out-of-bounds access looks like this:: ...@@ -88,8 +88,8 @@ A typical out-of-bounds access looks like this::
The header of the report provides a short summary of the function involved in The header of the report provides a short summary of the function involved in
the access. It is followed by more detailed information about the access and the access. It is followed by more detailed information about the access and
its origin. Note that, real kernel addresses are only shown for its origin. Note that, real kernel addresses are only shown when using the
``CONFIG_DEBUG_KERNEL=y`` builds. kernel command line option ``no_hash_pointers``.
Use-after-free accesses are reported as:: Use-after-free accesses are reported as::
...@@ -184,8 +184,8 @@ invalidly written bytes (offset from the address) are shown; in this ...@@ -184,8 +184,8 @@ invalidly written bytes (offset from the address) are shown; in this
representation, '.' denote untouched bytes. In the example above ``0xac`` is representation, '.' denote untouched bytes. In the example above ``0xac`` is
the value written to the invalid address at offset 0, and the remaining '.' the value written to the invalid address at offset 0, and the remaining '.'
denote that no following bytes have been touched. Note that, real values are denote that no following bytes have been touched. Note that, real values are
only shown for ``CONFIG_DEBUG_KERNEL=y`` builds; to avoid information only shown if the kernel was booted with ``no_hash_pointers``; to avoid
disclosure for non-debug builds, '!' is used instead to denote invalidly information disclosure otherwise, '!' is used instead to denote invalidly
written bytes. written bytes.
And finally, KFENCE may also report on invalid accesses to any protected page And finally, KFENCE may also report on invalid accesses to any protected page
......
...@@ -646,13 +646,9 @@ void __init kfence_init(void) ...@@ -646,13 +646,9 @@ void __init kfence_init(void)
WRITE_ONCE(kfence_enabled, true); WRITE_ONCE(kfence_enabled, true);
schedule_delayed_work(&kfence_timer, 0); schedule_delayed_work(&kfence_timer, 0);
pr_info("initialized - using %lu bytes for %d objects", KFENCE_POOL_SIZE, pr_info("initialized - using %lu bytes for %d objects at 0x%p-0x%p\n", KFENCE_POOL_SIZE,
CONFIG_KFENCE_NUM_OBJECTS); CONFIG_KFENCE_NUM_OBJECTS, (void *)__kfence_pool,
if (IS_ENABLED(CONFIG_DEBUG_KERNEL))
pr_cont(" at 0x%px-0x%px\n", (void *)__kfence_pool,
(void *)(__kfence_pool + KFENCE_POOL_SIZE)); (void *)(__kfence_pool + KFENCE_POOL_SIZE));
else
pr_cont("\n");
} }
void kfence_shutdown_cache(struct kmem_cache *s) void kfence_shutdown_cache(struct kmem_cache *s)
......
...@@ -16,13 +16,6 @@ ...@@ -16,13 +16,6 @@
#include "../slab.h" /* for struct kmem_cache */ #include "../slab.h" /* for struct kmem_cache */
/* For non-debug builds, avoid leaking kernel pointers into dmesg. */
#ifdef CONFIG_DEBUG_KERNEL
#define PTR_FMT "%px"
#else
#define PTR_FMT "%p"
#endif
/* /*
* Get the canary byte pattern for @addr. Use a pattern that varies based on the * Get the canary byte pattern for @addr. Use a pattern that varies based on the
* lower 3 bits of the address, to detect memory corruptions with higher * lower 3 bits of the address, to detect memory corruptions with higher
......
...@@ -146,7 +146,7 @@ static bool report_matches(const struct expect_report *r) ...@@ -146,7 +146,7 @@ static bool report_matches(const struct expect_report *r)
break; break;
} }
cur += scnprintf(cur, end - cur, " 0x" PTR_FMT, (void *)r->addr); cur += scnprintf(cur, end - cur, " 0x%p", (void *)r->addr);
spin_lock_irqsave(&observed.lock, flags); spin_lock_irqsave(&observed.lock, flags);
if (!report_available()) if (!report_available())
......
...@@ -19,6 +19,8 @@ ...@@ -19,6 +19,8 @@
#include "kfence.h" #include "kfence.h"
extern bool no_hash_pointers;
/* Helper function to either print to a seq_file or to console. */ /* Helper function to either print to a seq_file or to console. */
__printf(2, 3) __printf(2, 3)
static void seq_con_printf(struct seq_file *seq, const char *fmt, ...) static void seq_con_printf(struct seq_file *seq, const char *fmt, ...)
...@@ -118,7 +120,7 @@ void kfence_print_object(struct seq_file *seq, const struct kfence_metadata *met ...@@ -118,7 +120,7 @@ void kfence_print_object(struct seq_file *seq, const struct kfence_metadata *met
} }
seq_con_printf(seq, seq_con_printf(seq,
"kfence-#%zd [0x" PTR_FMT "-0x" PTR_FMT "kfence-#%zd [0x%p-0x%p"
", size=%d, cache=%s] allocated by task %d:\n", ", size=%d, cache=%s] allocated by task %d:\n",
meta - kfence_metadata, (void *)start, (void *)(start + size - 1), size, meta - kfence_metadata, (void *)start, (void *)(start + size - 1), size,
(cache && cache->name) ? cache->name : "<destroyed>", meta->alloc_track.pid); (cache && cache->name) ? cache->name : "<destroyed>", meta->alloc_track.pid);
...@@ -148,7 +150,7 @@ static void print_diff_canary(unsigned long address, size_t bytes_to_show, ...@@ -148,7 +150,7 @@ static void print_diff_canary(unsigned long address, size_t bytes_to_show,
for (cur = (const u8 *)address; cur < end; cur++) { for (cur = (const u8 *)address; cur < end; cur++) {
if (*cur == KFENCE_CANARY_PATTERN(cur)) if (*cur == KFENCE_CANARY_PATTERN(cur))
pr_cont(" ."); pr_cont(" .");
else if (IS_ENABLED(CONFIG_DEBUG_KERNEL)) else if (no_hash_pointers)
pr_cont(" 0x%02x", *cur); pr_cont(" 0x%02x", *cur);
else /* Do not leak kernel memory in non-debug builds. */ else /* Do not leak kernel memory in non-debug builds. */
pr_cont(" !"); pr_cont(" !");
...@@ -201,7 +203,7 @@ void kfence_report_error(unsigned long address, bool is_write, struct pt_regs *r ...@@ -201,7 +203,7 @@ void kfence_report_error(unsigned long address, bool is_write, struct pt_regs *r
pr_err("BUG: KFENCE: out-of-bounds %s in %pS\n\n", get_access_type(is_write), pr_err("BUG: KFENCE: out-of-bounds %s in %pS\n\n", get_access_type(is_write),
(void *)stack_entries[skipnr]); (void *)stack_entries[skipnr]);
pr_err("Out-of-bounds %s at 0x" PTR_FMT " (%luB %s of kfence-#%zd):\n", pr_err("Out-of-bounds %s at 0x%p (%luB %s of kfence-#%zd):\n",
get_access_type(is_write), (void *)address, get_access_type(is_write), (void *)address,
left_of_object ? meta->addr - address : address - meta->addr, left_of_object ? meta->addr - address : address - meta->addr,
left_of_object ? "left" : "right", object_index); left_of_object ? "left" : "right", object_index);
...@@ -210,24 +212,24 @@ void kfence_report_error(unsigned long address, bool is_write, struct pt_regs *r ...@@ -210,24 +212,24 @@ void kfence_report_error(unsigned long address, bool is_write, struct pt_regs *r
case KFENCE_ERROR_UAF: case KFENCE_ERROR_UAF:
pr_err("BUG: KFENCE: use-after-free %s in %pS\n\n", get_access_type(is_write), pr_err("BUG: KFENCE: use-after-free %s in %pS\n\n", get_access_type(is_write),
(void *)stack_entries[skipnr]); (void *)stack_entries[skipnr]);
pr_err("Use-after-free %s at 0x" PTR_FMT " (in kfence-#%zd):\n", pr_err("Use-after-free %s at 0x%p (in kfence-#%zd):\n",
get_access_type(is_write), (void *)address, object_index); get_access_type(is_write), (void *)address, object_index);
break; break;
case KFENCE_ERROR_CORRUPTION: case KFENCE_ERROR_CORRUPTION:
pr_err("BUG: KFENCE: memory corruption in %pS\n\n", (void *)stack_entries[skipnr]); pr_err("BUG: KFENCE: memory corruption in %pS\n\n", (void *)stack_entries[skipnr]);
pr_err("Corrupted memory at 0x" PTR_FMT " ", (void *)address); pr_err("Corrupted memory at 0x%p ", (void *)address);
print_diff_canary(address, 16, meta); print_diff_canary(address, 16, meta);
pr_cont(" (in kfence-#%zd):\n", object_index); pr_cont(" (in kfence-#%zd):\n", object_index);
break; break;
case KFENCE_ERROR_INVALID: case KFENCE_ERROR_INVALID:
pr_err("BUG: KFENCE: invalid %s in %pS\n\n", get_access_type(is_write), pr_err("BUG: KFENCE: invalid %s in %pS\n\n", get_access_type(is_write),
(void *)stack_entries[skipnr]); (void *)stack_entries[skipnr]);
pr_err("Invalid %s at 0x" PTR_FMT ":\n", get_access_type(is_write), pr_err("Invalid %s at 0x%p:\n", get_access_type(is_write),
(void *)address); (void *)address);
break; break;
case KFENCE_ERROR_INVALID_FREE: case KFENCE_ERROR_INVALID_FREE:
pr_err("BUG: KFENCE: invalid free in %pS\n\n", (void *)stack_entries[skipnr]); pr_err("BUG: KFENCE: invalid free in %pS\n\n", (void *)stack_entries[skipnr]);
pr_err("Invalid free of 0x" PTR_FMT " (in kfence-#%zd):\n", (void *)address, pr_err("Invalid free of 0x%p (in kfence-#%zd):\n", (void *)address,
object_index); object_index);
break; break;
} }
...@@ -242,7 +244,7 @@ void kfence_report_error(unsigned long address, bool is_write, struct pt_regs *r ...@@ -242,7 +244,7 @@ void kfence_report_error(unsigned long address, bool is_write, struct pt_regs *r
/* Print report footer. */ /* Print report footer. */
pr_err("\n"); pr_err("\n");
if (IS_ENABLED(CONFIG_DEBUG_KERNEL) && regs) if (no_hash_pointers && regs)
show_regs(regs); show_regs(regs);
else else
dump_stack_print_info(KERN_ERR); dump_stack_print_info(KERN_ERR);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment