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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id DD3A8C433EF for ; Sat, 5 Mar 2022 05:26:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 760198D0011; Sat, 5 Mar 2022 00:26:42 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 70F168D0001; Sat, 5 Mar 2022 00:26:42 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5FDF28D0011; Sat, 5 Mar 2022 00:26:42 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0083.hostedemail.com [216.40.44.83]) by kanga.kvack.org (Postfix) with ESMTP id 4EB708D0001 for ; Sat, 5 Mar 2022 00:26:42 -0500 (EST) Received: from smtpin25.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 0AB478249980 for ; Sat, 5 Mar 2022 05:26:42 +0000 (UTC) X-FDA: 79209197844.25.1321BDB Received: from out30-45.freemail.mail.aliyun.com (out30-45.freemail.mail.aliyun.com [115.124.30.45]) by imf09.hostedemail.com (Postfix) with ESMTP id 913EF140005 for ; Sat, 5 Mar 2022 05:26:40 +0000 (UTC) X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R191e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=e01e01424;MF=dtcccc@linux.alibaba.com;NM=1;PH=DS;RN=7;SR=0;TI=SMTPD_---0V6FOtNg_1646457995; Received: from 192.168.0.205(mailfrom:dtcccc@linux.alibaba.com fp:SMTPD_---0V6FOtNg_1646457995) by smtp.aliyun-inc.com(127.0.0.1); Sat, 05 Mar 2022 13:26:36 +0800 Message-ID: Date: Sat, 5 Mar 2022 13:26:35 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [RFC PATCH 1/2] kfence: Allow re-enabling KFENCE after system startup Content-Language: en-US To: Marco Elver Cc: Alexander Potapenko , Dmitry Vyukov , Andrew Morton , kasan-dev@googlegroups.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20220303031505.28495-1-dtcccc@linux.alibaba.com> <20220303031505.28495-2-dtcccc@linux.alibaba.com> From: Tianchen Ding In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 913EF140005 X-Stat-Signature: zb8gdonkg1hhwnu54i11uf49n7c7rtpx Authentication-Results: imf09.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=alibaba.com; spf=pass (imf09.hostedemail.com: domain of dtcccc@linux.alibaba.com designates 115.124.30.45 as permitted sender) smtp.mailfrom=dtcccc@linux.alibaba.com X-HE-Tag: 1646458000-552223 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 2022/3/5 02:13, Marco Elver wrote: > On Thu, 3 Mar 2022 at 04:15, Tianchen Ding wrote: >> >> If once KFENCE is disabled by: >> echo 0 > /sys/module/kfence/parameters/sample_interval >> KFENCE could never be re-enabled until next rebooting. >> >> Allow re-enabling it by writing a positive num to sample_interval. >> >> Signed-off-by: Tianchen Ding > > The only problem I see with this is if KFENCE was disabled because of > a KFENCE_WARN_ON(). See below. > >> --- >> mm/kfence/core.c | 16 ++++++++++++++-- >> 1 file changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/mm/kfence/core.c b/mm/kfence/core.c >> index 13128fa13062..19eb123c0bba 100644 >> --- a/mm/kfence/core.c >> +++ b/mm/kfence/core.c >> @@ -55,6 +55,7 @@ EXPORT_SYMBOL_GPL(kfence_sample_interval); /* Export for test modules. */ >> #endif >> #define MODULE_PARAM_PREFIX "kfence." >> >> +static int kfence_enable_late(void); >> static int param_set_sample_interval(const char *val, const struct kernel_param *kp) >> { >> unsigned long num; >> @@ -65,10 +66,11 @@ static int param_set_sample_interval(const char *val, const struct kernel_param >> >> if (!num) /* Using 0 to indicate KFENCE is disabled. */ >> WRITE_ONCE(kfence_enabled, false); >> - else if (!READ_ONCE(kfence_enabled) && system_state != SYSTEM_BOOTING) >> - return -EINVAL; /* Cannot (re-)enable KFENCE on-the-fly. */ >> >> *((unsigned long *)kp->arg) = num; >> + >> + if (num && !READ_ONCE(kfence_enabled) && system_state != SYSTEM_BOOTING) > > Should probably have an 'old_sample_interval = *((unsigned long > *)kp->arg)' somewhere before, and add a '&& !old_sample_interval', > because if old_sample_interval!=0 then KFENCE was disabled due to a > KFENCE_WARN_ON(). Also in this case, it should return -EINVAL. So you > want a flow like this: > > old_sample_interval = ...; > ... > if (num && !READ_ONCE(kfence_enabled) && system_state != SYSTEM_BOOTING) > return old_sample_interval ? -EINVAL : kfence_enable_late(); > ... > Because sample_interval will used by delayed_work, we must put setting sample_interval before enabling KFENCE. So the order would be: old_sample_interval = sample_interval; sample_interval = num; if (...) kfence_enable_late(); This may be bypassed after KFENCE_WARN_ON() happens, if we first write 0, and then write 100 to it. How about this one: if (ret < 0) return ret; + /* Cannot set sample_interval after KFENCE_WARN_ON(). */ + if (unlikely(*((unsigned long *)kp->arg) && !READ_ONCE(kfence_enabled))) + return -EINVAL; + if (!num) /* Using 0 to indicate KFENCE is disabled. */ WRITE_ONCE(kfence_enabled, false); > Thanks, > -- Marco