linux-kernel - Re: slab: clean up 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 for Android: free password hash cracker in your pocket

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

Date:	Fri, 24 Jan 2014 13:20:48 -0500
From:	Dave Jones <davej@...hat.com>
To:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Cc:	vdavydov@...allels.com
Subject: Re: slab: clean up kmem_cache_create_memcg() error handling

On Fri, Jan 24, 2014 at 03:33:41AM +0000, Linux Kernel wrote:
 > Gitweb:     http://git.kernel.org/linus/;a=commit;h=3965fc3652244651006ebb31c8c45318ce84818f
 > Commit:     3965fc3652244651006ebb31c8c45318ce84818f
 > Parent:     309381feaee564281c3d9e90fbca8963bb7428ad
 > Author:     Vladimir Davydov <vdavydov@...allels.com>
 > AuthorDate: Thu Jan 23 15:52:55 2014 -0800
 > Committer:  Linus Torvalds <torvalds@...ux-foundation.org>
 > CommitDate: Thu Jan 23 16:36:50 2014 -0800
 > 
 >     slab: clean up kmem_cache_create_memcg() error handling
 >     
 >     Currently kmem_cache_create_memcg() backoffs on failure inside
 >     conditionals, without using gotos.  This results in the rollback code
 >     duplication, which makes the function look cumbersome even though on
 >     error we should only free the allocated cache.  Since in the next patch
 >     I am going to add yet another rollback function call on error path
 >     there, let's employ labels instead of conditionals for undoing any
 >     changes on failure to keep things clean.

...

 > +out_unlock:
 >  	mutex_unlock(&slab_mutex);
 >  	put_online_cpus();
 >  
 >  	if (err) {
 > -
 >  		if (flags & SLAB_PANIC)
 >  			panic("kmem_cache_create: Failed to create slab '%s'. Error %d\n",
 >  				name, err);
 > @@ -236,11 +230,14 @@ out_locked:
 >  				name, err);
 >  			dump_stack();
 >  		}
 > -
 >  		return NULL;
 >  	}
 > -
 >  	return s;
 > +
 > +out_free_cache:
 > +	kfree(s->name);
 > +	kmem_cache_free(kmem_cache, s);
 > +	goto out_unlock;
 >  }

This is now returning a freed pointer as 's' if an error occurs.
Perhaps the patch below ?

	Dave


diff --git a/mm/slab_common.c b/mm/slab_common.c
index 8e40321da091..2c62294cee23 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -257,6 +257,7 @@ out_free_cache:
 	memcg_free_cache_params(s);
 	kfree(s->name);
 	kmem_cache_free(kmem_cache, s);
+	s = NULL;
 	goto out_unlock;
 }
 
--
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/