Revision f6ba488073fe8159851fe398cc3c5ee383bb4c7a authored by Vladimir Davydov on 18 August 2017, 22:16:08 UTC, committed by Linus Torvalds on 18 August 2017, 22:32:01 UTC
To avoid a possible deadlock, sysfs_slab_remove() schedules an
asynchronous work to delete sysfs entries corresponding to the kmem
cache.  To ensure the cache isn't freed before the work function is
called, it takes a reference to the cache kobject.  The reference is
supposed to be released by the work function.

However, the work function (sysfs_slab_remove_workfn()) does nothing in
case the cache sysfs entry has already been deleted, leaking the kobject
and the corresponding cache.

This may happen on a per memcg cache destruction, because sysfs entries
of a per memcg cache are deleted on memcg offline if the cache is empty
(see __kmemcg_cache_deactivate()).

The kmemleak report looks like this:

  unreferenced object 0xffff9f798a79f540 (size 32):
    comm "kworker/1:4", pid 15416, jiffies 4307432429 (age 28687.554s)
    hex dump (first 32 bytes):
      6b 6d 61 6c 6c 6f 63 2d 31 36 28 31 35 39 39 3a  kmalloc-16(1599:
      6e 65 77 72 6f 6f 74 29 00 23 6b c0 ff ff ff ff  newroot).#k.....
    backtrace:
       kmemleak_alloc+0x4a/0xa0
       __kmalloc_track_caller+0x148/0x2c0
       kvasprintf+0x66/0xd0
       kasprintf+0x49/0x70
       memcg_create_kmem_cache+0xe6/0x160
       memcg_kmem_cache_create_func+0x20/0x110
       process_one_work+0x205/0x5d0
       worker_thread+0x4e/0x3a0
       kthread+0x109/0x140
       ret_from_fork+0x2a/0x40
  unreferenced object 0xffff9f79b6136840 (size 416):
    comm "kworker/1:4", pid 15416, jiffies 4307432429 (age 28687.573s)
    hex dump (first 32 bytes):
      40 fb 80 c2 3e 33 00 00 00 00 00 40 00 00 00 00  @...>3.....@....
      00 00 00 00 00 00 00 00 10 00 00 00 10 00 00 00  ................
    backtrace:
       kmemleak_alloc+0x4a/0xa0
       kmem_cache_alloc+0x128/0x280
       create_cache+0x3b/0x1e0
       memcg_create_kmem_cache+0x118/0x160
       memcg_kmem_cache_create_func+0x20/0x110
       process_one_work+0x205/0x5d0
       worker_thread+0x4e/0x3a0
       kthread+0x109/0x140
       ret_from_fork+0x2a/0x40

Fix the leak by adding the missing call to kobject_put() to
sysfs_slab_remove_workfn().

Link: http://lkml.kernel.org/r/20170812181134.25027-1-vdavydov.dev@gmail.com
Fixes: 3b7b314053d02 ("slub: make sysfs file removal asynchronous")
Signed-off-by: Vladimir Davydov <vdavydov.dev@gmail.com>
Reported-by: Andrei Vagin <avagin@gmail.com>
Tested-by: Andrei Vagin <avagin@gmail.com>
Acked-by: Tejun Heo <tj@kernel.org>
Acked-by: David Rientjes <rientjes@google.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: <stable@vger.kernel.org>	[4.12.x]
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
1 parent 3010f87
Raw File
test_hash.c
/*
 * Test cases for <linux/hash.h> and <linux/stringhash.h>
 * This just verifies that various ways of computing a hash
 * produce the same thing and, for cases where a k-bit hash
 * value is requested, is of the requested size.
 *
 * We fill a buffer with a 255-byte null-terminated string,
 * and use both full_name_hash() and hashlen_string() to hash the
 * substrings from i to j, where 0 <= i < j < 256.
 *
 * The returned values are used to check that __hash_32() and
 * __hash_32_generic() compute the same thing.  Likewise hash_32()
 * and hash_64().
 */

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt "\n"

