[PATCH v2 0/4] mm: kmemleak: store objects allocated with physical address separately and check when scan

Patrick Wang posted 4 patches 3 years, 10 months ago
There is a newer version of this series
mm/kmemleak.c | 193 ++++++++++++++++++++++++++++++++------------------
1 file changed, 123 insertions(+), 70 deletions(-)
[PATCH v2 0/4] mm: kmemleak: store objects allocated with physical address separately and check when scan
Posted by Patrick Wang 3 years, 10 months ago
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. The following steps will be taken:

1) Add OBJECT_PHYS flag and rbtree for the objects allocated
   with physical address
2) Store physical address in objects if allocated with OBJECT_PHYS
3) Check the boundary when scan instead of in kmemleak_*_phys()

This patch set will solve:
https://lore.kernel.org/r/20220527032504.30341-1-yee.lee@mediatek.com
https://lore.kernel.org/r/9dd08bb5-f39e-53d8-f88d-bec598a08c93@gmail.com

v1: https://lore.kernel.org/r/20220531150823.1004101-1-patrick.wang.shcn@gmail.com

v1->v2:
 - add rbtree for the objects allocated with physical address
 - store physical address in objects if allocated with OBJECT_PHYS
 - check the upper object boundary as well and avoid duplicate check

Patrick Wang (4):
  mm: kmemleak: add OBJECT_PHYS flag for objects allocated with physical
    address
  mm: kmemleak: add rbtree for objects allocated with physical address
  mm: kmemleak: handle address stored in object based on its type
  mm: kmemleak: kmemleak_*_phys() set address type and check PA when
    scan

 mm/kmemleak.c | 193 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 123 insertions(+), 70 deletions(-)

-- 
2.25.1
Re: [PATCH v2 0/4] mm: kmemleak: store objects allocated with physical address separately and check when scan
Posted by Kuan-Ying Lee 3 years, 10 months ago
On Fri, 2022-06-03 at 11:54 +0800, Patrick Wang wrote:
> 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. The following steps will be taken:
> 
> 1) Add OBJECT_PHYS flag and rbtree for the objects allocated
>    with physical address
> 2) Store physical address in objects if allocated with OBJECT_PHYS
> 3) Check the boundary when scan instead of in kmemleak_*_phys()
> 
> This patch set will solve:
> https://lore.kernel.org/r/20220527032504.30341-1-yee.lee@mediatek.com
> 
https://lore.kernel.org/r/9dd08bb5-f39e-53d8-f88d-bec598a08c93@gmail.com

Hi Patrick,

If this patchset fix the above issue, I think we need to add
the below fixes tag.

Fixes: 23c2d497de21 ("mm: kmemleak: take a full lowmem check in
kmemleak_*_phys()")

Thanks.

> 
> v1: 
> https://lore.kernel.org/r/20220531150823.1004101-1-patrick.wang.shcn@gmail.com
> 
> v1->v2:
>  - add rbtree for the objects allocated with physical address
>  - store physical address in objects if allocated with OBJECT_PHYS
>  - check the upper object boundary as well and avoid duplicate check
> 
> Patrick Wang (4):
>   mm: kmemleak: add OBJECT_PHYS flag for objects allocated with
> physical
>     address
>   mm: kmemleak: add rbtree for objects allocated with physical
> address
>   mm: kmemleak: handle address stored in object based on its type
>   mm: kmemleak: kmemleak_*_phys() set address type and check PA when
>     scan
> 
>  mm/kmemleak.c | 193 ++++++++++++++++++++++++++++++++--------------
> ----
>  1 file changed, 123 insertions(+), 70 deletions(-)
> 
> -- 
> 2.25.1
> 
>
Re: [PATCH v2 0/4] mm: kmemleak: store objects allocated with physical address separately and check when scan
Posted by patrick wang 3 years, 10 months ago
On Wed, Jun 8, 2022 at 10:46 AM Kuan-Ying Lee
<Kuan-Ying.Lee@mediatek.com> wrote:
>
> On Fri, 2022-06-03 at 11:54 +0800, Patrick Wang wrote:
> > 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. The following steps will be taken:
> >
> > 1) Add OBJECT_PHYS flag and rbtree for the objects allocated
> >    with physical address
> > 2) Store physical address in objects if allocated with OBJECT_PHYS
> > 3) Check the boundary when scan instead of in kmemleak_*_phys()
> >
> > This patch set will solve:
> > https://lore.kernel.org/r/20220527032504.30341-1-yee.lee@mediatek.com
> >
> https://lore.kernel.org/r/9dd08bb5-f39e-53d8-f88d-bec598a08c93@gmail.com
>
> Hi Patrick,
>
> If this patchset fix the above issue, I think we need to add
> the below fixes tag.
>
> Fixes: 23c2d497de21 ("mm: kmemleak: take a full lowmem check in
> kmemleak_*_phys()")

Hi Kuan-Ying,

Thanks for taking a look.

This series should solve the false positive on ppc32 and arm64.
And the false positive on arm64 is triggered by commit
23c2d497de21. So I will add the fixes tag.

Thanks,
Patrick
Re: [PATCH v2 0/4] mm: kmemleak: store objects allocated with physical address separately and check when scan
Posted by Catalin Marinas 3 years, 10 months ago
On Fri, Jun 03, 2022 at 11:54:11AM +0800, Patrick Wang wrote:
> Patrick Wang (4):
>   mm: kmemleak: add OBJECT_PHYS flag for objects allocated with physical
>     address
>   mm: kmemleak: add rbtree for objects allocated with physical address
>   mm: kmemleak: handle address stored in object based on its type
>   mm: kmemleak: kmemleak_*_phys() set address type and check PA when
>     scan

This looks fine at a very quick look but I'll do a in-depth review next
week. One more thing needed is to remove the min_count argument to
kmemleak_alloc_phys() and assume it's always 0. After this series we
can't track them for leaking anyway.

Thanks for putting this together.

-- 
Catalin
Re: [PATCH v2 0/4] mm: kmemleak: store objects allocated with physical address separately and check when scan
Posted by patrick wang 3 years, 10 months ago
On Fri, Jun 3, 2022 at 7:01 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Fri, Jun 03, 2022 at 11:54:11AM +0800, Patrick Wang wrote:
> > Patrick Wang (4):
> >   mm: kmemleak: add OBJECT_PHYS flag for objects allocated with physical
> >     address
> >   mm: kmemleak: add rbtree for objects allocated with physical address
> >   mm: kmemleak: handle address stored in object based on its type
> >   mm: kmemleak: kmemleak_*_phys() set address type and check PA when
> >     scan
>
> This looks fine at a very quick look but I'll do a in-depth review next
> week. One more thing needed is to remove the min_count argument to
> kmemleak_alloc_phys() and assume it's always 0. After this series we
> can't track them for leaking anyway.

Will do in the next version.

Thanks,
Patrick