[PATCH] mm: kmemleak: Skip check in kmemleak_*_phys when pfn bound is not ready

[PATCH] mm: kmemleak: Skip check in kmemleak_*_phys when pfn bound is not ready

[yee.lee]

From: Yee Lee <yee.lee@mediatek.com>

In some archs (arm64), memblock allocates memory in boot time when
the pfn boundary (max_pfn/min_pfn) is not ready. The lowmen checks in
kmemleak_*_phys() drop those blocks and cause some false leak alarms
on common kernel objects.

Kmemleak output: (Qemu/arm64)
unreferenced object 0xffff0000c0170a00 (size 128):
  comm "swapper/0", pid 1, jiffies 4294892404 (age 126.208s)
  hex dump (first 32 bytes):
    62 61 73 65 00 00 00 00 00 00 00 00 00 00 00 00  base............
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<(____ptrval____)>] __kmalloc_track_caller+0x1b0/0x2e4
    [<(____ptrval____)>] kstrdup_const+0x8c/0xc4
    [<(____ptrval____)>] kvasprintf_const+0xbc/0xec
    [<(____ptrval____)>] kobject_set_name_vargs+0x58/0xe4
    [<(____ptrval____)>] kobject_add+0x84/0x100
    [<(____ptrval____)>] __of_attach_node_sysfs+0x78/0xec
    [<(____ptrval____)>] of_core_init+0x68/0x104
    [<(____ptrval____)>] driver_init+0x28/0x48
    [<(____ptrval____)>] do_basic_setup+0x14/0x28
    [<(____ptrval____)>] kernel_init_freeable+0x110/0x178
    [<(____ptrval____)>] kernel_init+0x20/0x1a0
    [<(____ptrval____)>] ret_from_fork+0x10/0x20

This patch relaxs the boundary checking in kmemleak_*_phys api
if max_low_pfn is uninitialzed.

Fixes: 23c2d4 (mm: kmemleak: take a full lowmem check in kmemleak_*_phy)
Signed-off-by: Yee Lee <yee.lee@mediatek.com>
---
 mm/kmemleak.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index a182f5ddaf68..6b2af544aa0f 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -1132,7 +1132,7 @@ EXPORT_SYMBOL(kmemleak_no_scan);
 void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, int min_count,
 			       gfp_t gfp)
 {
-	if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
+	if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn))
 		kmemleak_alloc(__va(phys), size, min_count, gfp);
 }
 EXPORT_SYMBOL(kmemleak_alloc_phys);
@@ -1146,7 +1146,7 @@ EXPORT_SYMBOL(kmemleak_alloc_phys);
  */
 void __ref kmemleak_free_part_phys(phys_addr_t phys, size_t size)
 {
-	if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
+	if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn))
 		kmemleak_free_part(__va(phys), size);
 }
 EXPORT_SYMBOL(kmemleak_free_part_phys);
@@ -1158,7 +1158,7 @@ EXPORT_SYMBOL(kmemleak_free_part_phys);
  */
 void __ref kmemleak_not_leak_phys(phys_addr_t phys)
 {
-	if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
+	if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn))
 		kmemleak_not_leak(__va(phys));
 }
 EXPORT_SYMBOL(kmemleak_not_leak_phys);
@@ -1170,7 +1170,7 @@ EXPORT_SYMBOL(kmemleak_not_leak_phys);
  */
 void __ref kmemleak_ignore_phys(phys_addr_t phys)
 {
-	if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
+	if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn))
 		kmemleak_ignore(__va(phys));
 }
 EXPORT_SYMBOL(kmemleak_ignore_phys);
-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mm: kmemleak: Skip check in kmemleak_*_phys when pfn bound is not ready

[patrick wang]

On Fri, May 27, 2022 at 11:25 AM <yee.lee@mediatek.com> wrote:
>
> From: Yee Lee <yee.lee@mediatek.com>
>
> In some archs (arm64), memblock allocates memory in boot time when
> the pfn boundary (max_pfn/min_pfn) is not ready. The lowmen checks in
> kmemleak_*_phys() drop those blocks and cause some false leak alarms
> on common kernel objects.

If I understand correctly, if those blocks are dropped, they will
not be added as kmemleak objects. So how could this affect
other objects?

Thanks,
Patrick