#include <linux/compiler.h>
#include <linux/types.h>
#include <linux/module.h>
#include <linux/hash.h>
#include <linux/stringhash.h>
#include <linux/printk.h>

/* 32-bit XORSHIFT generator.  Seed must not be zero. */
static u32 __init __attribute_const__
xorshift(u32 seed)
{
	seed ^= seed << 13;
	seed ^= seed >> 17;
	seed ^= seed << 5;
	return seed;
}

/* Given a non-zero x, returns a non-zero byte. */
static u8 __init __attribute_const__
mod255(u32 x)
{
	x = (x & 0xffff) + (x >> 16);	/* 1 <= x <= 0x1fffe */
	x = (x & 0xff) + (x >> 8);	/* 1 <= x <= 0x2fd */
	x = (x & 0xff) + (x >> 8);	/* 1 <= x <= 0x100 */
	x = (x & 0xff) + (x >> 8);	/* 1 <= x <= 0xff */
	return x;
}

/* Fill the buffer with non-zero bytes. */
static void __init
fill_buf(char *buf, size_t len, u32 seed)
{
	size_t i;

	for (i = 0; i < len; i++) {
		seed = xorshift(seed);
		buf[i] = mod255(seed);
	}
}

/*
 * Test the various integer hash functions.  h64 (or its low-order bits)
 * is the integer to hash.  hash_or accumulates the OR of the hash values,
 * which are later checked to see that they cover all the requested bits.
 *
 * Because these functions (as opposed to the string hashes) are all
 * inline, the code being tested is actually in the module, and you can
 * recompile and re-test the module without rebooting.
 */
static bool __init
test_int_hash(unsigned long long h64, u32 hash_or[2][33])
{
	int k;
	u32 h0 = (u32)h64, h1, h2;

	/* Test __hash32 */
	hash_or[0][0] |= h1 = __hash_32(h0);
#ifdef HAVE_ARCH__HASH_32
	hash_or[1][0] |= h2 = __hash_32_generic(h0);
#if HAVE_ARCH__HASH_32 == 1
	if (h1 != h2) {
		pr_err("__hash_32(%#x) = %#x != __hash_32_generic() = %#x",
			h0, h1, h2);
		return false;
	}
#endif
#endif

	/* Test k = 1..32 bits */
	for (k = 1; k <= 32; k++) {
		u32 const m = ((u32)2 << (k-1)) - 1;	/* Low k bits set */

		/* Test hash_32 */
		hash_or[0][k] |= h1 = hash_32(h0, k);
		if (h1 > m) {
			pr_err("hash_32(%#x, %d) = %#x > %#x", h0, k, h1, m);
			return false;
		}
#ifdef HAVE_ARCH_HASH_32
		h2 = hash_32_generic(h0, k);
#if HAVE_ARCH_HASH_32 == 1
		if (h1 != h2) {
			pr_err("hash_32(%#x, %d) = %#x != hash_32_generic() "
				" = %#x", h0, k, h1, h2);
			return false;
		}
#else
		if (h2 > m) {
			pr_err("hash_32_generic(%#x, %d) = %#x > %#x",
				h0, k, h1, m);
			return false;
		}
#endif
#endif
		/* Test hash_64 */
		hash_or[1][k] |= h1 = hash_64(h64, k);
		if (h1 > m) {
			pr_err("hash_64(%#llx, %d) = %#x > %#x", h64, k, h1, m);
			return false;
		}
#ifdef HAVE_ARCH_HASH_64
		h2 = hash_64_generic(h64, k);
#if HAVE_ARCH_HASH_64 == 1
		if (h1 != h2) {
			pr_err("hash_64(%#llx, %d) = %#x != hash_64_generic() "
				"= %#x", h64, k, h1, h2);
			return false;
		}
#else
		if (h2 > m) {
			pr_err("hash_64_generic(%#llx, %d) = %#x > %#x",
				h64, k, h1, m);
			return false;
		}
#endif
#endif
	}

	(void)h2;	/* Suppress unused variable warning */
	return true;
}

