Skip to content

Commit 7591c12

Browse files
ctmarinasakpm00
authored andcommitted
kmemleak: iommu/iova: fix transient kmemleak false positive
The introduction of iova_depot_pop() in 911aa12 ("iommu/iova: Make the rcache depot scale better") confused kmemleak by moving a struct iova_magazine object from a singly linked list to rcache->depot and resetting the 'next' pointer referencing it. Unlike doubly linked lists, the content of the object being referred is never changed on removal from a singly linked list and the kmemleak checksum heuristics do not detect such scenario. This leads to false positives like: unreferenced object 0xffff8881a5301000 (size 1024): comm "softirq", pid 0, jiffies 4306297099 (age 462.991s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 e7 7d 05 00 00 00 00 00 .........}...... 0f b4 05 00 00 00 00 00 b4 96 05 00 00 00 00 00 ................ backtrace: [<ffffffff819f5f08>] __kmem_cache_alloc_node+0x1e8/0x320 [<ffffffff818a239a>] kmalloc_trace+0x2a/0x60 [<ffffffff8231d31e>] free_iova_fast+0x28e/0x4e0 [<ffffffff82310860>] fq_ring_free_locked+0x1b0/0x310 [<ffffffff8231225d>] fq_flush_timeout+0x19d/0x2e0 [<ffffffff813e95ba>] call_timer_fn+0x19a/0x5c0 [<ffffffff813ea16b>] __run_timers+0x78b/0xb80 [<ffffffff813ea5bd>] run_timer_softirq+0x5d/0xd0 [<ffffffff82f1d915>] __do_softirq+0x205/0x8b5 Introduce kmemleak_transient_leak() which resets the object checksum requiring another scan pass before it is reported (if still unreferenced). Call this new API in iova_depot_pop(). Link: https://lkml.kernel.org/r/[email protected] Link: https://lore.kernel.org/r/ZY1osaGLyT-sdKE8@shredder/ Signed-off-by: Catalin Marinas <[email protected]> Reported-by: Ido Schimmel <[email protected]> Tested-by: Ido Schimmel <[email protected]> Acked-by: Robin Murphy <[email protected]> Cc: Joerg Roedel <[email protected]> Cc: Will Deacon <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
1 parent da0c025 commit 7591c12

File tree

4 files changed

+50
-0
lines changed

4 files changed

+50
-0
lines changed

Documentation/dev-tools/kmemleak.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ See the include/linux/kmemleak.h header for the functions prototype.
161161
- ``kmemleak_free_percpu`` - notify of a percpu memory block freeing
162162
- ``kmemleak_update_trace`` - update object allocation stack trace
163163
- ``kmemleak_not_leak`` - mark an object as not a leak
164+
- ``kmemleak_transient_leak`` - mark an object as a transient leak
164165
- ``kmemleak_ignore`` - do not scan or report an object as leak
165166
- ``kmemleak_scan_area`` - add scan areas inside a memory block
166167
- ``kmemleak_no_scan`` - do not scan a memory block

drivers/iommu/iova.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77

88
#include <linux/iova.h>
9+
#include <linux/kmemleak.h>
910
#include <linux/module.h>
1011
#include <linux/slab.h>
1112
#include <linux/smp.h>
@@ -673,6 +674,11 @@ static struct iova_magazine *iova_depot_pop(struct iova_rcache *rcache)
673674
{
674675
struct iova_magazine *mag = rcache->depot;
675676

677+
/*
678+
* As the mag->next pointer is moved to rcache->depot and reset via
679+
* the mag->size assignment, mark it as a transient false positive.
680+
*/
681+
kmemleak_transient_leak(mag->next);
676682
rcache->depot = mag->next;
677683
mag->size = IOVA_MAG_SIZE;
678684
rcache->depot_size--;

include/linux/kmemleak.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ extern void kmemleak_free_part(const void *ptr, size_t size) __ref;
2626
extern void kmemleak_free_percpu(const void __percpu *ptr) __ref;
2727
extern void kmemleak_update_trace(const void *ptr) __ref;
2828
extern void kmemleak_not_leak(const void *ptr) __ref;
29+
extern void kmemleak_transient_leak(const void *ptr) __ref;
2930
extern void kmemleak_ignore(const void *ptr) __ref;
3031
extern void kmemleak_scan_area(const void *ptr, size_t size, gfp_t gfp) __ref;
3132
extern void kmemleak_no_scan(const void *ptr) __ref;
@@ -93,6 +94,9 @@ static inline void kmemleak_update_trace(const void *ptr)
9394
static inline void kmemleak_not_leak(const void *ptr)
9495
{
9596
}
97+
static inline void kmemleak_transient_leak(const void *ptr)
98+
{
99+
}
96100
static inline void kmemleak_ignore(const void *ptr)
97101
{
98102
}

mm/kmemleak.c

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -934,6 +934,28 @@ static void make_black_object(unsigned long ptr, unsigned int objflags)
934934
paint_ptr(ptr, KMEMLEAK_BLACK, objflags);
935935
}
936936

937+
/*
938+
* Reset the checksum of an object. The immediate effect is that it will not
939+
* be reported as a leak during the next scan until its checksum is updated.
940+
*/
941+
static void reset_checksum(unsigned long ptr)
942+
{
943+
unsigned long flags;
944+
struct kmemleak_object *object;
945+
946+
object = find_and_get_object(ptr, 0);
947+
if (!object) {
948+
kmemleak_warn("Not resetting the checksum of an unknown object at 0x%08lx\n",
949+
ptr);
950+
return;
951+
}
952+
953+
raw_spin_lock_irqsave(&object->lock, flags);
954+
object->checksum = 0;
955+
raw_spin_unlock_irqrestore(&object->lock, flags);
956+
put_object(object);
957+
}
958+
937959
/*
938960
* Add a scanning area to the object. If at least one such area is added,
939961
* kmemleak will only scan these ranges rather than the whole memory block.
@@ -1202,6 +1224,23 @@ void __ref kmemleak_not_leak(const void *ptr)
12021224
}
12031225
EXPORT_SYMBOL(kmemleak_not_leak);
12041226

1227+
/**
1228+
* kmemleak_transient_leak - mark an allocated object as transient false positive
1229+
* @ptr: pointer to beginning of the object
1230+
*
1231+
* Calling this function on an object will cause the memory block to not be
1232+
* reported as a leak temporarily. This may happen, for example, if the object
1233+
* is part of a singly linked list and the ->next reference to it is changed.
1234+
*/
1235+
void __ref kmemleak_transient_leak(const void *ptr)
1236+
{
1237+
pr_debug("%s(0x%px)\n", __func__, ptr);
1238+
1239+
if (kmemleak_enabled && ptr && !IS_ERR(ptr))
1240+
reset_checksum((unsigned long)ptr);
1241+
}
1242+
EXPORT_SYMBOL(kmemleak_transient_leak);
1243+
12051244
/**
12061245
* kmemleak_ignore - ignore an allocated object
12071246
* @ptr: pointer to beginning of the object

0 commit comments

Comments
 (0)