>
> Kmemleak output: (Qemu/arm64)
> unreferenced object 0xffff0000c0170a00 (size 128):
>   comm "swapper/0", pid 1, jiffies 4294892404 (age 126.208s)
>   hex dump (first 32 bytes):
>     62 61 73 65 00 00 00 00 00 00 00 00 00 00 00 00  base............
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<(____ptrval____)>] __kmalloc_track_caller+0x1b0/0x2e4
>     [<(____ptrval____)>] kstrdup_const+0x8c/0xc4
>     [<(____ptrval____)>] kvasprintf_const+0xbc/0xec
>     [<(____ptrval____)>] kobject_set_name_vargs+0x58/0xe4
>     [<(____ptrval____)>] kobject_add+0x84/0x100
>     [<(____ptrval____)>] __of_attach_node_sysfs+0x78/0xec
>     [<(____ptrval____)>] of_core_init+0x68/0x104
>     [<(____ptrval____)>] driver_init+0x28/0x48
>     [<(____ptrval____)>] do_basic_setup+0x14/0x28
>     [<(____ptrval____)>] kernel_init_freeable+0x110/0x178
>     [<(____ptrval____)>] kernel_init+0x20/0x1a0
>     [<(____ptrval____)>] ret_from_fork+0x10/0x20
>
> This patch relaxs the boundary checking in kmemleak_*_phys api
> if max_low_pfn is uninitialzed.
>
> Fixes: 23c2d4 (mm: kmemleak: take a full lowmem check in kmemleak_*_phy)
> Signed-off-by: Yee Lee <yee.lee@mediatek.com>
> ---
>  mm/kmemleak.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index a182f5ddaf68..6b2af544aa0f 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -1132,7 +1132,7 @@ EXPORT_SYMBOL(kmemleak_no_scan);
>  void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, int min_count,
>                                gfp_t gfp)
>  {
> -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
> +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn))
>                 kmemleak_alloc(__va(phys), size, min_count, gfp);
>  }
>  EXPORT_SYMBOL(kmemleak_alloc_phys);
> @@ -1146,7 +1146,7 @@ EXPORT_SYMBOL(kmemleak_alloc_phys);
>   */
>  void __ref kmemleak_free_part_phys(phys_addr_t phys, size_t size)
>  {
> -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
> +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn))
>                 kmemleak_free_part(__va(phys), size);
>  }
>  EXPORT_SYMBOL(kmemleak_free_part_phys);
> @@ -1158,7 +1158,7 @@ EXPORT_SYMBOL(kmemleak_free_part_phys);
>   */
>  void __ref kmemleak_not_leak_phys(phys_addr_t phys)
>  {
> -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
> +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn))
>                 kmemleak_not_leak(__va(phys));
>  }
>  EXPORT_SYMBOL(kmemleak_not_leak_phys);
> @@ -1170,7 +1170,7 @@ EXPORT_SYMBOL(kmemleak_not_leak_phys);
>   */
>  void __ref kmemleak_ignore_phys(phys_addr_t phys)
>  {
> -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
> +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn))
>                 kmemleak_ignore(__va(phys));
>  }
>  EXPORT_SYMBOL(kmemleak_ignore_phys);
> --
> 2.18.0
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mm: kmemleak: Skip check in kmemleak_*_phys when pfn bound is not ready

[Yee Lee]

On Fri, 2022-05-27 at 18:20 +0800, patrick wang wrote:
> On Fri, May 27, 2022 at 11:25 AM <yee.lee@mediatek.com> wrote:
> > 
> > From: Yee Lee <yee.lee@mediatek.com>
> > 
> > In some archs (arm64), memblock allocates memory in boot time when
> > the pfn boundary (max_pfn/min_pfn) is not ready. The lowmen checks
> > in
> > kmemleak_*_phys() drop those blocks and cause some false leak
> > alarms
> > on common kernel objects.
> 
> If I understand correctly, if those blocks are dropped, they will
> not be added as kmemleak objects. So how could this affect
> other objects?
> 
> Thanks,
> Patrick

Yes, in fact, memblock needs those areas never reported as leaks and
add them as grey blocks. More detailed comments can be found inside
memblock_alloc_range_nid. 

> 
> > 
> > Kmemleak output: (Qemu/arm64)
> > unreferenced object 0xffff0000c0170a00 (size 128):
> >   comm "swapper/0", pid 1, jiffies 4294892404 (age 126.208s)
> >   hex dump (first 32 bytes):
> >     62 61 73 65 00 00 00 00 00 00 00 00 00 00 00
> > 00  base............
> >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 00  ................
> >   backtrace:
> >     [<(____ptrval____)>] __kmalloc_track_caller+0x1b0/0x2e4
> >     [<(____ptrval____)>] kstrdup_const+0x8c/0xc4
> >     [<(____ptrval____)>] kvasprintf_const+0xbc/0xec
> >     [<(____ptrval____)>] kobject_set_name_vargs+0x58/0xe4
> >     [<(____ptrval____)>] kobject_add+0x84/0x100
> >     [<(____ptrval____)>] __of_attach_node_sysfs+0x78/0xec
> >     [<(____ptrval____)>] of_core_init+0x68/0x104
> >     [<(____ptrval____)>] driver_init+0x28/0x48
> >     [<(____ptrval____)>] do_basic_setup+0x14/0x28
> >     [<(____ptrval____)>] kernel_init_freeable+0x110/0x178
> >     [<(____ptrval____)>] kernel_init+0x20/0x1a0
> >     [<(____ptrval____)>] ret_from_fork+0x10/0x20
> > 
> > This patch relaxs the boundary checking in kmemleak_*_phys api
> > if max_low_pfn is uninitialzed.
> > 
> > Fixes: 23c2d4 (mm: kmemleak: take a full lowmem check in
> > kmemleak_*_phy)
> > Signed-off-by: Yee Lee <yee.lee@mediatek.com>
> > ---
> >  mm/kmemleak.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> > index a182f5ddaf68..6b2af544aa0f 100644
> > --- a/mm/kmemleak.c
> > +++ b/mm/kmemleak.c
> > @@ -1132,7 +1132,7 @@ EXPORT_SYMBOL(kmemleak_no_scan);
> >  void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, int
> > min_count,
> >                                gfp_t gfp)
> >  {
> > -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) <
> > max_low_pfn)
> > +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn &&
> > PHYS_PFN(phys) < max_low_pfn))
> >                 kmemleak_alloc(__va(phys), size, min_count, gfp);
> >  }
> >  EXPORT_SYMBOL(kmemleak_alloc_phys);
> > @@ -1146,7 +1146,7 @@ EXPORT_SYMBOL(kmemleak_alloc_phys);
> >   */
> >  void __ref kmemleak_free_part_phys(phys_addr_t phys, size_t size)
> >  {
> > -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) <
> > max_low_pfn)
> > +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn &&
> > PHYS_PFN(phys) < max_low_pfn))
> >                 kmemleak_free_part(__va(phys), size);
> >  }
> >  EXPORT_SYMBOL(kmemleak_free_part_phys);
> > @@ -1158,7 +1158,7 @@ EXPORT_SYMBOL(kmemleak_free_part_phys);
> >   */
> >  void __ref kmemleak_not_leak_phys(phys_addr_t phys)
> >  {
> > -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) <
> > max_low_pfn)
> > +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn &&
> > PHYS_PFN(phys) < max_low_pfn))
> >                 kmemleak_not_leak(__va(phys));
> >  }
> >  EXPORT_SYMBOL(kmemleak_not_leak_phys);
> > @@ -1170,7 +1170,7 @@ EXPORT_SYMBOL(kmemleak_not_leak_phys);
> >   */
> >  void __ref kmemleak_ignore_phys(phys_addr_t phys)
> >  {
> > -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) <
> > max_low_pfn)
> > +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn &&
> > PHYS_PFN(phys) < max_low_pfn))
> >                 kmemleak_ignore(__va(phys));
> >  }
> >  EXPORT_SYMBOL(kmemleak_ignore_phys);
> > --
> > 2.18.0
> > 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mm: kmemleak: Skip check in kmemleak_*_phys when pfn bound is not ready

