Fix a couple of issues with kmemleak that lead to missed (not reported)
leaks. Parts of the kernel keep references to allocated objects which
are not properly communicated to kmemleak. This causes kmemleak to fail
to report some memory leaks.
I have also written a kmemleak test. It is a userspace Python script
and a device driver that communicate over ioctl, allocate and free
memory and verify that memory leaks are reported correctly. You can find
it here:
https://gist.github.com/george-enf/2d7cc288202b94defbe253f4ca26357b
George Prekas (9):
mm: kmemleak: properly disable task stack scanning
Revert "mm/kmemleak: make create_object return void"
mm: kmemleak: propagate NO_SCAN flag in delete_object_part
mm: kmemleak: add kmemleak_noscan_phys function
mm: kmemleak: do not scan sparsemap_buf
mm: kmemleak: do not scan cpu_cache of struct kmem_cache
mm: kmemleak: erase page->s_mem in slab_destroy
mm: kmemleak: erase page->freelist in slab_destroy
mm: kmemleak: fix undetected leaks for page aligned objects
include/linux/kmemleak.h | 5 ++
kernel/fork.c | 3 ++
mm/kmemleak.c | 98 ++++++++++++++++++++++++++--------------
mm/slab.c | 20 +++-----
mm/slab.h | 7 ++-
mm/sparse.c | 3 ++
6 files changed, 88 insertions(+), 48 deletions(-)
--
2.37.1
kmemleak has a flag to enable or disable task stack scanning.
Unfortunately, this flag does not work as intended. Even when the user
disables stack scanning, kmemleak will scan them. Stacks are allocated
with vmalloc and are included in the list of objects to scan of
kmemleak.
Introduce a new function kmemleak_mark_stack that marks an allocation as
a stack. This allows kmemleak to properly decide whether to scan or not
the object based on the respective flag.
Signed-off-by: George Prekas <[email protected]>
---
include/linux/kmemleak.h | 1 +
kernel/fork.c | 3 +++
mm/kmemleak.c | 39 ++++++++++++++++++++++-----------------
3 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/include/linux/kmemleak.h b/include/linux/kmemleak.h
index 6a3cd1bf4680..1e2e8deac6dc 100644
--- a/include/linux/kmemleak.h
+++ b/include/linux/kmemleak.h
@@ -33,6 +33,7 @@ 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_ignore_phys(phys_addr_t phys) __ref;
+extern void kmemleak_mark_stack(const void *ptr) __ref;
static inline void kmemleak_alloc_recursive(const void *ptr, size_t size,
int min_count, slab_flags_t flags,
diff --git a/kernel/fork.c b/kernel/fork.c
index 9f7fe3541897..e66337ce88d4 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -97,6 +97,7 @@
#include <linux/io_uring.h>
#include <linux/bpf.h>
#include <linux/stackprotector.h>
+#include <linux/kmemleak.h>
#include <asm/pgalloc.h>
#include <linux/uaccess.h>
@@ -316,6 +317,8 @@ static int alloc_thread_stack_node(struct task_struct *tsk, int node)
if (!stack)
return -ENOMEM;
+ kmemleak_mark_stack(stack);
+
vm = find_vm_area(stack);
if (memcg_charge_kernel_stack(vm)) {
vfree(stack);
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 92f670edbf51..b40735539abd 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -176,6 +176,8 @@ struct kmemleak_object {
#define OBJECT_FULL_SCAN (1 << 3)
/* flag set for object allocated with physical address */
#define OBJECT_PHYS (1 << 4)
+/* flag set by alloc_thread_stack_node for stacks */
+#define OBJECT_STACK (1 << 5)
#define HEX_PREFIX " "
/* number of bytes to print per line; must be 16 or 32 */
@@ -1229,6 +1231,24 @@ void __ref kmemleak_ignore_phys(phys_addr_t phys)
}
EXPORT_SYMBOL(kmemleak_ignore_phys);
+/**
+ * kmemleak_mark_stack - mark the allocated object as a kernel stack
+ *
+ * @ptr: pointer to beginning of the object
+ */
+void __ref kmemleak_mark_stack(const void *ptr)
+{
+ struct kmemleak_object *object;
+
+ if (kmemleak_enabled && ptr && !IS_ERR(ptr)) {
+ object = find_and_get_object(ptr, 0);
+ if (object) {
+ object->flags |= OBJECT_STACK;
+ put_object(object);
+ }
+ }
+}
+
/*
* Update an object's checksum and return true if it was modified.
*/
@@ -1404,6 +1424,8 @@ static void scan_object(struct kmemleak_object *object)
if (!(object->flags & OBJECT_ALLOCATED))
/* already freed object */
goto out;
+ if (!kmemleak_stack_scan && object->flags & OBJECT_STACK)
+ goto out;
obj_ptr = object->flags & OBJECT_PHYS ?
__va((phys_addr_t)object->pointer) :
@@ -1586,23 +1608,6 @@ static void kmemleak_scan(void)
}
put_online_mems();
- /*
- * Scanning the task stacks (may introduce false negatives).
- */
- if (kmemleak_stack_scan) {
- struct task_struct *p, *g;
-
- rcu_read_lock();
- for_each_process_thread(g, p) {
- void *stack = try_get_task_stack(p);
- if (stack) {
- scan_block(stack, stack + THREAD_SIZE, NULL);
- put_task_stack(p);
- }
- }
- rcu_read_unlock();
- }
-
/*
* Scan the objects already referenced from the sections scanned
* above.
--
2.37.1
This reverts commit b955aa70a3ac9f1dd5e26d4c7673ec3c28a8c2af.
The function delete_object_part can use the return value of
create_object to propagate flags from the old object to the new objects.
Signed-off-by: George Prekas <[email protected]>
---
mm/kmemleak.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index b40735539abd..2b9c9ad68806 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -616,8 +616,9 @@ static noinline depot_stack_handle_t set_track_prepare(void)
* memory block and add it to the object_list and object_tree_root (or
* object_phys_tree_root).
*/
-static void __create_object(unsigned long ptr, size_t size,
- int min_count, gfp_t gfp, bool is_phys)
+static struct kmemleak_object *__create_object(unsigned long ptr, size_t size,
+ int min_count, gfp_t gfp,
+ bool is_phys)
{
unsigned long flags;
struct kmemleak_object *object, *parent;
@@ -629,7 +630,7 @@ static void __create_object(unsigned long ptr, size_t size,
if (!object) {
pr_warn("Cannot allocate a kmemleak_object structure\n");
kmemleak_disable();
- return;
+ return NULL;
}
INIT_LIST_HEAD(&object->object_list);
@@ -698,6 +699,7 @@ static void __create_object(unsigned long ptr, size_t size,
*/
dump_object_info(parent);
kmem_cache_free(object_cache, object);
+ object = NULL;
goto out;
}
}
@@ -707,20 +709,21 @@ static void __create_object(unsigned long ptr, size_t size,
list_add_tail_rcu(&object->object_list, &object_list);
out:
raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
+ return object;
}
/* Create kmemleak object which allocated with virtual address. */
-static void create_object(unsigned long ptr, size_t size,
- int min_count, gfp_t gfp)
+static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
+ int min_count, gfp_t gfp)
{
- __create_object(ptr, size, min_count, gfp, false);
+ return __create_object(ptr, size, min_count, gfp, false);
}
/* Create kmemleak object which allocated with physical address. */
-static void create_object_phys(unsigned long ptr, size_t size,
- int min_count, gfp_t gfp)
+static struct kmemleak_object *create_object_phys(unsigned long ptr, size_t size,
+ int min_count, gfp_t gfp)
{
- __create_object(ptr, size, min_count, gfp, true);
+ return __create_object(ptr, size, min_count, gfp, true);
}
/*
--
2.37.1
When part of an object is deleted, propagate the OBJECT_NO_SCAN flag to
the 2 new created objects.
Signed-off-by: George Prekas <[email protected]>
---
mm/kmemleak.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 2b9c9ad68806..5882f60d127c 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -772,7 +772,7 @@ static void delete_object_full(unsigned long ptr)
*/
static void delete_object_part(unsigned long ptr, size_t size, bool is_phys)
{
- struct kmemleak_object *object;
+ struct kmemleak_object *object, *o1 = NULL, *o2 = NULL;
unsigned long start, end;
object = find_and_remove_object(ptr, 1, is_phys);
@@ -792,11 +792,19 @@ static void delete_object_part(unsigned long ptr, size_t size, bool is_phys)
start = object->pointer;
end = object->pointer + object->size;
if (ptr > start)
- __create_object(start, ptr - start, object->min_count,
- GFP_KERNEL, is_phys);
+ o1 = __create_object(start, ptr - start, object->min_count,
+ GFP_KERNEL, is_phys);
if (ptr + size < end)
- __create_object(ptr + size, end - ptr - size, object->min_count,
- GFP_KERNEL, is_phys);
+ o2 = __create_object(ptr + size, end - ptr - size,
+ object->min_count, GFP_KERNEL, is_phys);
+
+ /* Propagate the NO_SCAN flag */
+ if (object->flags & OBJECT_NO_SCAN) {
+ if (o1)
+ o1->flags |= OBJECT_NO_SCAN;
+ if (o2)
+ o2->flags |= OBJECT_NO_SCAN;
+ }
__delete_object(object);
}
--
2.37.1
This function should be used instead of kmemleak_noscan when the object
has been registered with kmemleak_alloc_phys.
Signed-off-by: George Prekas <[email protected]>
---
include/linux/kmemleak.h | 4 ++++
mm/kmemleak.c | 20 +++++++++++++++++---
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/include/linux/kmemleak.h b/include/linux/kmemleak.h
index 1e2e8deac6dc..42069e0c1ac8 100644
--- a/include/linux/kmemleak.h
+++ b/include/linux/kmemleak.h
@@ -29,6 +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_no_scan_phys(phys_addr_t phys) __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;
@@ -106,6 +107,9 @@ static inline void kmemleak_erase(void **ptr)
static inline void kmemleak_no_scan(const void *ptr)
{
}
+static inline void kmemleak_no_scan_phys(phys_addr_t phys)
+{
+}
static inline void kmemleak_alloc_phys(phys_addr_t phys, size_t size,
gfp_t gfp)
{
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 5882f60d127c..6e037dcf322f 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -939,12 +939,12 @@ static void object_set_excess_ref(unsigned long ptr, unsigned long excess_ref)
* pointer. Such object will not be scanned by kmemleak but references to it
* are searched.
*/
-static void object_no_scan(unsigned long ptr)
+static void object_no_scan(unsigned long ptr, bool is_phys)
{
unsigned long flags;
struct kmemleak_object *object;
- object = find_and_get_object(ptr, 0);
+ object = __find_and_get_object(ptr, 0, is_phys);
if (!object) {
kmemleak_warn("Not scanning unknown object at 0x%08lx\n", ptr);
return;
@@ -1188,10 +1188,24 @@ void __ref kmemleak_no_scan(const void *ptr)
pr_debug("%s(0x%p)\n", __func__, ptr);
if (kmemleak_enabled && ptr && !IS_ERR(ptr))
- object_no_scan((unsigned long)ptr);
+ object_no_scan((unsigned long)ptr, false);
}
EXPORT_SYMBOL(kmemleak_no_scan);
+/**
+ * kmemleak_no_scan_phys - similar to kmemleak_no_scan but taking a physical
+ * address argument
+ * @phys: physical address of the object
+ */
+void __ref kmemleak_no_scan_phys(phys_addr_t phys)
+{
+ pr_debug("%s(0x%pa)\n", __func__, &phys);
+
+ if (kmemleak_enabled && phys)
+ object_no_scan((unsigned long)phys, true);
+}
+EXPORT_SYMBOL(kmemleak_no_scan_phys);
+
/**
* kmemleak_alloc_phys - similar to kmemleak_alloc but taking a physical
* address argument
--
2.37.1
sparsemap_buf is used for vmemmap and vmemmap contains an array of
struct page. kmemleak scans the pages separately. If it scans them
twice, then it will duplicate the number of found references and will
cause missed leaks.
Signed-off-by: George Prekas <[email protected]>
---
mm/sparse.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/mm/sparse.c b/mm/sparse.c
index 2779b419ef2a..49df7a052037 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -14,6 +14,7 @@
#include <linux/swap.h>
#include <linux/swapops.h>
#include <linux/bootmem_info.h>
+#include <linux/kmemleak.h>
#include "internal.h"
#include <asm/dma.h>
@@ -466,6 +467,8 @@ static void __init sparse_buffer_init(unsigned long size, int nid)
*/
sparsemap_buf = memmap_alloc(size, section_map_size(), addr, nid, true);
sparsemap_buf_end = sparsemap_buf + size;
+ /* sparsemap_buf has been registered with its physical address in kmemleak */
+ kmemleak_no_scan_phys(virt_to_phys(sparsemap_buf));
}
static void __init sparse_buffer_fini(void)
--
2.37.1
The code already makes sure that kmemleak will not scan similar caches:
array_cache and alien_cache. For the cpu_cache, the code takes a
different approach using kmemleak_erase. This approach handles object
allocations but does not handle transfers to other caches and leads to
undetected leaks.
According to the comment in alloc_arraycache: [...] when such objects
are allocated or transferred to another cache the pointers are not
cleared and they could be counted as valid references during a kmemleak
scan. Therefore, kmemleak must not scan such objects.
Signed-off-by: George Prekas <[email protected]>
---
mm/slab.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/mm/slab.c b/mm/slab.c
index 29300fc1289a..a927e1a285d1 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1744,6 +1744,7 @@ static struct array_cache __percpu *alloc_kmem_cache_cpus(
for_each_possible_cpu(cpu) {
init_arraycache(per_cpu_ptr(cpu_cache, cpu),
entries, batchcount);
+ kmemleak_no_scan(per_cpu_ptr(cpu_cache, cpu));
}
return cpu_cache;
@@ -3023,20 +3024,8 @@ static inline void *____cache_alloc(struct kmem_cache *cachep, gfp_t flags)
STATS_INC_ALLOCMISS(cachep);
objp = cache_alloc_refill(cachep, flags);
- /*
- * the 'ac' may be updated by cache_alloc_refill(),
- * and kmemleak_erase() requires its correct value.
- */
- ac = cpu_cache_get(cachep);
out:
- /*
- * To avoid a false negative, if an object that is in one of the
- * per-CPU caches is leaked, we need to make sure kmemleak doesn't
- * treat the array pointers as a reference to the object.
- */
- if (objp)
- kmemleak_erase(&ac->entry[ac->avail]);
return objp;
}
--
2.37.1
The field s_mem of struct page is initialized with the virtual address
of the page in function alloc_slabmgmt. If kmalloc allocates an object
that starts on this page, then kmemleak knows that this object has 2
references. On slab_destroy, s_mem should not continue referring to any
allocated object in the future.
Specifically, assume that initially the 4KB cache uses page[5] and its
s_mem = 0x5000. Then assume that this cache releases page[5] and the 8KB
cache allocates page[4] and page[5]. Subsequently, kmalloc returns an
8KB object at address 0x4000 which will have 3 references: the returned
pointer from kmalloc, page[4].s_mem = 0x4000, and page[5].s_mem. This
object can leak without detection.
Signed-off-by: George Prekas <[email protected]>
---
mm/slab.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/mm/slab.c b/mm/slab.c
index a927e1a285d1..aa5eb725ee9c 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1611,6 +1611,9 @@ static void slab_destroy(struct kmem_cache *cachep, struct slab *slab)
{
void *freelist;
+ /* Erase the page's virtual address from s_mem */
+ kmemleak_erase(&slab->s_mem);
+
freelist = slab->freelist;
slab_destroy_debugcheck(cachep, slab);
if (unlikely(cachep->flags & SLAB_TYPESAFE_BY_RCU))
--
2.37.1
slab_destroy frees the allocated memory pointed by page->freelist. It
should also erase the reference to it. Otherwise, when the memory is
reused for another object, kmemleak will find the stale pointer and
erroneously increase the reference count of the new object. That will
lead to undetected memory leaks.
Signed-off-by: George Prekas <[email protected]>
---
mm/slab.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/mm/slab.c b/mm/slab.c
index aa5eb725ee9c..12acce274502 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1625,8 +1625,10 @@ static void slab_destroy(struct kmem_cache *cachep, struct slab *slab)
* From now on, we don't use freelist
* although actual page can be freed in rcu context
*/
- if (OFF_SLAB(cachep))
+ if (OFF_SLAB(cachep)) {
kfree(freelist);
+ kmemleak_erase(&slab->freelist);
+ }
}
/*
--
2.37.1
If kmalloc returns a page aligned object, then the object has 2
references: the pointer returned by kmalloc and page->s_mem of the first
page of the object. Account for this extra reference, so that kmemleak
can correctly detect leaks for page aligned objects.
Signed-off-by: George Prekas <[email protected]>
---
mm/slab.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/mm/slab.h b/mm/slab.h
index 7cc432969945..76341c7c048e 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -734,6 +734,7 @@ static inline void slab_post_alloc_hook(struct kmem_cache *s,
unsigned int orig_size)
{
unsigned int zero_size = s->object_size;
+ int min_count;
size_t i;
flags &= gfp_allowed_mask;
@@ -761,7 +762,11 @@ static inline void slab_post_alloc_hook(struct kmem_cache *s,
p[i] = kasan_slab_alloc(s, p[i], flags, init);
if (p[i] && init && !kasan_has_integrated_init())
memset(p[i], 0, zero_size);
- kmemleak_alloc_recursive(p[i], s->object_size, 1,
+ min_count = 1;
+ /* If p[i] is page aligned, then a page->s_mem refers to it. */
+ if (((uintptr_t)p[i] & ~PAGE_MASK) == 0)
+ min_count++;
+ kmemleak_alloc_recursive(p[i], s->object_size, min_count,
s->flags, flags);
kmsan_slab_alloc(s, p[i], flags);
}
--
2.37.1
Hi George,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on vbabka-slab/for-next]
[also build test ERROR on linus/master v6.2-rc5 next-20230123]
[cannot apply to akpm-mm/mm-everything]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/George-Prekas/mm-kmemleak-properly-disable-task-stack-scanning/20230124-010911
base: git://git.kernel.org/pub/scm/linux/kernel/git/vbabka/slab.git for-next
patch link: https://lore.kernel.org/r/20230123170419.7292-2-george%40enfabrica.net
patch subject: [PATCH 1/9] mm: kmemleak: properly disable task stack scanning
config: s390-defconfig (https://download.01.org/0day-ci/archive/20230124/[email protected]/config)
compiler: s390-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/f0d9df4305849ecea4402bc614cadb0dd357da77
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review George-Prekas/mm-kmemleak-properly-disable-task-stack-scanning/20230124-010911
git checkout f0d9df4305849ecea4402bc614cadb0dd357da77
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
kernel/fork.c: In function 'alloc_thread_stack_node':
>> kernel/fork.c:320:9: error: implicit declaration of function 'kmemleak_mark_stack'; did you mean 'kmemleak_no_scan'? [-Werror=implicit-function-declaration]
320 | kmemleak_mark_stack(stack);
| ^~~~~~~~~~~~~~~~~~~
| kmemleak_no_scan
kernel/fork.c: At top level:
kernel/fork.c:865:20: warning: no previous prototype for 'arch_task_cache_init' [-Wmissing-prototypes]
865 | void __init __weak arch_task_cache_init(void) { }
| ^~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +320 kernel/fork.c
274
275 static int alloc_thread_stack_node(struct task_struct *tsk, int node)
276 {
277 struct vm_struct *vm;
278 void *stack;
279 int i;
280
281 for (i = 0; i < NR_CACHED_STACKS; i++) {
282 struct vm_struct *s;
283
284 s = this_cpu_xchg(cached_stacks[i], NULL);
285
286 if (!s)
287 continue;
288
289 /* Reset stack metadata. */
290 kasan_unpoison_range(s->addr, THREAD_SIZE);
291
292 stack = kasan_reset_tag(s->addr);
293
294 /* Clear stale pointers from reused stack. */
295 memset(stack, 0, THREAD_SIZE);
296
297 if (memcg_charge_kernel_stack(s)) {
298 vfree(s->addr);
299 return -ENOMEM;
300 }
301
302 tsk->stack_vm_area = s;
303 tsk->stack = stack;
304 return 0;
305 }
306
307 /*
308 * Allocated stacks are cached and later reused by new threads,
309 * so memcg accounting is performed manually on assigning/releasing
310 * stacks to tasks. Drop __GFP_ACCOUNT.
311 */
312 stack = __vmalloc_node_range(THREAD_SIZE, THREAD_ALIGN,
313 VMALLOC_START, VMALLOC_END,
314 THREADINFO_GFP & ~__GFP_ACCOUNT,
315 PAGE_KERNEL,
316 0, node, __builtin_return_address(0));
317 if (!stack)
318 return -ENOMEM;
319
> 320 kmemleak_mark_stack(stack);
321
322 vm = find_vm_area(stack);
323 if (memcg_charge_kernel_stack(vm)) {
324 vfree(stack);
325 return -ENOMEM;
326 }
327 /*
328 * We can't call find_vm_area() in interrupt context, and
329 * free_thread_stack() can be called in interrupt context,
330 * so cache the vm_struct.
331 */
332 tsk->stack_vm_area = vm;
333 stack = kasan_reset_tag(stack);
334 tsk->stack = stack;
335 return 0;
336 }
337
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Hi George,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on vbabka-slab/for-next]
[also build test WARNING on linus/master v6.2-rc5 next-20230123]
[cannot apply to akpm-mm/mm-everything]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/George-Prekas/mm-kmemleak-properly-disable-task-stack-scanning/20230124-010911
base: git://git.kernel.org/pub/scm/linux/kernel/git/vbabka/slab.git for-next
patch link: https://lore.kernel.org/r/20230123170419.7292-2-george%40enfabrica.net
patch subject: [PATCH 1/9] mm: kmemleak: properly disable task stack scanning
config: arc-randconfig-r043-20230123 (https://download.01.org/0day-ci/archive/20230124/[email protected]/config)
compiler: arceb-elf-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/f0d9df4305849ecea4402bc614cadb0dd357da77
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review George-Prekas/mm-kmemleak-properly-disable-task-stack-scanning/20230124-010911
git checkout f0d9df4305849ecea4402bc614cadb0dd357da77
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
mm/kmemleak.c: In function 'kmemleak_mark_stack':
>> mm/kmemleak.c:1244:46: warning: passing argument 1 of 'find_and_get_object' makes integer from pointer without a cast [-Wint-conversion]
1244 | object = find_and_get_object(ptr, 0);
| ^~~
| |
| const void *
mm/kmemleak.c:561:66: note: expected 'long unsigned int' but argument is of type 'const void *'
561 | static struct kmemleak_object *find_and_get_object(unsigned long ptr, int alias)
| ~~~~~~~~~~~~~~^~~
vim +/find_and_get_object +1244 mm/kmemleak.c
1233
1234 /**
1235 * kmemleak_mark_stack - mark the allocated object as a kernel stack
1236 *
1237 * @ptr: pointer to beginning of the object
1238 */
1239 void __ref kmemleak_mark_stack(const void *ptr)
1240 {
1241 struct kmemleak_object *object;
1242
1243 if (kmemleak_enabled && ptr && !IS_ERR(ptr)) {
> 1244 object = find_and_get_object(ptr, 0);
1245 if (object) {
1246 object->flags |= OBJECT_STACK;
1247 put_object(object);
1248 }
1249 }
1250 }
1251
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Hi George,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on vbabka-slab/for-next]
[also build test ERROR on linus/master v6.2-rc5]
[cannot apply to akpm-mm/mm-everything]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/George-Prekas/mm-kmemleak-properly-disable-task-stack-scanning/20230124-010911
base: git://git.kernel.org/pub/scm/linux/kernel/git/vbabka/slab.git for-next
patch link: https://lore.kernel.org/r/20230123170419.7292-2-george%40enfabrica.net
patch subject: [PATCH 1/9] mm: kmemleak: properly disable task stack scanning
config: riscv-randconfig-r042-20230123 (https://download.01.org/0day-ci/archive/20230124/[email protected]/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 4196ca3278f78c6e19246e54ab0ecb364e37d66a)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/f0d9df4305849ecea4402bc614cadb0dd357da77
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review George-Prekas/mm-kmemleak-properly-disable-task-stack-scanning/20230124-010911
git checkout f0d9df4305849ecea4402bc614cadb0dd357da77
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
kernel/fork.c:163:13: warning: no previous prototype for function 'arch_release_task_struct' [-Wmissing-prototypes]
void __weak arch_release_task_struct(struct task_struct *tsk)
^
kernel/fork.c:163:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void __weak arch_release_task_struct(struct task_struct *tsk)
^
static
>> kernel/fork.c:320:2: error: call to undeclared function 'kmemleak_mark_stack'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
kmemleak_mark_stack(stack);
^
kernel/fork.c:865:20: warning: no previous prototype for function 'arch_task_cache_init' [-Wmissing-prototypes]
void __init __weak arch_task_cache_init(void) { }
^
kernel/fork.c:865:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void __init __weak arch_task_cache_init(void) { }
^
static
2 warnings and 1 error generated.
vim +/kmemleak_mark_stack +320 kernel/fork.c
274
275 static int alloc_thread_stack_node(struct task_struct *tsk, int node)
276 {
277 struct vm_struct *vm;
278 void *stack;
279 int i;
280
281 for (i = 0; i < NR_CACHED_STACKS; i++) {
282 struct vm_struct *s;
283
284 s = this_cpu_xchg(cached_stacks[i], NULL);
285
286 if (!s)
287 continue;
288
289 /* Reset stack metadata. */
290 kasan_unpoison_range(s->addr, THREAD_SIZE);
291
292 stack = kasan_reset_tag(s->addr);
293
294 /* Clear stale pointers from reused stack. */
295 memset(stack, 0, THREAD_SIZE);
296
297 if (memcg_charge_kernel_stack(s)) {
298 vfree(s->addr);
299 return -ENOMEM;
300 }
301
302 tsk->stack_vm_area = s;
303 tsk->stack = stack;
304 return 0;
305 }
306
307 /*
308 * Allocated stacks are cached and later reused by new threads,
309 * so memcg accounting is performed manually on assigning/releasing
310 * stacks to tasks. Drop __GFP_ACCOUNT.
311 */
312 stack = __vmalloc_node_range(THREAD_SIZE, THREAD_ALIGN,
313 VMALLOC_START, VMALLOC_END,
314 THREADINFO_GFP & ~__GFP_ACCOUNT,
315 PAGE_KERNEL,
316 0, node, __builtin_return_address(0));
317 if (!stack)
318 return -ENOMEM;
319
> 320 kmemleak_mark_stack(stack);
321
322 vm = find_vm_area(stack);
323 if (memcg_charge_kernel_stack(vm)) {
324 vfree(stack);
325 return -ENOMEM;
326 }
327 /*
328 * We can't call find_vm_area() in interrupt context, and
329 * free_thread_stack() can be called in interrupt context,
330 * so cache the vm_struct.
331 */
332 tsk->stack_vm_area = vm;
333 stack = kasan_reset_tag(stack);
334 tsk->stack = stack;
335 return 0;
336 }
337
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Hi George,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on vbabka-slab/for-next]
[also build test ERROR on linus/master v6.2-rc5 next-20230124]
[cannot apply to akpm-mm/mm-everything]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/George-Prekas/mm-kmemleak-properly-disable-task-stack-scanning/20230124-010911
base: git://git.kernel.org/pub/scm/linux/kernel/git/vbabka/slab.git for-next
patch link: https://lore.kernel.org/r/20230123170419.7292-2-george%40enfabrica.net
patch subject: [PATCH 1/9] mm: kmemleak: properly disable task stack scanning
config: s390-randconfig-r044-20230123 (https://download.01.org/0day-ci/archive/20230124/[email protected]/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 4196ca3278f78c6e19246e54ab0ecb364e37d66a)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/f0d9df4305849ecea4402bc614cadb0dd357da77
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review George-Prekas/mm-kmemleak-properly-disable-task-stack-scanning/20230124-010911
git checkout f0d9df4305849ecea4402bc614cadb0dd357da77
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
In file included from mm/kmemleak.c:85:
In file included from include/linux/memblock.h:13:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
^
include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
#define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
^
In file included from mm/kmemleak.c:85:
In file included from include/linux/memblock.h:13:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
^
In file included from mm/kmemleak.c:85:
In file included from include/linux/memblock.h:13:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> mm/kmemleak.c:1244:32: error: incompatible pointer to integer conversion passing 'const void *' to parameter of type 'unsigned long' [-Wint-conversion]
object = find_and_get_object(ptr, 0);
^~~
mm/kmemleak.c:561:66: note: passing argument to parameter 'ptr' here
static struct kmemleak_object *find_and_get_object(unsigned long ptr, int alias)
^
12 warnings and 1 error generated.
vim +1244 mm/kmemleak.c
1233
1234 /**
1235 * kmemleak_mark_stack - mark the allocated object as a kernel stack
1236 *
1237 * @ptr: pointer to beginning of the object
1238 */
1239 void __ref kmemleak_mark_stack(const void *ptr)
1240 {
1241 struct kmemleak_object *object;
1242
1243 if (kmemleak_enabled && ptr && !IS_ERR(ptr)) {
> 1244 object = find_and_get_object(ptr, 0);
1245 if (object) {
1246 object->flags |= OBJECT_STACK;
1247 put_object(object);
1248 }
1249 }
1250 }
1251
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
On Mon, Jan 23, 2023 at 11:04:19AM -0600, George Prekas wrote:
> If kmalloc returns a page aligned object, then the object has 2
> references: the pointer returned by kmalloc and page->s_mem of the first
> page of the object. Account for this extra reference, so that kmemleak
> can correctly detect leaks for page aligned objects.
Is this true for multi-page slabs? Imagine if you allocate objects of
size 4kB. Slab/Slub will allocate slabs of size 32kB and try to
allocate 8 objects from each slab. Only the first one will have an
s_mem pointing into it. No?
> - kmemleak_alloc_recursive(p[i], s->object_size, 1,
> + min_count = 1;
> + /* If p[i] is page aligned, then a page->s_mem refers to it. */
Also this comment should read 'slab->s_mem'. Individual pages within
the slab do not have s_mem pointers, only the slab.
> + if (((uintptr_t)p[i] & ~PAGE_MASK) == 0)
> + min_count++;
> + kmemleak_alloc_recursive(p[i], s->object_size, min_count,
> s->flags, flags);
On Mon, Jan 23, 2023 at 11:04:11AM -0600, George Prekas wrote:
> @@ -1586,23 +1608,6 @@ static void kmemleak_scan(void)
> }
> put_online_mems();
>
> - /*
> - * Scanning the task stacks (may introduce false negatives).
> - */
> - if (kmemleak_stack_scan) {
> - struct task_struct *p, *g;
> -
> - rcu_read_lock();
> - for_each_process_thread(g, p) {
> - void *stack = try_get_task_stack(p);
> - if (stack) {
> - scan_block(stack, stack + THREAD_SIZE, NULL);
> - put_task_stack(p);
> - }
> - }
> - rcu_read_unlock();
> - }
On architectures without VMAP_STACK, we still need the above otherwise
it could lead to some false positives.
--
Catalin
On Mon, 23 Jan 2023, George Prekas wrote:
> If kmalloc returns a page aligned object, then the object has 2
> references: the pointer returned by kmalloc and page->s_mem of the first
> page of the object. Account for this extra reference, so that kmemleak
> can correctly detect leaks for page aligned objects.
s_mem is a reference to the array of slab objects in a SLAB page. It is
not referring to a particular object. s_mem allows access to the Nth
object in a slab page.
See the function index_to_obj() in slab.c
static inline void *index_to_obj(struct kmem_cache *cache,
const struct slab *slab, unsigned int idx)
{
return slab->s_mem + cache->size * idx;
}
On Mon, 23 Jan 2023, George Prekas wrote:
> The field s_mem of struct page is initialized with the virtual address
> of the page in function alloc_slabmgmt. If kmalloc allocates an object
> that starts on this page, then kmemleak knows that this object has 2
> references. On slab_destroy, s_mem should not continue referring to any
> allocated object in the future.
Nope.
s_mem is a pointer to an array of objects. It is not a pointer to a page.
If a slab-caches is used for slabmanagement then these objects
contain slab metadata which may be a bit confusing.
Hi George,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on vbabka-slab/for-next]
[also build test WARNING on linus/master v6.2-rc6]
[cannot apply to akpm-mm/mm-everything]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/George-Prekas/mm-kmemleak-properly-disable-task-stack-scanning/20230124-010911
base: git://git.kernel.org/pub/scm/linux/kernel/git/vbabka/slab.git for-next
patch link: https://lore.kernel.org/r/20230123170419.7292-2-george%40enfabrica.net
patch subject: [PATCH 1/9] mm: kmemleak: properly disable task stack scanning
config: s390-randconfig-s051-20230129 (https://download.01.org/0day-ci/archive/20230201/[email protected]/config)
compiler: s390-linux-gcc (GCC) 12.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/f0d9df4305849ecea4402bc614cadb0dd357da77
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review George-Prekas/mm-kmemleak-properly-disable-task-stack-scanning/20230124-010911
git checkout f0d9df4305849ecea4402bc614cadb0dd357da77
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=s390 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=s390 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
sparse warnings: (new ones prefixed by >>)
>> mm/kmemleak.c:1244:46: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected unsigned long ptr @@ got void const *ptr @@
mm/kmemleak.c:1244:46: sparse: expected unsigned long ptr
mm/kmemleak.c:1244:46: sparse: got void const *ptr
mm/kmemleak.c:1772:13: sparse: sparse: context imbalance in 'kmemleak_seq_start' - different lock contexts for basic block
mm/kmemleak.c: note: in included file (through include/linux/rculist.h, include/linux/sched/signal.h):
include/linux/rcupdate.h:795:9: sparse: sparse: context imbalance in 'kmemleak_seq_stop' - unexpected unlock
vim +1244 mm/kmemleak.c
1233
1234 /**
1235 * kmemleak_mark_stack - mark the allocated object as a kernel stack
1236 *
1237 * @ptr: pointer to beginning of the object
1238 */
1239 void __ref kmemleak_mark_stack(const void *ptr)
1240 {
1241 struct kmemleak_object *object;
1242
1243 if (kmemleak_enabled && ptr && !IS_ERR(ptr)) {
> 1244 object = find_and_get_object(ptr, 0);
1245 if (object) {
1246 object->flags |= OBJECT_STACK;
1247 put_object(object);
1248 }
1249 }
1250 }
1251
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests