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 06D65C43334 for ; Tue, 28 Jun 2022 07:11:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 170C28E0002; Tue, 28 Jun 2022 03:11:39 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 121D48E0001; Tue, 28 Jun 2022 03:11:39 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F04FD8E0002; Tue, 28 Jun 2022 03:11:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id DF1618E0001 for ; Tue, 28 Jun 2022 03:11:38 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay13.hostedemail.com (Postfix) with ESMTP id B71F161040 for ; Tue, 28 Jun 2022 07:11:38 +0000 (UTC) X-FDA: 79626774276.26.562B681 Received: from mailgw01.mediatek.com (mailgw01.mediatek.com [216.200.240.184]) by imf11.hostedemail.com (Postfix) with ESMTP id E156E40038 for ; Tue, 28 Jun 2022 07:11:35 +0000 (UTC) X-UUID: c6b3026726b04a4587a2f041b59f0213-20220628 X-CID-P-RULE: Release_Ham X-CID-O-INFO: VERSION:1.1.7,REQID:da3885e9-caf0-44d0-a510-407ae4ff029b,OB:0,LO B:0,IP:0,URL:0,TC:0,Content:-5,EDM:0,RT:0,SF:0,FILE:0,RULE:Release_Ham,ACT ION:release,TS:-5 X-CID-META: VersionHash:87442a2,CLOUDID:c2c8f785-57f0-47ca-ba27-fe8c57fbf305,C OID:IGNORED,Recheck:0,SF:nil,TC:0,Content:0,EDM:-3,IP:nil,URL:1,File:nil,Q S:nil,BEC:nil,COL:0 X-UUID: c6b3026726b04a4587a2f041b59f0213-20220628 Received: from mtkmbs11n1.mediatek.inc [(172.21.101.185)] by mailgw01.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLSv1.2 ECDHE-RSA-AES256-GCM-SHA384 256/256) with ESMTP id 885114633; Tue, 28 Jun 2022 00:11:28 -0700 Received: from mtkcas11.mediatek.inc (172.21.101.40) by mtkmbs11n1.mediatek.inc (172.21.101.185) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.2.792.3; Tue, 28 Jun 2022 14:40:50 +0800 Received: from mtksdccf07 (172.21.84.99) by mtkcas11.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Tue, 28 Jun 2022 14:40:49 +0800 Message-ID: <9c6fcb1c178a923f2406466a3f9f2345e4e7a1c1.camel@mediatek.com> Subject: Re: [PATCH 1/1] mm: kfence: skip kmemleak alloc in kfence_pool From: Yee Lee To: Marco Elver CC: , Alexander Potapenko , Dmitry Vyukov , Andrew Morton , Matthias Brugger , "open list:KFENCE" , "open list:MEMORY MANAGEMENT" , "moderated list:ARM/Mediatek SoC support" , "moderated list:ARM/Mediatek SoC support" , Date: Tue, 28 Jun 2022 14:40:49 +0800 In-Reply-To: References: <20220623111937.6491-1-yee.lee@mediatek.com> <20220623111937.6491-2-yee.lee@mediatek.com> Content-Type: multipart/alternative; boundary="=-tlhIpc4LLWzE61Oh4jJE" X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.2 MIME-Version: 1.0 X-MTK: N ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1656400298; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=YzqQIMEhrsIhrLBGFKZKzc02lgPdNGB9Mb+A2CTuIGI=; b=czuM2xrtkTdmOSp5ADFha7TD+HV+SoP+96A3l3/mPmt69lFq3QLxYn8ZwCzVZIbHrB4N2p 0nXV3Iy/gc6kBf4Z20XUstQEIM5EtGRXvaCSe4BR3SfPBpHsfxr0abq+WL7DK/GoPllGAz nbJA/Ygtvax4kzdG6s2ocwUbThrbYxY= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=mediatek.com; spf=pass (imf11.hostedemail.com: domain of yee.lee@mediatek.com designates 216.200.240.184 as permitted sender) smtp.mailfrom=yee.lee@mediatek.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1656400298; a=rsa-sha256; cv=none; b=H1Ywbza/h8HM6GmMxMp9fgMWM4FTx4wwZrz6va+va/OcMf5kZsXFQaSEqrKiKDJmQmHhEc y6H+cMb/5D3VmE5doS2VzPVClXfWbwaQHDfxPXOftWLbjxHBiY80GHz8Q3EOKtVHX0ObpT 5ND/yjTbpVBBsAPAe7znauByW2g+wTc= X-Rspamd-Queue-Id: E156E40038 Authentication-Results: imf11.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=mediatek.com; spf=pass (imf11.hostedemail.com: domain of yee.lee@mediatek.com designates 216.200.240.184 as permitted sender) smtp.mailfrom=yee.lee@mediatek.com X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: 1kzy5wjnwrnwbe1cprjrygcmdoc5qfqt X-HE-Tag: 1656400295-138377 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: --=-tlhIpc4LLWzE61Oh4jJE Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit On Fri, 2022-06-24 at 10:28 +0200, Marco Elver wrote: > On Fri, 24 Jun 2022 at 10:20, 'Yee Lee' via kasan-dev > wrote: > > > > On Thu, 2022-06-23 at 13:59 +0200, Marco Elver wrote: > > > On Thu, 23 Jun 2022 at 13:20, yee.lee via kasan-dev > > > wrote: > > > > > > > > From: Yee Lee > > > > > > > > Use MEMBLOCK_ALLOC_NOLEAKTRACE to skip kmemleak registration > > > > when > > > > the kfence pool is allocated from memblock. And the > > > > kmemleak_free > > > > later can be removed too. > > > > > > Is this purely meant to be a cleanup and non-functional change? > > > > > > > Signed-off-by: Yee Lee > > > > > > > > --- > > > > mm/kfence/core.c | 18 ++++++++---------- > > > > 1 file changed, 8 insertions(+), 10 deletions(-) > > > > > > > > diff --git a/mm/kfence/core.c b/mm/kfence/core.c > > > > index 4e7cd4c8e687..0d33d83f5244 100644 > > > > --- a/mm/kfence/core.c > > > > +++ b/mm/kfence/core.c > > > > @@ -600,14 +600,6 @@ static unsigned long > > > > kfence_init_pool(void) > > > > addr += 2 * PAGE_SIZE; > > > > } > > > > > > > > - /* > > > > - * The pool is live and will never be deallocated from > > > > this > > > > point on. > > > > - * Remove the pool object from the kmemleak object > > > > tree, as > > > > it would > > > > - * otherwise overlap with allocations returned by > > > > kfence_alloc(), which > > > > - * are registered with kmemleak through the slab post- > > > > alloc > > > > hook. > > > > - */ > > > > - kmemleak_free(__kfence_pool); > > > > > > This appears to only be a non-functional change if the pool is > > > allocated early. If the pool is allocated late using page-alloc, > > > then > > > there'll not be a kmemleak_free() on that memory and we'll have > > > the > > > same problem. > > > > Do you mean the kzalloc(slab_is_available) in memblock_allc()? That > > implies that MEMBLOCK_ALLOC_NOLEAKTRACE has no guarantee skipping > > kmemleak_alloc from this. (Maybe add it?) > > No, if KFENCE is initialized through kfence_init_late() -> > kfence_init_pool_late() -> kfence_init_pool(). Thanks for the information. But as I known, page-alloc does not request kmemleak areas. So the current kfence_pool_init_late() would cause another kmemleak warning on unknown freeing. Reproducing test: (kfence late enable + kmemleak debug on) / # echo 500 > /sys/module/kfence/parameters/sample_interval [ 153.433518] kmemleak: Freeing unknown object at 0xffff0000c0600000 [ 153.433804] CPU: 0 PID: 100 Comm: sh Not tainted 5.19.0-rc3-74069- gde5c208d533a-dirty #1 [ 153.434027] Hardware name: linux,dummy-virt (DT) [ 153.434265] Call trace: [ 153.434331] dump_backtrace+0xdc/0xfc [ 153.434962] show_stack+0x18/0x24 [ 153.435106] dump_stack_lvl+0x64/0x7c [ 153.435232] dump_stack+0x18/0x38 [ 153.435347] kmemleak_free+0x184/0x1c8 [ 153.435462] kfence_init_pool+0x16c/0x194 [ 153.435587] param_set_sample_interval+0xe0/0x1c4 [ 153.435694] param_attr_store+0x98/0xf4 [ 153.435804] module_attr_store+0x24/0x3c [ 153.435910] sysfs_kf_write+0x3c/0x50 ...(skip) [ 153.444496] kfence: initialized - using 524288 bytes for 63 objects at 0x00000000a3236b01-0x00000000901655d3 / # Hence, now there are two issues to solve. (1) (The original)To prevent the undesired kmemleak scanning on the kfence pool. As Cataline's suggestion, we can just apply kmemleak_ignore_phys instead of free it at all. ref: https://lore.kernel.org/linux-mm/YrWPg3xIHbm9bFxP@arm.com/ (2) The late-allocated kfence pool doesn't need to go through kmemleak_free. We can relocate the opeartion to kfence_init_pool_early() to seperate them. That is, kfence_init_pool_early(memblock) has it and kfence_init_pool_late(page alloc) does not. The draft is like the following. diff --git a/mm/kfence/core.c b/mm/kfence/core.c index 11a954763be9..a52db7f06c04 100644 --- a/mm/kfence/core.c +++ b/mm/kfence/core.c @@ -591,14 +591,6 @@ static unsigned long kfence_init_pool(void) addr += 2 * PAGE_SIZE; } - /* - * The pool is live and will never be deallocated from this point on. - * Remove the pool object from the kmemleak object tree, as it would - * otherwise overlap with allocations returned by kfence_alloc(), which - * are registered with kmemleak through the slab post-alloc hook. - */ - kmemleak_free(__kfence_pool); - return 0; } @@ -611,8 +603,16 @@ static bool __init kfence_init_pool_early(void) addr = kfence_init_pool(); - if (!addr) + if (!addr) { + /* + * The pool is live and will never be deallocated from this point on. + * Ignore the pool object from the kmemleak phys object tree, as it would + * otherwise overlap with allocations returned by kfence_alloc(), which + * are registered with kmemleak through the slab post- alloc hook. + */ + kmemleak_ignore_phys(__pa(__kfence_pool)); return true; + } /* * Only release unprotected pages, and do not try to go back and change --=-tlhIpc4LLWzE61Oh4jJE Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable
On Fri, 2022-06-24 at 10:28 +0200, Marco Elver wrote:
On Fri, 24 Jun 2022 at 10:20, 'Yee Lee' via kasan-= dev

On Thu, 2022-06-23 at 13:59 +0200, Marco Elver wrote:
On Thu, 23 Jun 2022 at 13:20, yee.lee via kasan-dev=

=
From: Yee Lee <yee.lee@medi= atek.com>

Use MEMBLOCK_ALLOC_NOLEAKTRACE to= skip kmemleak registration when
the kfence pool is allocated fro= m memblock. And the kmemleak_free
later can be removed too.
=

Is this purely meant to be a cleanup and n= on-functional change?

Signed-off-by: Yee Lee <yee.lee= @mediatek.com>

---
 mm/kfenc= e/core.c | 18 ++++++++----------
 1 file changed, 8 insertio= ns(+), 10 deletions(-)

diff --git a/mm/kfence/core= .c b/mm/kfence/core.c
index 4e7cd4c8e687..0d33d83f5244 100644
--- a/mm/kfence/core.c
+++ b/mm/kfence/core.c
@@= -600,14 +600,6 @@ static unsigned long kfence_init_pool(void)
&n= bsp;            = ;   addr +=3D 2 * PAGE_SIZE;
   &nb= sp;    }

-   &n= bsp;   /*
-      &nb= sp; * The pool is live and will never be deallocated from this
point on.
-        * R= emove the pool object from the kmemleak object tree, as
it would<= /div>
-        * otherwise over= lap with allocations returned by
kfence_alloc(), which
= -        * are registered with kmem= leak through the slab post-alloc
hook.
-  &nb= sp;     */
-    &nbs= p;  kmemleak_free(__kfence_pool);

This appears to only be a non-functional change if the pool is
=
allocated early. If the pool is allocated late using page-alloc, then<= /div>
there'll not be a kmemleak_free() on that memory and we'll have t= he
same problem.

Do you mea= n the kzalloc(slab_is_available) in memblock_allc()? That
implies= that MEMBLOCK_ALLOC_NOLEAKTRACE has no guarantee skipping
kmemle= ak_alloc from this. (Maybe add it?)

N= o, if KFENCE is initialized through kfence_init_late() ->
kfen= ce_init_pool_late() -> kfence_init_pool().
Thanks= for the information.

But as I known, page-alloc d= oes not request kmemleak areas.
So the current kfence_pool_init_l= ate() would cause another kmemleak warning on unknown freeing. 
<= div>
Reproducing test: (kfence late enable + kmemleak debug o= n)

/ # echo 500 > /sys/module/kfence/parameters= /sample_interval
[  153.433518] kmemleak: Freeing unkno= wn object at 0xffff0000c0600000
[  153.433804] CPU: 0 P= ID: 100 Comm: sh Not tainted 5.19.0-rc3-74069-gde5c208d533a-dirty #1
<= div>[  153.434027] Hardware name: linux,dummy-virt (DT)
[  153.434265] Call trace:
[  153.434331]&nb= sp; dump_backtrace+0xdc/0xfc
[  153.434962] &= nbsp;show_stack+0x18/0x24
[  153.435106]  dum= p_stack_lvl+0x64/0x7c
[  153.435232]  dump_st= ack+0x18/0x38
[  153.435347]  kmemleak_free+0= x184/0x1c8
[  153.435462]  kfence_init_pool+0= x16c/0x194
[  153.435587]  param_set_sample_i= nterval+0xe0/0x1c4
[  153.435694]  param_attr= _store+0x98/0xf4
[  153.435804]  module_attr_= store+0x24/0x3c
[  153.435910]  sysfs_kf_writ= e+0x3c/0x50
...(skip)
[  153.444496] kfence: = initialized - using 524288 bytes for 63 objects at 0x00000000a3236b01-0x000= 00000901655d3
/ # 

Hence, now there= are two issues to solve.
(1) (The original)To prevent the undesi= red kmemleak scanning on the kfence pool. As Cataline's suggestion, we can = just apply kmemleak_ignore_phys instead of free it at all. 
<= br>
(2) The late-allocated kfence pool doesn't need to go through= kmemleak_free. We can relocate the opeartion to kfence_init_pool_early() t= o seperate them. 
That is, kfence_init_pool_early(memblock) = has it and kfence_init_pool_late(page alloc) does not. 

=
The draft is like the following.

diff -= -git a/mm/kfence/core.c b/mm/kfence/core.c
index 11a954763be9..a5= 2db7f06c04 100644
--- a/mm/kfence/core.c
+++ b/mm/kfenc= e/core.c
@@ -591,14 +591,6 @@ static unsigned long kfence_init_po= ol(void)
         &n= bsp;      addr +=3D 2 * PAGE_SIZE;
=         }

= -       /*
-   =      * The pool is live and will never be dealloca= ted from this point on.
-      &nbs= p; * Remove the pool object from the kmemleak object tree, as it would=
-        * otherwise ove= rlap with allocations returned by kfence_alloc(), which
- &n= bsp;      * are registered with kmemleak thro= ugh the slab post-alloc hook.
-     &nbs= p;  */
-       kmeml= eak_free(__kfence_pool);
-
    &nbs= p;   return 0;
 }

@@= -611,8 +603,16 @@ static bool __init kfence_init_pool_early(void)

        addr =3D k= fence_init_pool();

-     =   if (!addr)
+       = ;if (!addr) {
+        &n= bsp;      /*
+   &nb= sp;            = * The pool is live and will never be deallocated from this point on.
<= div>+           &nbs= p;    * Ignore the pool object from the kmemleak phys o= bject tree, as it would
+      &nbs= p;         * otherwise overlap= with allocations returned by kfence_alloc(), which
+  =             &nb= sp; * are registered with kmemleak through the slab post-alloc hook.
+           = ;     */
+     =           kmemleak_ignore= _phys(__pa(__kfence_pool));
      &= nbsp;         return true;
+       }

        /*
  =        * Only release unprotected pages,= and do not try to go back and change
--=-tlhIpc4LLWzE61Oh4jJE--