[patrick wang]

On Fri, May 27, 2022 at 11:25 AM <yee.lee@mediatek.com> wrote:
>
> From: Yee Lee <yee.lee@mediatek.com>
>
> In some archs (arm64), memblock allocates memory in boot time when
> the pfn boundary (max_pfn/min_pfn) is not ready. The lowmen checks in
> kmemleak_*_phys() drop those blocks and cause some false leak alarms
> on common kernel objects.
>
> Kmemleak output: (Qemu/arm64)
> unreferenced object 0xffff0000c0170a00 (size 128):
>   comm "swapper/0", pid 1, jiffies 4294892404 (age 126.208s)
>   hex dump (first 32 bytes):
>     62 61 73 65 00 00 00 00 00 00 00 00 00 00 00 00  base............
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<(____ptrval____)>] __kmalloc_track_caller+0x1b0/0x2e4
>     [<(____ptrval____)>] kstrdup_const+0x8c/0xc4
>     [<(____ptrval____)>] kvasprintf_const+0xbc/0xec
>     [<(____ptrval____)>] kobject_set_name_vargs+0x58/0xe4
>     [<(____ptrval____)>] kobject_add+0x84/0x100
>     [<(____ptrval____)>] __of_attach_node_sysfs+0x78/0xec
>     [<(____ptrval____)>] of_core_init+0x68/0x104
>     [<(____ptrval____)>] driver_init+0x28/0x48
>     [<(____ptrval____)>] do_basic_setup+0x14/0x28
>     [<(____ptrval____)>] kernel_init_freeable+0x110/0x178
>     [<(____ptrval____)>] kernel_init+0x20/0x1a0
>     [<(____ptrval____)>] ret_from_fork+0x10/0x20
>
> This patch relaxs the boundary checking in kmemleak_*_phys api
> if max_low_pfn is uninitialzed.
>
> Fixes: 23c2d4 (mm: kmemleak: take a full lowmem check in kmemleak_*_phy)
> Signed-off-by: Yee Lee <yee.lee@mediatek.com>
> ---
>  mm/kmemleak.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index a182f5ddaf68..6b2af544aa0f 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -1132,7 +1132,7 @@ EXPORT_SYMBOL(kmemleak_no_scan);
>  void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, int min_count,
>                                gfp_t gfp)
>  {
> -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
> +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn))

Just skip checking will bring the crash possibility back. Seems it's beyond
these interfaces' handle scope for this situation, since "min_low_pfn" and
"max_low_pfn" are depending on arches.

>                 kmemleak_alloc(__va(phys), size, min_count, gfp);
>  }
>  EXPORT_SYMBOL(kmemleak_alloc_phys);
> @@ -1146,7 +1146,7 @@ EXPORT_SYMBOL(kmemleak_alloc_phys);
>   */
>  void __ref kmemleak_free_part_phys(phys_addr_t phys, size_t size)
>  {
> -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
> +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn))
>                 kmemleak_free_part(__va(phys), size);
>  }
>  EXPORT_SYMBOL(kmemleak_free_part_phys);
> @@ -1158,7 +1158,7 @@ EXPORT_SYMBOL(kmemleak_free_part_phys);
>   */
>  void __ref kmemleak_not_leak_phys(phys_addr_t phys)
>  {
> -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
> +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn))
>                 kmemleak_not_leak(__va(phys));
>  }
>  EXPORT_SYMBOL(kmemleak_not_leak_phys);
> @@ -1170,7 +1170,7 @@ EXPORT_SYMBOL(kmemleak_not_leak_phys);
>   */
>  void __ref kmemleak_ignore_phys(phys_addr_t phys)
>  {
> -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
> +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn))
>                 kmemleak_ignore(__va(phys));
>  }
>  EXPORT_SYMBOL(kmemleak_ignore_phys);
> --
> 2.18.0
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mm: kmemleak: Skip check in kmemleak_*_phys when pfn bound is not ready

