Re: Crash in crc32_le during kmemleak_scan()

[Catalin Marinas <catalin.marinas@arm.com>]

On Wed, May 13, 2015 at 02:15:08PM +0100, vigneshr@codeaurora.org wrote:
> We are seeing a panic in crc32_le after kmemleak_scan(). I have pasted the
> snippet of the crash log below. This is seen on 3.10 Kernel.
> 
> This issue was earlier discussed over this thread as well -
> https://lkml.org/lkml/2014/2/6/287.

Thanks for raising this again and the in depth analysis.

> We tried the following as well to stop the scan thread first and then
> schedule work but it did not help :
> 
> 
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index 0bd522f..6105dfe 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -1460,7 +1460,6 @@ static void start_scan_thread(void)
> static void stop_scan_thread(void)
> {
>         if (scan_thread) {
> -               kthread_stop(scan_thread);
>                 scan_thread = NULL;
>         }
> }
> @@ -1747,6 +1746,9 @@ static void kmemleak_disable(void)
>         /* stop any memory operation tracing */
>         atomic_set(&kmemleak_enabled, 0);
> 
> +       if(scan_thread)
> +               kthread_stop(scan_thread);
> +
>         /* check whether it is too early for a kernel thread */

kmemleak_disable() may be called in an atomic context, so calling
kthread_stop() here is not safe. We have a scan_should_stop() function
which checks for the kmemleak_enabled variable but it doesn't seem to be
enough.

Basically the object_list has some vmalloc'ed objects. Scanning such
objects is protected by the kmemleak_object.lock and the look-up by the
kmemleak_lock. What happens during kmemleak_disable() is that we set
kmemleak_enable to 0 and kmemleak_free() simply exits. When this happens
during a scan, objects in the object_list are freed/vunmap'ed but
kmemleak doesn't know about this until the clean-up completes (which, as
you found, may be blocked on the scanning to complete).

A patch I had but never managed to test it properly (as in reproducing
the low mem conditions during a scan) postpones the kmemleak disabling
until after the clean-up is finished. If it works for you, I'll add a
proper commit message:

-----8<-------------------------

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 5405aff5a590..dcba05812678 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -521,6 +521,10 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
 	struct kmemleak_object *object, *parent;
 	struct rb_node **link, *rb_parent;
 
+	/* stop further allocations while kmemleak is being disabled */
+	if (kmemleak_error)
+		return NULL;
+
 	object = kmem_cache_alloc(object_cache, gfp_kmemleak_mask(gfp));
 	if (!object) {
 		pr_warning("Cannot allocate a kmemleak_object structure\n");
@@ -741,6 +745,10 @@ static void add_scan_area(unsigned long ptr, size_t size, gfp_t gfp)
 	struct kmemleak_object *object;
 	struct kmemleak_scan_area *area;
 
+	/* stop further allocations while kmemleak is being disabled */
+	if (kmemleak_error)
+		return;
+
 	object = find_and_get_object(ptr, 1);
 	if (!object) {
 		kmemleak_warn("Adding scan area to unknown object at 0x%08lx\n",
@@ -1127,7 +1135,7 @@ static bool update_checksum(struct kmemleak_object *object)
  */
 static int scan_should_stop(void)
 {
-	if (!kmemleak_enabled)
+	if (kmemleak_error)
 		return 1;
 
 	/*
@@ -1755,6 +1763,10 @@ static void kmemleak_do_cleanup(struct work_struct *work)
 		pr_info("Kmemleak disabled without freeing internal data. "
 			"Reclaim the memory with \"echo clear > /sys/kernel/debug/kmemleak\"\n");
 	mutex_unlock(&scan_mutex);
+
+	/* stop any memory operation tracing */
+	kmemleak_enabled = 0;
+
 }
 
 static DECLARE_WORK(cleanup_work, kmemleak_do_cleanup);
@@ -1769,12 +1781,11 @@ static void kmemleak_disable(void)
 	if (cmpxchg(&kmemleak_error, 0, 1))
 		return;
 
-	/* stop any memory operation tracing */
-	kmemleak_enabled = 0;
-
 	/* check whether it is too early for a kernel thread */
 	if (kmemleak_initialized)
 		schedule_work(&cleanup_work);
+	else
+		kmemleak_enabled = 0;
 
 	pr_info("Kernel memory leak detector disabled\n");
 }

-- 
Catalin