mm: kmemleak: store objects allocated with physical address separately and check when scan

PATCH Superseded Patrick Wang 2 participants 9 messages Jun 9, 2022 – Jun 11, 2022

Subsystems (6)

Memory management Documentation Device tree Documentation process +2 more subsystems

lore

Share & tools

lore

Patrick Wang

author

2022-06-09 12:50 UTC #0 lore raw

v3 cover mm: kmemleak: store objects allocated with physical address separately and check when scan

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/[email protected]
https://lore.kernel.org/r/[email protected]

v2: https://lore.kernel.org/r/[email protected]
v1: https://lore.kernel.org/r/[email protected]

v2->v3:
- remove the min_count argument to kmemleak_alloc_phys() function and assume it's 0
- remove unused kmemleak_not_leak_phys() function
- add functions to reduce unnecessary changes
- remove the check for kasan_reset_tag()
- add Fixes tag in patch 3

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 (3):
mm: kmemleak: add OBJECT_PHYS flag for objects allocated with physical
address
mm: kmemleak: add rbtree and store physical address for objects
allocated with PA
mm: kmemleak: check physical address when scan

 Documentation/dev-tools/kmemleak.rst    |   1 -
 drivers/of/fdt.c                        |   2 +-
 include/linux/kmemleak.h                |   8 +-
 mm/kmemleak.c                           | 200 ++++++++++++++++--------
 mm/memblock.c                           |  14 +-
 tools/testing/memblock/linux/kmemleak.h |   2 +-
 6 files changed, 145 insertions(+), 82 deletions(-)

Signature · 1 line

2.25.1

Patrick Wang

author

2022-06-09 12:51 UTC #1 lore raw

PATCH v3 1/3 mm: kmemleak: add OBJECT_PHYS flag for objects allocated with physical address

Add OBJECT_PHYS flag for object. This flag is used to identify the objects allocated with physical address.The create_object_phys() function is added as well to set that flag. And remove the min_count argument to kmemleak_alloc_phys() function, assume it's 0.

Signed-off-by Patrick Wang <[email protected]>

---
 drivers/of/fdt.c                        |  2 +-
 include/linux/kmemleak.h                |  4 +--
 mm/kmemleak.c                           | 44 ++++++++++++++++++-------
 mm/memblock.c                           | 14 ++++----
 tools/testing/memblock/linux/kmemleak.h |  2 +-
 5 files changed, 43 insertions(+), 23 deletions(-)

drivers/of/fdt.c +1 -1

index a8f5b6532165..2c677e84c3f5 100644
@@ -529,7 +529,7 @@ static int __init __reserved_mem_reserve_reg(unsigned long node,
 			pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
 				uname, &base, (unsigned long)(size / SZ_1M));
 			if (!nomap)