[Yee Lee]

On Fri, 2022-05-27 at 21:39 +0800, patrick wang wrote:
> On Fri, May 27, 2022 at 11:25 AM <yee.lee@mediatek.com> wrote:
> > 
> > From: Yee Lee <yee.lee@mediatek.com>
> > 
> > In some archs (arm64), memblock allocates memory in boot time when
> > the pfn boundary (max_pfn/min_pfn) is not ready. The lowmen checks
> > in
> > kmemleak_*_phys() drop those blocks and cause some false leak
> > alarms
> > on common kernel objects.
> > 
> > Kmemleak output: (Qemu/arm64)
> > unreferenced object 0xffff0000c0170a00 (size 128):
> >   comm "swapper/0", pid 1, jiffies 4294892404 (age 126.208s)
> >   hex dump (first 32 bytes):
> >     62 61 73 65 00 00 00 00 00 00 00 00 00 00 00
> > 00  base............
> >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 00  ................
> >   backtrace:
> >     [<(____ptrval____)>] __kmalloc_track_caller+0x1b0/0x2e4
> >     [<(____ptrval____)>] kstrdup_const+0x8c/0xc4
> >     [<(____ptrval____)>] kvasprintf_const+0xbc/0xec
> >     [<(____ptrval____)>] kobject_set_name_vargs+0x58/0xe4
> >     [<(____ptrval____)>] kobject_add+0x84/0x100
> >     [<(____ptrval____)>] __of_attach_node_sysfs+0x78/0xec
> >     [<(____ptrval____)>] of_core_init+0x68/0x104
> >     [<(____ptrval____)>] driver_init+0x28/0x48
> >     [<(____ptrval____)>] do_basic_setup+0x14/0x28
> >     [<(____ptrval____)>] kernel_init_freeable+0x110/0x178
> >     [<(____ptrval____)>] kernel_init+0x20/0x1a0
> >     [<(____ptrval____)>] ret_from_fork+0x10/0x20
> > 
> > This patch relaxs the boundary checking in kmemleak_*_phys api
> > if max_low_pfn is uninitialzed.
> > 
> > Fixes: 23c2d4 (mm: kmemleak: take a full lowmem check in
> > kmemleak_*_phy)
> > Signed-off-by: Yee Lee <yee.lee@mediatek.com>
> > ---
> >  mm/kmemleak.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> > index a182f5ddaf68..6b2af544aa0f 100644
> > --- a/mm/kmemleak.c
> > +++ b/mm/kmemleak.c
> > @@ -1132,7 +1132,7 @@ EXPORT_SYMBOL(kmemleak_no_scan);
> >  void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, int
> > min_count,
> >                                gfp_t gfp)
> >  {
> > -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) <
> > max_low_pfn)
> > +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn &&
> > PHYS_PFN(phys) < max_low_pfn))
> 
> Just skip checking will bring the crash possibility back. Seems it's
> beyond
> these interfaces' handle scope for this situation, since
> "min_low_pfn" and
> "max_low_pfn" are depending on arches.
> 
Yes, for the cases beyond the pfn guard, users have to take care the
boundary by themselves. 

> >                 kmemleak_alloc(__va(phys), size, min_count, gfp);
> >  }
> >  EXPORT_SYMBOL(kmemleak_alloc_phys);
> > @@ -1146,7 +1146,7 @@ EXPORT_SYMBOL(kmemleak_alloc_phys);
> >   */
> >  void __ref kmemleak_free_part_phys(phys_addr_t phys, size_t size)
> >  {
> > -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) <
> > max_low_pfn)
> > +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn &&
> > PHYS_PFN(phys) < max_low_pfn))
> >                 kmemleak_free_part(__va(phys), size);
> >  }
> >  EXPORT_SYMBOL(kmemleak_free_part_phys);
> > @@ -1158,7 +1158,7 @@ EXPORT_SYMBOL(kmemleak_free_part_phys);
> >   */
> >  void __ref kmemleak_not_leak_phys(phys_addr_t phys)
> >  {
> > -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) <
> > max_low_pfn)
> > +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn &&
> > PHYS_PFN(phys) < max_low_pfn))
> >                 kmemleak_not_leak(__va(phys));
> >  }
> >  EXPORT_SYMBOL(kmemleak_not_leak_phys);
> > @@ -1170,7 +1170,7 @@ EXPORT_SYMBOL(kmemleak_not_leak_phys);
> >   */
> >  void __ref kmemleak_ignore_phys(phys_addr_t phys)
> >  {
> > -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) <
> > max_low_pfn)
> > +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn &&
> > PHYS_PFN(phys) < max_low_pfn))
> >                 kmemleak_ignore(__va(phys));
> >  }
> >  EXPORT_SYMBOL(kmemleak_ignore_phys);
> > --
> > 2.18.0
> > 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mm: kmemleak: Skip check in kmemleak_*_phys when pfn bound is not ready

[Patrick Wang]

