linux-kernel - [PATCH] slab: fix wrong retval on kmem_cache_create

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <1390598126-4332-1-git-send-email-vdavydov@parallels.com>
Date:	Sat, 25 Jan 2014 01:15:26 +0400
From:	Vladimir Davydov <vdavydov@...allels.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	<linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>,
	Dave Jones <davej@...hat.com>,
	Pekka Enberg <penberg@...nel.org>,
	Christoph Lameter <cl@...ux.com>
Subject: [PATCH] slab: fix wrong retval on kmem_cache_create_memcg error path

From: Dave Jones <davej@...hat.com>

On kmem_cache_create_memcg() error path we set 'err', but leave 's' (the
new cache ptr) undefined. The latter can be NULL if we could not
allocate the cache, or pointing to a freed area if we failed somewhere
later while trying to initialize it. Initially we checked 'err'
immediately before exiting the function and returned NULL if it was set
ignoring the value of 's':

    out_unlock:
        ...
        if (err) {
            ...
            return NULL;
        }
        return s;

Recently this check was, in fact, broken by commit f717eb3abb5e ("slab:
do not panic if we fail to create memcg cache"), which turned it to:

    out_unlock:
        ...
        if (err && !memcg) {
            ...
            return NULL;
        }
        return s;

As a result, if we are failing creating a cache for a memcg, we will
skip the check and return 's' that can contain crap. Let's fix it by
assuring that on error path there are always two conditions satisfied at
the same time, err != 0 and s == NULL, by explicitly zeroing 's' after
freeing it on error path.

Signed-off-by: Dave Jones <davej@...hat.com>
Signed-off-by: Vladimir Davydov <vdavydov@...allels.com>
Cc: Pekka Enberg <penberg@...nel.org>
Cc: Christoph Lameter <cl@...ux.com>
---
 mm/slab_common.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 8e40321..499b53c 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -249,7 +249,6 @@ out_unlock:
 				name, err);
 			dump_stack();
 		}
-		return NULL;
 	}
 	return s;

@@ -257,6 +256,7 @@ out_free_cache:
 	memcg_free_cache_params(s);
 	kfree(s->name);
 	kmem_cache_free(kmem_cache, s);
+	s = NULL;
 	goto out_unlock;
 }

-- 
1.7.10.4

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