Skip to content

Commit

Permalink
slab: fix wrong retval on kmem_cache_create_memcg error path
Browse files Browse the repository at this point in the history
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) {
            /* report error */
            return NULL;
        }
        return s;

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

    out_unlock:
        ...
        if (err && !memcg) {
            /* report error */
            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.  Obviously, commit
f717eb3 intended not to return crap on error allocating a cache for
a memcg, but only to remove the error reporting in this case, so the
check should look like this:

    out_unlock:
        ...
        if (err) {
            if (!memcg)
                return NULL;
            /* report error */
            return NULL;
        }
        return s;

[[email protected]: despaghettification]
[[email protected]: patch monkeying]
Signed-off-by: David Rientjes <[email protected]>
Signed-off-by: Vladimir Davydov <[email protected]>
Signed-off-by: Dave Jones <[email protected]>
Reported-by: Dave Jones <[email protected]>
Acked-by: Pekka Enberg <[email protected]>
Cc: Christoph Lameter <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
Dave Jones authored and torvalds committed Jan 30, 2014
1 parent 49382d9 commit ba3253c
Showing 1 changed file with 11 additions and 8 deletions.
19 changes: 11 additions & 8 deletions mm/slab_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -233,14 +233,17 @@ kmem_cache_create_memcg(struct mem_cgroup *memcg, const char *name, size_t size,
mutex_unlock(&slab_mutex);
put_online_cpus();

/*
* There is no point in flooding logs with warnings or especially
* crashing the system if we fail to create a cache for a memcg. In
* this case we will be accounting the memcg allocation to the root
* cgroup until we succeed to create its own cache, but it isn't that
* critical.
*/
if (err && !memcg) {
if (err) {
/*
* There is no point in flooding logs with warnings or
* especially crashing the system if we fail to create a cache
* for a memcg. In this case we will be accounting the memcg
* allocation to the root cgroup until we succeed to create its
* own cache, but it isn't that critical.
*/
if (!memcg)
return NULL;

if (flags & SLAB_PANIC)
panic("kmem_cache_create: Failed to create slab '%s'. Error %d\n",
name, err);
Expand Down

0 comments on commit ba3253c

Please sign in to comment.