#define SIZE 256	/* Run time is cubic in SIZE */

static int __init
test_hash_init(void)
{
	char buf[SIZE+1];
	u32 string_or = 0, hash_or[2][33] = { { 0, } };
	unsigned tests = 0;
	unsigned long long h64 = 0;
	int i, j;

	fill_buf(buf, SIZE, 1);

	/* Test every possible non-empty substring in the buffer. */
	for (j = SIZE; j > 0; --j) {
		buf[j] = '\0';

		for (i = 0; i <= j; i++) {
			u64 hashlen = hashlen_string(buf+i, buf+i);
			u32 h0 = full_name_hash(buf+i, buf+i, j-i);

			/* Check that hashlen_string gets the length right */
			if (hashlen_len(hashlen) != j-i) {
				pr_err("hashlen_string(%d..%d) returned length"
					" %u, expected %d",
					i, j, hashlen_len(hashlen), j-i);
				return -EINVAL;
			}
			/* Check that the hashes match */
			if (hashlen_hash(hashlen) != h0) {
				pr_err("hashlen_string(%d..%d) = %08x != "
					"full_name_hash() = %08x",
					i, j, hashlen_hash(hashlen), h0);
				return -EINVAL;
			}

			string_or |= h0;
			h64 = h64 << 32 | h0;	/* For use with hash_64 */
			if (!test_int_hash(h64, hash_or))
				return -EINVAL;
			tests++;
		} /* i */
	} /* j */

	/* The OR of all the hash values should cover all the bits */
	if (~string_or) {
		pr_err("OR of all string hash results = %#x != %#x",
			string_or, -1u);
		return -EINVAL;
	}
	if (~hash_or[0][0]) {
		pr_err("OR of all __hash_32 results = %#x != %#x",
			hash_or[0][0], -1u);
		return -EINVAL;
	}
#ifdef HAVE_ARCH__HASH_32
#if HAVE_ARCH__HASH_32 != 1	/* Test is pointless if results match */
	if (~hash_or[1][0]) {
		pr_err("OR of all __hash_32_generic results = %#x != %#x",
			hash_or[1][0], -1u);
		return -EINVAL;
	}
#endif
#endif

	/* Likewise for all the i-bit hash values */
	for (i = 1; i <= 32; i++) {
		u32 const m = ((u32)2 << (i-1)) - 1;	/* Low i bits set */

		if (hash_or[0][i] != m) {
			pr_err("OR of all hash_32(%d) results = %#x "
				"(%#x expected)", i, hash_or[0][i], m);
			return -EINVAL;
		}
		if (hash_or[1][i] != m) {
			pr_err("OR of all hash_64(%d) results = %#x "
				"(%#x expected)", i, hash_or[1][i], m);
			return -EINVAL;
		}
	}

	/* Issue notices about skipped tests. */
#ifdef HAVE_ARCH__HASH_32
#if HAVE_ARCH__HASH_32 != 1
	pr_info("__hash_32() is arch-specific; not compared to generic.");
#endif
#else
	pr_info("__hash_32() has no arch implementation to test.");
#endif
#ifdef HAVE_ARCH_HASH_32
#if HAVE_ARCH_HASH_32 != 1
	pr_info("hash_32() is arch-specific; not compared to generic.");
#endif
#else
	pr_info("hash_32() has no arch implementation to test.");
#endif
#ifdef HAVE_ARCH_HASH_64
#if HAVE_ARCH_HASH_64 != 1
	pr_info("hash_64() is arch-specific; not compared to generic.");
#endif
#else
	pr_info("hash_64() has no arch implementation to test.");
#endif

	pr_notice("%u tests passed.", tests);

	return 0;
}

static void __exit test_hash_exit(void)
{
}

module_init(test_hash_init);	/* Does everything */
module_exit(test_hash_exit);	/* Does nothing */

MODULE_LICENSE("GPL");
back to top