On 2/9/21 4:13 PM, Marco Elver wrote: > 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. > > Therefore, add the option CONFIG_KFENCE_REPORT_SENSITIVE to decide if we > should add potentially sensitive information to KFENCE reports. The > default behaviour remains unchanged. > > Signed-off-by: Marco Elver <el...@google.com>
Hi, could we drop this kconfig approach in favour of the boot option proposed here? [1] Just do the prints with %p unconditionally and the boot option takes care of it? Also Linus mentioned dislike of controlling potential memory leak to be a config option [2] Thanks, Vlastimil [1] https://lore.kernel.org/linux-mm/20210202213633.755469-1-ti...@kernel.org/ [2] https://lore.kernel.org/linux-mm/CAHk-=wgaK4cz=k-jb4p-kpxbv73m9bja2w1w1lr3iu8+nep...@mail.gmail.com/ > --- > Documentation/dev-tools/kfence.rst | 6 +++--- > lib/Kconfig.kfence | 8 ++++++++ > mm/kfence/core.c | 2 +- > mm/kfence/kfence.h | 3 +-- > mm/kfence/report.c | 6 +++--- > 5 files changed, 16 insertions(+), 9 deletions(-) > > diff --git a/Documentation/dev-tools/kfence.rst > b/Documentation/dev-tools/kfence.rst > index 58a0a5fa1ddc..5280d644f826 100644 > --- a/Documentation/dev-tools/kfence.rst > +++ b/Documentation/dev-tools/kfence.rst > @@ -89,7 +89,7 @@ A typical out-of-bounds access looks like this:: > 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 > its origin. Note that, real kernel addresses are only shown for > -``CONFIG_DEBUG_KERNEL=y`` builds. > +``CONFIG_KFENCE_REPORT_SENSITIVE=y`` builds. > > Use-after-free accesses are reported as:: > > @@ -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 > 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 > -only shown for ``CONFIG_DEBUG_KERNEL=y`` builds; to avoid information > -disclosure for non-debug builds, '!' is used instead to denote invalidly > +only shown for ``CONFIG_KFENCE_REPORT_SENSITIVE=y`` builds; to avoid > +information disclosure otherwise, '!' is used instead to denote invalidly > written bytes. > > And finally, KFENCE may also report on invalid accesses to any protected page > diff --git a/lib/Kconfig.kfence b/lib/Kconfig.kfence > index 78f50ccb3b45..141494a5f530 100644 > --- a/lib/Kconfig.kfence > +++ b/lib/Kconfig.kfence > @@ -55,6 +55,14 @@ config KFENCE_NUM_OBJECTS > pages are required; with one containing the object and two adjacent > ones used as guard pages. > > +config KFENCE_REPORT_SENSITIVE > + bool "Show potentially sensitive information in reports" > + default y if DEBUG_KERNEL > + help > + Show potentially sensitive information such as unhashed pointers, > + context bytes on memory corruptions, as well as dump registers in > + KFENCE reports. > + > config KFENCE_STRESS_TEST_FAULTS > int "Stress testing of fault handling and error reporting" if EXPERT > default 0 > diff --git a/mm/kfence/core.c b/mm/kfence/core.c > index cfe3d32ac5b7..5f7e02db5f53 100644 > --- a/mm/kfence/core.c > +++ b/mm/kfence/core.c > @@ -648,7 +648,7 @@ void __init kfence_init(void) > schedule_delayed_work(&kfence_timer, 0); > pr_info("initialized - using %lu bytes for %d objects", > KFENCE_POOL_SIZE, > CONFIG_KFENCE_NUM_OBJECTS); > - if (IS_ENABLED(CONFIG_DEBUG_KERNEL)) > + if (IS_ENABLED(CONFIG_KFENCE_REPORT_SENSITIVE)) > pr_cont(" at 0x%px-0x%px\n", (void *)__kfence_pool, > (void *)(__kfence_pool + KFENCE_POOL_SIZE)); > else > diff --git a/mm/kfence/kfence.h b/mm/kfence/kfence.h > index 1accc840dbbe..48a8196b947b 100644 > --- a/mm/kfence/kfence.h > +++ b/mm/kfence/kfence.h > @@ -16,8 +16,7 @@ > > #include "../slab.h" /* for struct kmem_cache */ > > -/* For non-debug builds, avoid leaking kernel pointers into dmesg. */ > -#ifdef CONFIG_DEBUG_KERNEL > +#ifdef CONFIG_KFENCE_REPORT_SENSITIVE > #define PTR_FMT "%px" > #else > #define PTR_FMT "%p" > diff --git a/mm/kfence/report.c b/mm/kfence/report.c > index 901bd7ee83d8..5e2dbabbab1d 100644 > --- a/mm/kfence/report.c > +++ b/mm/kfence/report.c > @@ -148,9 +148,9 @@ static void print_diff_canary(unsigned long address, > size_t bytes_to_show, > for (cur = (const u8 *)address; cur < end; cur++) { > if (*cur == KFENCE_CANARY_PATTERN(cur)) > pr_cont(" ."); > - else if (IS_ENABLED(CONFIG_DEBUG_KERNEL)) > + else if (IS_ENABLED(CONFIG_KFENCE_REPORT_SENSITIVE)) > pr_cont(" 0x%02x", *cur); > - else /* Do not leak kernel memory in non-debug builds. */ > + else /* Do not leak kernel memory. */ > pr_cont(" !"); > } > pr_cont(" ]"); > @@ -242,7 +242,7 @@ void kfence_report_error(unsigned long address, bool > is_write, struct pt_regs *r > > /* Print report footer. */ > pr_err("\n"); > - if (IS_ENABLED(CONFIG_DEBUG_KERNEL) && regs) > + if (IS_ENABLED(CONFIG_KFENCE_REPORT_SENSITIVE) && regs) > show_regs(regs); > else > dump_stack_print_info(KERN_ERR); >