-				kmemleak_alloc_phys(base, size, 0, 0);
+				kmemleak_alloc_phys(base, size, 0);
 		}
 		else
 			pr_info("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",

include/linux/kmemleak.h +2 -2

index 34684b2026ab..df245d875b1e 100644
@@ -29,7 +29,7 @@ extern void kmemleak_not_leak(const void *ptr) __ref;
 extern void kmemleak_ignore(const void *ptr) __ref;
 extern void kmemleak_scan_area(const void *ptr, size_t size, gfp_t gfp) __ref;
 extern void kmemleak_no_scan(const void *ptr) __ref;
-extern void kmemleak_alloc_phys(phys_addr_t phys, size_t size, int min_count,
+extern void kmemleak_alloc_phys(phys_addr_t phys, size_t size,
 				gfp_t gfp) __ref;
 extern void kmemleak_free_part_phys(phys_addr_t phys, size_t size) __ref;
 extern void kmemleak_not_leak_phys(phys_addr_t phys) __ref;

@@ -107,7 +107,7 @@ static inline void kmemleak_no_scan(const void *ptr)
 {
 }
 static inline void kmemleak_alloc_phys(phys_addr_t phys, size_t size,
-				       int min_count, gfp_t gfp)
+				       gfp_t gfp)
 {
 }
 static inline void kmemleak_free_part_phys(phys_addr_t phys, size_t size)

mm/kmemleak.c +32 ~~-12~~

85 lines not on this page · raw message

mm/memblock.c +7 -7

index e4f03a6e8e56..749abd2685c4 100644
@@ -1345,8 +1345,8 @@ __next_mem_pfn_range_in_zone(u64 *idx, struct zone *zone,
  * from the regions with mirroring enabled and then retried from any
  * memory region.
  *
- * In addition, function sets the min_count to 0 using kmemleak_alloc_phys for
- * allocated boot memory block, so that it is never reported as leaks.
+ * In addition, function using kmemleak_alloc_phys for allocated boot
+ * memory block, it is never reported as leaks.
  *
  * Return:
  * Physical address of allocated memory block on success, %0 on failure.

@@ -1398,12 +1398,12 @@ phys_addr_t __init memblock_alloc_range_nid(phys_addr_t size,
 	 */
 	if (end != MEMBLOCK_ALLOC_NOLEAKTRACE)
 		/*
-		 * The min_count is set to 0 so that memblock allocated
-		 * blocks are never reported as leaks. This is because many
-		 * of these blocks are only referred via the physical
-		 * address which is not looked up by kmemleak.
+		 * Memblock allocated blocks are never reported as
+		 * leaks. This is because many of these blocks are
+		 * only referred via the physical address which is
+		 * not looked up by kmemleak.
 		 */
-		kmemleak_alloc_phys(found, size, 0, 0);
+		kmemleak_alloc_phys(found, size, 0);
 
 	return found;
 }

tools/testing/memblock/linux/kmemleak.h +1 -1

index 462f8c5e8aa0..5fed13bb9ec4 100644
@@ -7,7 +7,7 @@ static inline void kmemleak_free_part_phys(phys_addr_t phys, size_t size)
 }
 
 static inline void kmemleak_alloc_phys(phys_addr_t phys, size_t size,
-				       int min_count, gfp_t gfp)
+				       gfp_t gfp)
 {
 }

Signature · 1 line

2.25.1

Catalin Marinas

maintainer Reviewed-by on v3 1/3

2022-06-09 18:06 UTC #2 lore raw

Re: [PATCH v3 1/3] mm: kmemleak: add OBJECT_PHYS flag for objects allocated with physical address

Quoted 62 lines · Patrick Wang

mm/kmemleak.c +8 -5

@@ -1125,15 +1142,18 @@ EXPORT_SYMBOL(kmemleak_no_scan);
  *			 address argument
  * @phys:	physical address of the object
  * @size:	size of the object
- * @min_count:	minimum number of references to this object.
- *              See kmemleak_alloc()
  * @gfp:	kmalloc() flags used for kmemleak internal memory allocations
  */
-void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, int min_count,
-			       gfp_t gfp)
+void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, gfp_t gfp)
 {
+	pr_debug("%s(0x%pa, %zu)\n", __func__, &phys, size);
+
 	if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
-		kmemleak_alloc(__va(phys), size, min_count, gfp);
+		/*
+		 * Create object with OBJECT_PHYS flag and
+		 * assume min_count 0.
+		 */
+		create_object_phys((unsigned long)__va(phys), size, 0, gfp);
 }
 EXPORT_SYMBOL(kmemleak_alloc_phys);

mm/memblock.c +7 -7