On 2022/5/30 10:27, Yee Lee wrote:
> On Fri, 2022-05-27 at 21:39 +0800, patrick wang wrote:
>> On Fri, May 27, 2022 at 11:25 AM <yee.lee@mediatek.com> wrote:
>>>
>>> From: Yee Lee <yee.lee@mediatek.com>
>>>
>>> In some archs (arm64), memblock allocates memory in boot time when
>>> the pfn boundary (max_pfn/min_pfn) is not ready. The lowmen checks
>>> in
>>> kmemleak_*_phys() drop those blocks and cause some false leak
>>> alarms
>>> on common kernel objects.
>>>
>>> Kmemleak output: (Qemu/arm64)
>>> unreferenced object 0xffff0000c0170a00 (size 128):
>>>    comm "swapper/0", pid 1, jiffies 4294892404 (age 126.208s)
>>>    hex dump (first 32 bytes):
>>>      62 61 73 65 00 00 00 00 00 00 00 00 00 00 00
>>> 00  base............
>>>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 00  ................
>>>    backtrace:
>>>      [<(____ptrval____)>] __kmalloc_track_caller+0x1b0/0x2e4
>>>      [<(____ptrval____)>] kstrdup_const+0x8c/0xc4
>>>      [<(____ptrval____)>] kvasprintf_const+0xbc/0xec
>>>      [<(____ptrval____)>] kobject_set_name_vargs+0x58/0xe4
>>>      [<(____ptrval____)>] kobject_add+0x84/0x100
>>>      [<(____ptrval____)>] __of_attach_node_sysfs+0x78/0xec
>>>      [<(____ptrval____)>] of_core_init+0x68/0x104
>>>      [<(____ptrval____)>] driver_init+0x28/0x48
>>>      [<(____ptrval____)>] do_basic_setup+0x14/0x28
>>>      [<(____ptrval____)>] kernel_init_freeable+0x110/0x178
>>>      [<(____ptrval____)>] kernel_init+0x20/0x1a0
>>>      [<(____ptrval____)>] ret_from_fork+0x10/0x20
>>>
>>> This patch relaxs the boundary checking in kmemleak_*_phys api
>>> if max_low_pfn is uninitialzed.
>>>
>>> Fixes: 23c2d4 (mm: kmemleak: take a full lowmem check in
>>> kmemleak_*_phy)
>>> Signed-off-by: Yee Lee <yee.lee@mediatek.com>
>>> ---
>>>   mm/kmemleak.c | 8 ++++----
>>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
>>> index a182f5ddaf68..6b2af544aa0f 100644
>>> --- a/mm/kmemleak.c
>>> +++ b/mm/kmemleak.c
>>> @@ -1132,7 +1132,7 @@ EXPORT_SYMBOL(kmemleak_no_scan);
>>>   void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, int
>>> min_count,
>>>                                 gfp_t gfp)
>>>   {
>>> -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) <
>>> max_low_pfn)
>>> +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn &&
>>> PHYS_PFN(phys) < max_low_pfn))
>>
>> Just skip checking will bring the crash possibility back. Seems it's
>> beyond
>> these interfaces' handle scope for this situation, since
>> "min_low_pfn" and
>> "max_low_pfn" are depending on arches.
>>
> Yes, for the cases beyond the pfn guard, users have to take care the
> boundary by themselves.
> 

Could we record these early calling and deal with them when it's
ready? Is this appropriate?

I have an implementation based on this approach. Could you please
help to have a test on your machine as well? And someone to take
a look or review?



 From 82cae75dfaa78f414faf85bb49133e95159c041a Mon Sep 17 00:00:00 2001
From: Patrick Wang <patrick.wang.shcn@gmail.com>
Date: Mon, 30 May 2022 18:38:23 +0800
Subject: [PATCH] mm: kmemleak: record early operations and handle later

The kmemleak_*_phys() interface uses "min_low_pfn" and
"max_low_pfn" to check address. But on some architectures,
kmemleak_*_phys() is called before those two variables
initialized. Record these early operations and handle them
when kmemleak_*_phys() are ready.

Signed-off-by: Patrick Wang <patrick.wang.shcn@gmail.com>
---
  mm/kmemleak.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 158 insertions(+)

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index a182f5ddaf68..a71e41b49ebc 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -164,6 +164,26 @@ struct kmemleak_object {
  	char comm[TASK_COMM_LEN];	/* executable name */
  };
  
+/* maximum early recording count */
+#define EARLY_RECORDS	20
+
+/* early recording operation type */
+enum early_record_type {
+	record_alloc = 0,
+	record_free_part,
+	record_not_leak,
+	record_ignore
+};
+
+/* early recording operation */
+struct early_record_op {
+	enum early_record_type record_type;
+	phys_addr_t arg1;
+	size_t arg2;
+	int arg3;
+	gfp_t arg4;
+};
+
  /* flag representing the memory block allocation status */
  #define OBJECT_ALLOCATED	(1 << 0)
  /* flag set after the first reporting of an unreference object */
@@ -230,11 +250,26 @@ static int kmemleak_skip_disable;
  /* If there are leaks that can be reported */
  static bool kmemleak_found_leaks;
  
+/*
+ * Used to record early ops. Could recorded ops remain unhandled after
+ * initmem freed? Not likely.
+ */
+static struct early_record_op early_record_ops[EARLY_RECORDS] __initdata;
+static int early_record_op_count;
+/* indicate if recorded ops being handled */
+static bool early_record_in_handle;
+static DEFINE_RAW_SPINLOCK(early_record_lock);
+
  static bool kmemleak_verbose;
  module_param_named(verbose, kmemleak_verbose, bool, 0600);
  
  static void kmemleak_disable(void);
  
