Re: Crash in crc32_le during kmemleak_scan()

[vigneshr@codeaurora.org]

>
> 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
> --

Thank you for the explanation and the patch. I will give this a shot and
will get back with the results.

--
Thanks and regards,
Vignesh Radhakrishnan


QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of the Code Aurora Forum, hosted by The Linux Foundation.