index e4f03a6e8e56..749abd2685c4 100644
@@ -1345,8 +1345,8 @@ __next_mem_pfn_range_in_zone(u64 *idx, struct zone *zone,
  * from the regions with mirroring enabled and then retried from any
  * memory region.
  *
- * In addition, function sets the min_count to 0 using kmemleak_alloc_phys for
- * allocated boot memory block, so that it is never reported as leaks.
+ * In addition, function using kmemleak_alloc_phys for allocated boot
+ * memory block, it is never reported as leaks.
  *
  * Return:
  * Physical address of allocated memory block on success, %0 on failure.

@@ -1398,12 +1398,12 @@ phys_addr_t __init memblock_alloc_range_nid(phys_addr_t size,
 	 */
 	if (end != MEMBLOCK_ALLOC_NOLEAKTRACE)
 		/*
-		 * The min_count is set to 0 so that memblock allocated
-		 * blocks are never reported as leaks. This is because many
-		 * of these blocks are only referred via the physical
-		 * address which is not looked up by kmemleak.
+		 * Memblock allocated blocks are never reported as
+		 * leaks. This is because many of these blocks are
+		 * only referred via the physical address which is
+		 * not looked up by kmemleak.
 		 */
-		kmemleak_alloc_phys(found, size, 0, 0);
+		kmemleak_alloc_phys(found, size, 0);
 
 	return found;
 }

tools/testing/memblock/linux/kmemleak.h +0 -0

index 462f8c5e8aa0..5fed13bb9ec4 100644

tools/testing/memblock/linux/kmemleak.h static inline void kmemleak_free_part_phys(phys_addr_t phys, size_t size) · line 12

Quoted 7 lines

tools/testing/memblock/linux/kmemleak.h +1 -1

@@ -7,7 +7,7 @@ static inline void kmemleak_free_part_phys(phys_addr_t phys, size_t size)
 }
 
 static inline void kmemleak_alloc_phys(phys_addr_t phys, size_t size,
-				       int min_count, gfp_t gfp)
+				       gfp_t gfp)
 {
 }

If you respin, I'd move the prototype change to a separate patch (and make it first in the series). Otherwise it looks fine:

Reviewed-by Catalin Marinas <[email protected]>

Patrick Wang

author

2022-06-09 12:51 UTC #3 lore raw

PATCH v3 2/3 mm: kmemleak: add rbtree and store physical address for objects allocated with PA

In reply to Patrick Wang

Add object_phys_tree_root to store the objects allocated with physical address. Distinguish it from object_tree_root by OBJECT_PHYS flag or function argument. The physical address is stored directly in those objects. And remove the unused kmemleak_not_leak_phys() function.

Signed-off-by Patrick Wang <[email protected]>

---
 Documentation/dev-tools/kmemleak.rst |   1 -
 include/linux/kmemleak.h             |   4 -
 mm/kmemleak.c                        | 145 +++++++++++++++++----------
 3 files changed, 91 insertions(+), 59 deletions(-)

Documentation/dev-tools/kmemleak.rst +0 -1

index 1c935f41cd3a..5483fd39ef29 100644
@@ -174,7 +174,6 @@ mapping:
 
 - ``kmemleak_alloc_phys``
 - ``kmemleak_free_part_phys``
-- ``kmemleak_not_leak_phys``
 - ``kmemleak_ignore_phys``
 
 Dealing with false positives/negatives

include/linux/kmemleak.h +0 -4

index df245d875b1e..6a3cd1bf4680 100644
@@ -32,7 +32,6 @@ extern void kmemleak_no_scan(const void *ptr) __ref;
 extern void kmemleak_alloc_phys(phys_addr_t phys, size_t size,
 				gfp_t gfp) __ref;
 extern void kmemleak_free_part_phys(phys_addr_t phys, size_t size) __ref;