+static void record_early_ops(enum early_record_type record_type,
+			phys_addr_t arg1, size_t arg2,
+			int arg3, gfp_t arg4) __init;
+static void handle_early_ops(void) __init;
+
  /*
   * Print a warning and dump the stack trace.
   */
@@ -1132,6 +1167,26 @@ EXPORT_SYMBOL(kmemleak_no_scan);
  void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, int min_count,
  			       gfp_t gfp)
  {
+	/* Not ready, record this operation. */
+	if (!max_low_pfn && !early_record_in_handle) {
+		/*
+		 * record_early_ops is in __init section, make sure
+		 * text executable.
+		 */
+		if (core_kernel_text((unsigned long)record_early_ops))
+			record_early_ops(record_alloc, phys, size, min_count, gfp);
+		return;
+	}
+	/* Ready, handle recorded ops if they exit and not being handled. */
+	else if (early_record_op_count && !early_record_in_handle) {
+		/*
+		 * handle_early_ops is in __init section, make sure
+		 * text executable.
+		 */
+		if (core_kernel_text((unsigned long)handle_early_ops))
+			handle_early_ops();
+	}
+
  	if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
  		kmemleak_alloc(__va(phys), size, min_count, gfp);
  }
@@ -1146,6 +1201,18 @@ EXPORT_SYMBOL(kmemleak_alloc_phys);
   */
  void __ref kmemleak_free_part_phys(phys_addr_t phys, size_t size)
  {
+	/* Not ready, record this operation. */
+	if (!max_low_pfn && !early_record_in_handle) {
+		if (core_kernel_text((unsigned long)record_early_ops))
+			record_early_ops(record_free_part, phys, size, 0, 0);
+		return;
+	}
+	/* Ready, handle recorded ops if they exit and not being handled. */
+	else if (early_record_op_count && !early_record_in_handle) {
+		if (core_kernel_text((unsigned long)handle_early_ops))
+			handle_early_ops();
+	}
+
  	if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
  		kmemleak_free_part(__va(phys), size);
  }
@@ -1158,6 +1225,18 @@ EXPORT_SYMBOL(kmemleak_free_part_phys);
   */
  void __ref kmemleak_not_leak_phys(phys_addr_t phys)
  {
+	/* Not ready, record this operation. */
+	if (!max_low_pfn && !early_record_in_handle) {
+		if (core_kernel_text((unsigned long)record_early_ops))
+			record_early_ops(record_not_leak, phys, 0, 0, 0);
+		return;
+	}
+	/* Ready, handle recorded ops if they exit and not being handled. */
+	else if (early_record_op_count && !early_record_in_handle) {
+		if (core_kernel_text((unsigned long)handle_early_ops))
+			handle_early_ops();
+	}
+
  	if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
  		kmemleak_not_leak(__va(phys));
  }
@@ -1170,11 +1249,90 @@ EXPORT_SYMBOL(kmemleak_not_leak_phys);
   */
  void __ref kmemleak_ignore_phys(phys_addr_t phys)
  {
+	/* Not ready, record this operation. */
+	if (!max_low_pfn && !early_record_in_handle) {
+		if (core_kernel_text((unsigned long)record_early_ops))
+			record_early_ops(record_ignore, phys, 0, 0, 0);
+		return;
+	}
+	/* Ready, handle recorded ops if they exit and not being handled. */
+	else if (early_record_op_count && !early_record_in_handle) {
+		if (core_kernel_text((unsigned long)handle_early_ops))
+			handle_early_ops();
+	}
+
  	if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
  		kmemleak_ignore(__va(phys));
  }
  EXPORT_SYMBOL(kmemleak_ignore_phys);
  
+/*
+ * Record early operation to early_record_ops array.
+ */
+static void __init record_early_ops(enum early_record_type record_type,
+			phys_addr_t arg1, size_t arg2,
+			int arg3, gfp_t arg4)
+{
+	struct early_record_op *op;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&early_record_lock, flags);
+	/* early_record_ops array full */
+	if (early_record_op_count >= EARLY_RECORDS)
+		goto out;
+
+	op = &early_record_ops[early_record_op_count];
+	op->record_type = record_type;
+	op->arg1 = arg1;
+	op->arg2 = arg2;
+	op->arg3 = arg3;
+	op->arg4 = arg4;
+
+	early_record_op_count++;
+out:
+	raw_spin_unlock_irqrestore(&early_record_lock, flags);
+}
+
+/*
+ * Handle the whole recorded operations.
+ */
+static void __init handle_early_ops(void)
+{
+	struct early_record_op *op = early_record_ops;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&early_record_lock, flags);
+	/* Indicate operations are being handled. */
+	early_record_in_handle = true;
+
+	while (early_record_op_count) {
+		/* Deal with operations according to their type. */
+		switch (op->record_type) {
+		case record_alloc:
+			kmemleak_alloc_phys(op->arg1, op->arg2,
+					op->arg3, op->arg4);
+			break;
+		case record_free_part:
+			kmemleak_free_part_phys(op->arg1, op->arg2);
+			break;
+		case record_not_leak:
+			kmemleak_not_leak_phys(op->arg1);
+			break;
+		case record_ignore:
+			kmemleak_ignore_phys(op->arg1);
+			break;
+		default:
+			break;
+		}
+
+		early_record_op_count--;
+		op++;
+	}
+
+	early_record_in_handle = false;
+	raw_spin_unlock_irqrestore(&early_record_lock, flags);
+}
+
  /*
   * Update an object's checksum and return true if it was modified.
   */
