Re: [PATCH] mm: kmemleak: Fix crashing during kmemleak disabling

From: Andrew Morton
Date: Wed Jun 03 2015 - 19:29:47 EST


On Wed, 3 Jun 2015 16:42:56 +0100 Catalin Marinas <catalin.marinas@xxxxxxx> wrote:

> With the current implementation, if kmemleak is disabled because of an
> error condition (e.g. fails to allocate metadata), alloc/free calls are
> no longer tracked. Usually this is not a problem since the kmemleak
> metadata is being removed via kmemleak_do_cleanup(). However, if the
> scanning thread is running at the time of disabling, kmemleak would no
> longer notice a potential vfree() call and the freed/unmapped object may
> still be accessed, causing a fault.
>
> This patch separates the kmemleak_free() enabling/disabling from the
> overall kmemleak_enabled nob so that we can defer the disabling of the
> object freeing tracking until the scanning thread completed. The
> kmemleak_free_part() is deliberately ignored by this patch since this is
> only called during boot before the scanning thread started.

I'm having trouble with this. afacit, kmemleak_free() can still be
called while kmemleak_scan() is running on another CPU.
kmemleak_free_enabled hasn't been cleared yet so the races remain.

However your statement "if the scanning thread is running at the time
of disabling" implies that the race is between kmemleak_scan() and
kmemleak_disable(). Yet the race avoidance code is placed in
kmemleak_free().

All confused. A more detailed description of the race would help.

Also, the words "kmemleak would no longer notice a potential vfree()
call" aren't sufficiently specific. kmemleak is a big place - what
*part* of kmemleak are you referring to here?

Finally, I'm concerned that a bare

kmemleak_free_enabled = 0;

lacks sufficient synchronization with respect to the
kmemleak_free_enabled readers from a locking/reordering point of view.
What's the story here?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/