-extern void kmemleak_not_leak_phys(phys_addr_t phys) __ref;
 extern void kmemleak_ignore_phys(phys_addr_t phys) __ref;
 
 static inline void kmemleak_alloc_recursive(const void *ptr, size_t size,

@@ -113,9 +112,6 @@ static inline void kmemleak_alloc_phys(phys_addr_t phys, size_t size,
 static inline void kmemleak_free_part_phys(phys_addr_t phys, size_t size)
 {
 }
-static inline void kmemleak_not_leak_phys(phys_addr_t phys)
-{
-}
 static inline void kmemleak_ignore_phys(phys_addr_t phys)
 {
 }

mm/kmemleak.c +91 ~~-54~~

327 lines not on this page · raw message

Signature · 1 line

2.25.1

Catalin Marinas

maintainer Reviewed-by on v3 2/3

2022-06-09 18:10 UTC #4 lore raw

Re: [PATCH v3 2/3] mm: kmemleak: add rbtree and store physical address for objects allocated with PA

Quoted 19 lines · Patrick Wang

Add object_phys_tree_root to store the objects allocated with physical address. Distinguish it from object_tree_root by OBJECT_PHYS flag or function argument. The physical address is stored directly in those objects. And remove the unused kmemleak_not_leak_phys() function.
Suggested-by Catalin Marinas <[email protected]>
Signed-off-by Patrick Wang <[email protected]>
---
 Documentation/dev-tools/kmemleak.rst |   1 -
 include/linux/kmemleak.h             |   4 -
 mm/kmemleak.c                        | 145 +++++++++++++++++----------
 3 files changed, 91 insertions(+), 59 deletions(-)
Documentation/dev-tools/kmemleak.rst +0 -0
index 1c935f41cd3a..5483fd39ef29 100644

Documentation/dev-tools/kmemleak.rst mapping: · line 177

Quoted 5 lines

Documentation/dev-tools/kmemleak.rst +0 -1

@@ -174,7 +174,6 @@ mapping:
 
 - ``kmemleak_alloc_phys``
 - ``kmemleak_free_part_phys``
-- ``kmemleak_not_leak_phys``
 - ``kmemleak_ignore_phys``

Same comment - only if you respin, move the kmemleak_not_leak_phys() removal to a separate patch. Otherwise:

Reviewed-by Catalin Marinas <[email protected]>

Patrick Wang

author

2022-06-09 12:51 UTC #5 lore raw

PATCH v3 3/3 mm: kmemleak: check physical address when scan

In reply to Patrick Wang

Check the physical address of objects for its boundary when scan instead of in kmemleak_*_phys().

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

Signed-off-by Patrick Wang <[email protected]>

---
 mm/kmemleak.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

mm/kmemleak.c +14 -3

index 155f50e1a604..387d6fa402c6 100644
@@ -1184,7 +1184,7 @@ void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, gfp_t gfp)
 {
 	pr_debug("%s(0x%pa, %zu)\n", __func__, &phys, size);
 
-	if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
+	if (kmemleak_enabled)
 		/*
 		 * Create object with OBJECT_PHYS flag and
 		 * assume min_count 0.

@@ -1204,7 +1204,7 @@ void __ref kmemleak_free_part_phys(phys_addr_t phys, size_t size)
 {
 	pr_debug("%s(0x%pa)\n", __func__, &phys);
 
-	if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
+	if (kmemleak_enabled)
 		delete_object_part((unsigned long)phys, size, true);
 }
 EXPORT_SYMBOL(kmemleak_free_part_phys);

@@ -1218,7 +1218,7 @@ void __ref kmemleak_ignore_phys(phys_addr_t phys)
 {
 	pr_debug("%s(0x%pa)\n", __func__, &phys);
 
-	if (PHYS_PFN(phys) >= min_low_pfn && PHYS_PFN(phys) < max_low_pfn)
+	if (kmemleak_enabled)
 		make_black_object((unsigned long)phys, true);
 }
 EXPORT_SYMBOL(kmemleak_ignore_phys);

@@ -1493,6 +1493,17 @@ static void kmemleak_scan(void)
 			dump_object_info(object);
 		}
 #endif
+
+		/* ignore objects outside lowmem (paint them black) */
+		if ((object->flags & OBJECT_PHYS) &&
+		   !(object->flags & OBJECT_NO_SCAN)) {
+			unsigned long phys = object->pointer;
+
+			if (PHYS_PFN(phys) < min_low_pfn ||
+			    PHYS_PFN(phys + object->size) >= max_low_pfn)
+				__paint_it(object, KMEMLEAK_BLACK);
+		}
+
 		/* reset the reference count (whiten the object) */
 		object->count = 0;
 		if (color_gray(object) && get_object(object))

Signature · 1 line

2.25.1

Catalin Marinas

maintainer Reviewed-by on v3 3/3

2022-06-09 18:16 UTC #6 lore raw

Re: [PATCH v3 3/3] mm: kmemleak: check physical address when scan

Quoted 6 lines · Patrick Wang

Check the physical address of objects for its boundary when scan instead of in kmemleak_*_phys().
Fixes 23c2d497de21 ("mm: kmemleak: take a full lowmem check in kmemleak_*_phys()")
Suggested-by Catalin Marinas <[email protected]>
Signed-off-by Patrick Wang <[email protected]>

Reviewed-by Catalin Marinas <[email protected]>

The fixed commit above was cc stable, so we'll probably need all these three patches in stable. But I'd keep them a bit in -next for testing first (and I see Andrew already picked them up; we might as well merge them in 5.20 and send them to -stable after, it's not some critical feature).

Thanks for the series. I don't think you need to respin unless others of comments.

Signature · 1 line

Catalin

Patrick Wang

author

2022-06-11 03:46 UTC #7 lore raw

Re: [PATCH v3 3/3] mm: kmemleak: check physical address when scan

Quoted 10 lines · Catalin Marinas

Quoted 6 lines · Patrick Wang
Check the physical address of objects for its boundary when scan instead of in kmemleak_*_phys().
Fixes 23c2d497de21 ("mm: kmemleak: take a full lowmem check in kmemleak_*_phys()")
Suggested-by Catalin Marinas <[email protected]>
Signed-off-by Patrick Wang <[email protected]>
Reviewed-by Catalin Marinas <[email protected]>

Quoted 8 lines

The fixed commit above was cc stable, so we'll probably need all these three patches in stable. But I'd keep them a bit in -next for testing first (and I see Andrew already picked them up; we might as well merge them in 5.20 and send them to -stable after, it's not some critical feature).
Thanks for the series. I don't think you need to respin unless others of comments.

I've received an auto build test WARNING from kernel test robot:

mm/kmemleak.c: In function 'scan_object':

Quoted 1 line

Quoted 1 line
arch/powerpc/include/asm/page.h:215:42: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]

       215 | #define __va(x) ((void *)(unsigned long)((phys_addr_t)(x) + VIRT_PHYS_OFFSET))
           |                                          ^
     mm/kmemleak.c:1403:19: note: in expansion of macro '__va'
      1403 |                   __va((void *)object->pointer) :
           |                   ^~~~

So I will replace __va((void *)object->pointer)
to __va((phys_addr_t)object->pointer) for fixing this warning, and move the prototype change and the kmemleak_not_leak_phys() removal to a separate one as you suggested at the same time.

Thanks for these comments and suggestions.

Thanks,
Patrick

Catalin Marinas

maintainer

2022-06-11 09:46 UTC #8 lore raw

Re: [PATCH v3 3/3] mm: kmemleak: check physical address when scan

Quoted 9 lines · Patrick Wang

I've received an auto build test WARNING from kernel test robot:
mm/kmemleak.c: In function 'scan_object':
Quoted 1 line
Quoted 1 line
arch/powerpc/include/asm/page.h:215:42: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
       215 | #define __va(x) ((void *)(unsigned long)((phys_addr_t)(x) + VIRT_PHYS_OFFSET))
           |                                          ^
     mm/kmemleak.c:1403:19: note: in expansion of macro '__va'
      1403 |                   __va((void *)object->pointer) :
           |                   ^~~~

Ah, yes, arm32 has the same issue with phys_addr_t defined as u64 in some configurations while long is 32-bit.

Quoted 2 lines

So I will replace __va((void *)object->pointer)
to __va((phys_addr_t)object->pointer) for fixing this warning,

It makes sense.

Signature · 1 line

Catalin