-- 
2.25.1


Thanks,
Patrick

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mm: kmemleak: Skip check in kmemleak_*_phys when pfn bound is not ready

[Catalin Marinas]

Hi Patrick,

On Mon, May 30, 2022 at 09:32:18PM +0800, Patrick Wang wrote:
> On 2022/5/30 10:27, Yee Lee wrote:
> > On Fri, 2022-05-27 at 21:39 +0800, patrick wang wrote:
> > > On Fri, May 27, 2022 at 11:25 AM <yee.lee@mediatek.com> wrote:
> > > > From: Yee Lee <yee.lee@mediatek.com>
> > > > 
> > > > In some archs (arm64), memblock allocates memory in boot time when
> > > > the pfn boundary (max_pfn/min_pfn) is not ready. The lowmen checks in
> > > > kmemleak_*_phys() drop those blocks and cause some false leak alarms
> > > > on common kernel objects.
> > > > 
> > > > Kmemleak output: (Qemu/arm64)
> > > > unreferenced object 0xffff0000c0170a00 (size 128):
> > > >    comm "swapper/0", pid 1, jiffies 4294892404 (age 126.208s)
> > > >    hex dump (first 32 bytes):
> > > >      62 61 73 65 00 00 00 00 00 00 00 00 00 00 00 00  base............
> > > >      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > > >    backtrace:
> > > >      [<(____ptrval____)>] __kmalloc_track_caller+0x1b0/0x2e4
> > > >      [<(____ptrval____)>] kstrdup_const+0x8c/0xc4
> > > >      [<(____ptrval____)>] kvasprintf_const+0xbc/0xec
> > > >      [<(____ptrval____)>] kobject_set_name_vargs+0x58/0xe4
> > > >      [<(____ptrval____)>] kobject_add+0x84/0x100
> > > >      [<(____ptrval____)>] __of_attach_node_sysfs+0x78/0xec
> > > >      [<(____ptrval____)>] of_core_init+0x68/0x104
> > > >      [<(____ptrval____)>] driver_init+0x28/0x48
> > > >      [<(____ptrval____)>] do_basic_setup+0x14/0x28
> > > >      [<(____ptrval____)>] kernel_init_freeable+0x110/0x178
> > > >      [<(____ptrval____)>] kernel_init+0x20/0x1a0
> > > >      [<(____ptrval____)>] ret_from_fork+0x10/0x20
> > > > 
> > > > This patch relaxs the boundary checking in kmemleak_*_phys api
> > > > if max_low_pfn is uninitialzed.
> > > > 
> > > > Fixes: 23c2d4 (mm: kmemleak: take a full lowmem check in kmemleak_*_phy)

BTW, please use at least 12 characters for the git sha1, the above is
ambiguous.

> > > > Signed-off-by: Yee Lee <yee.lee@mediatek.com>
> > > > ---
> > > >   mm/kmemleak.c | 8 ++++----
> > > >   1 file changed, 4 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> > > > index a182f5ddaf68..6b2af544aa0f 100644
> > > > --- a/mm/kmemleak.c
> > > > +++ b/mm/kmemleak.c
> > > > @@ -1132,7 +1132,7 @@ EXPORT_SYMBOL(kmemleak_no_scan);
> > > >   void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, int min_count,
> > > >                                 gfp_t gfp)
> > > >   {
> > > > -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
> > > > +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn))
> > > 
> > > Just skip checking will bring the crash possibility back. Seems
> > > it's beyond these interfaces' handle scope for this situation,
> > > since "min_low_pfn" and "max_low_pfn" are depending on arches.
> > 
> > Yes, for the cases beyond the pfn guard, users have to take care the
> > boundary by themselves.
> 
> Could we record these early calling and deal with them when it's
> ready? Is this appropriate?
> 
> I have an implementation based on this approach. Could you please
> help to have a test on your machine as well? And someone to take
> a look or review?

We had something similar until 5.4, removed by commit c5665868183f ("mm:
kmemleak: use the memory pool for early allocations"). It was a bit
painful as we never had the right buffer, so I'm not keen on adding it
back.

> From 82cae75dfaa78f414faf85bb49133e95159c041a Mon Sep 17 00:00:00 2001
> From: Patrick Wang <patrick.wang.shcn@gmail.com>
> Date: Mon, 30 May 2022 18:38:23 +0800
> Subject: [PATCH] mm: kmemleak: record early operations and handle later
> 
> The kmemleak_*_phys() interface uses "min_low_pfn" and
> "max_low_pfn" to check address. But on some architectures,
> kmemleak_*_phys() is called before those two variables
> initialized. Record these early operations and handle them
> when kmemleak_*_phys() are ready.

Could we instead record everything (no checks) but later avoid scanning
if below min or above max_low_pfn? We can add an OBJECT_PHYS flag to all
objects allocated via kmemleak_*_phys() and always check the
virt_to_phys() boundaries at scan time. It may actually help with this
problem as well:

https://lore.kernel.org/r/9dd08bb5-f39e-53d8-f88d-bec598a08c93@gmail.com

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] mm: kmemleak: Skip check in kmemleak_*_phys when pfn bound is not ready

[patrick wang]

