From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9D7F5C433E0 for ; Tue, 9 Feb 2021 18:06:56 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 3836664EB4 for ; Tue, 9 Feb 2021 18:06:54 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3836664EB4 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 9D4326B0005; Tue, 9 Feb 2021 13:06:53 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9877A6B006C; Tue, 9 Feb 2021 13:06:53 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 89BC56B006E; Tue, 9 Feb 2021 13:06:53 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0143.hostedemail.com [216.40.44.143]) by kanga.kvack.org (Postfix) with ESMTP id 735306B0005 for ; Tue, 9 Feb 2021 13:06:53 -0500 (EST) Received: from smtpin02.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 392B4988F for ; Tue, 9 Feb 2021 18:06:53 +0000 (UTC) X-FDA: 77799510306.02.food04_400ccca27609 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin02.hostedemail.com (Postfix) with ESMTP id 088CA10097AA3 for ; Tue, 9 Feb 2021 18:06:53 +0000 (UTC) X-HE-Tag: food04_400ccca27609 X-Filterd-Recvd-Size: 6740 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf14.hostedemail.com (Postfix) with ESMTP for ; Tue, 9 Feb 2021 18:06:52 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 1FBFFAB71; Tue, 9 Feb 2021 18:06:51 +0000 (UTC) Subject: Re: [PATCH mm] kfence: make reporting sensitive information configurable To: Marco Elver , akpm@linux-foundation.org Cc: glider@google.com, dvyukov@google.com, andreyknvl@google.com, jannh@google.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, kasan-dev@googlegroups.com, Timur Tabi , Petr Mladek , Kees Cook , Steven Rostedt References: <20210209151329.3459690-1-elver@google.com> From: Vlastimil Babka Message-ID: <4f39ad95-a773-acc6-dd9e-cb04f897ca16@suse.cz> Date: Tue, 9 Feb 2021 19:06:50 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: <20210209151329.3459690-1-elver@google.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: 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 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-timur@kernel.org/ [2] https://lore.kernel.org/linux-mm/CAHk-=wgaK4cz=K-JB4p-KPXBV73m9bja2w1W1Lr3iu8+NEPk7A@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); >