On Mon, May 30, 2022 at 10:57 PM Catalin Marinas
<catalin.marinas@arm.com> wrote:
>
> Hi Patrick,
>
> On Mon, May 30, 2022 at 09:32:18PM +0800, Patrick Wang wrote:
> > On 2022/5/30 10:27, Yee Lee wrote:
> > > On Fri, 2022-05-27 at 21:39 +0800, patrick wang wrote:
> > > > On Fri, May 27, 2022 at 11:25 AM <yee.lee@mediatek.com> wrote:
> > > > > From: Yee Lee <yee.lee@mediatek.com>
> > > > >
> > > > > In some archs (arm64), memblock allocates memory in boot time when
> > > > > the pfn boundary (max_pfn/min_pfn) is not ready. The lowmen checks in
> > > > > kmemleak_*_phys() drop those blocks and cause some false leak alarms
> > > > > on common kernel objects.
> > > > >
> > > > > Kmemleak output: (Qemu/arm64)
> > > > > unreferenced object 0xffff0000c0170a00 (size 128):
> > > > >    comm "swapper/0", pid 1, jiffies 4294892404 (age 126.208s)
> > > > >    hex dump (first 32 bytes):
> > > > >      62 61 73 65 00 00 00 00 00 00 00 00 00 00 00 00  base............
> > > > >      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> > > > >    backtrace:
> > > > >      [<(____ptrval____)>] __kmalloc_track_caller+0x1b0/0x2e4
> > > > >      [<(____ptrval____)>] kstrdup_const+0x8c/0xc4
> > > > >      [<(____ptrval____)>] kvasprintf_const+0xbc/0xec
> > > > >      [<(____ptrval____)>] kobject_set_name_vargs+0x58/0xe4
> > > > >      [<(____ptrval____)>] kobject_add+0x84/0x100
> > > > >      [<(____ptrval____)>] __of_attach_node_sysfs+0x78/0xec
> > > > >      [<(____ptrval____)>] of_core_init+0x68/0x104
> > > > >      [<(____ptrval____)>] driver_init+0x28/0x48
> > > > >      [<(____ptrval____)>] do_basic_setup+0x14/0x28
> > > > >      [<(____ptrval____)>] kernel_init_freeable+0x110/0x178
> > > > >      [<(____ptrval____)>] kernel_init+0x20/0x1a0
> > > > >      [<(____ptrval____)>] ret_from_fork+0x10/0x20
> > > > >
> > > > > This patch relaxs the boundary checking in kmemleak_*_phys api
> > > > > if max_low_pfn is uninitialzed.
> > > > >
> > > > > Fixes: 23c2d4 (mm: kmemleak: take a full lowmem check in kmemleak_*_phy)
>
> BTW, please use at least 12 characters for the git sha1, the above is
> ambiguous.
>
> > > > > Signed-off-by: Yee Lee <yee.lee@mediatek.com>
> > > > > ---
> > > > >   mm/kmemleak.c | 8 ++++----
> > > > >   1 file changed, 4 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> > > > > index a182f5ddaf68..6b2af544aa0f 100644
> > > > > --- a/mm/kmemleak.c
> > > > > +++ b/mm/kmemleak.c
> > > > > @@ -1132,7 +1132,7 @@ EXPORT_SYMBOL(kmemleak_no_scan);
> > > > >   void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, int min_count,
> > > > >                                 gfp_t gfp)
> > > > >   {
> > > > > -       if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
> > > > > +       if (!max_low_pfn || (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn))
> > > >
> > > > Just skip checking will bring the crash possibility back. Seems
> > > > it's beyond these interfaces' handle scope for this situation,
> > > > since "min_low_pfn" and "max_low_pfn" are depending on arches.
> > >
> > > Yes, for the cases beyond the pfn guard, users have to take care the
> > > boundary by themselves.
> >
> > Could we record these early calling and deal with them when it's
> > ready? Is this appropriate?
> >
> > I have an implementation based on this approach. Could you please
> > help to have a test on your machine as well? And someone to take
> > a look or review?
>
> We had something similar until 5.4, removed by commit c5665868183f ("mm:
> kmemleak: use the memory pool for early allocations"). It was a bit
> painful as we never had the right buffer, so I'm not keen on adding it
> back.

Agreed.

> > From 82cae75dfaa78f414faf85bb49133e95159c041a Mon Sep 17 00:00:00 2001
> > From: Patrick Wang <patrick.wang.shcn@gmail.com>
> > Date: Mon, 30 May 2022 18:38:23 +0800
> > Subject: [PATCH] mm: kmemleak: record early operations and handle later
> >
> > The kmemleak_*_phys() interface uses "min_low_pfn" and
> > "max_low_pfn" to check address. But on some architectures,
> > kmemleak_*_phys() is called before those two variables
> > initialized. Record these early operations and handle them
> > when kmemleak_*_phys() are ready.
>
> Could we instead record everything (no checks) but later avoid scanning
> if below min or above max_low_pfn? We can add an OBJECT_PHYS flag to all
> objects allocated via kmemleak_*_phys() and always check the
> virt_to_phys() boundaries at scan time. It may actually help with this
> problem as well:
>
> https://lore.kernel.org/r/9dd08bb5-f39e-53d8-f88d-bec598a08c93@gmail.com

Check in kmemleak_*_phys()  even recorded early operations, the condition
still relies on archs somewhat and is not that clear. Checking at scan
time should
be a proper direction. So that kmemleak doesn't have to rely on archs.

I have implemented it in this direction, and I believe it‘s also
helpful for the above
false positive report.

Thanks,
Patrick

>
> --
> Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel