linux-kernel - Re: [PATCHv2] mm/slab: fix race with dereferencing NULL ptr in alloc_calls

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-prev] [day] [month] [year] [list]

Date:	Tue, 2 Feb 2016 13:50:25 -0800 (PST)
From:	David Rientjes <rientjes@...gle.com>
To:	Dmitry Safonov <dsafonov@...tuozzo.com>
cc:	akpm@...ux-foundation.org, cl@...ux.com, penberg@...nel.org,
	iamjoonsoo.kim@....com, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, 0x7f454c46@...il.com,
	Vladimir Davydov <vdavydov@...tuozzo.com>
Subject: Re: [PATCHv2] mm/slab: fix race with dereferencing NULL ptr in
 alloc_calls_show

On Tue, 2 Feb 2016, Dmitry Safonov wrote:

> diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
> index b7e57927..43634cd 100644
> --- a/include/linux/slub_def.h
> +++ b/include/linux/slub_def.h
> @@ -101,16 +101,6 @@ struct kmem_cache {
>  	struct kmem_cache_node *node[MAX_NUMNODES];
>  };
>  
> -#ifdef CONFIG_SYSFS
> -#define SLAB_SUPPORTS_SYSFS
> -void sysfs_slab_remove(struct kmem_cache *);
> -#else
> -static inline void sysfs_slab_remove(struct kmem_cache *s)
> -{
> -}
> -#endif
> -
> -
>  /**
>   * virt_to_obj - returns address of the beginning of object.
>   * @s: object's kmem_cache
> diff --git a/mm/slab.h b/mm/slab.h
> index 834ad24..2983ab2 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -367,6 +367,14 @@ static inline struct kmem_cache_node *get_node(struct kmem_cache *s, int node)
>  
>  #endif
>  
> +#if defined(CONFIG_SLUB) && defined(CONFIG_SYSFS)
> +void sysfs_slab_remove(struct kmem_cache *);
> +#else
> +static inline void sysfs_slab_remove(struct kmem_cache *s)
> +{
> +}
> +#endif
> +
>  void *slab_start(struct seq_file *m, loff_t *pos);
>  void *slab_next(struct seq_file *m, void *p, loff_t *pos);
>  void slab_stop(struct seq_file *m, void *p);
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index b50aef0..6725eb3 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -468,13 +468,8 @@ static void release_caches(struct list_head *release, bool need_rcu_barrier)
>  	if (need_rcu_barrier)
>  		rcu_barrier();
>  
> -	list_for_each_entry_safe(s, s2, release, list) {
> -#ifdef SLAB_SUPPORTS_SYSFS
> -		sysfs_slab_remove(s);
> -#else
> +	list_for_each_entry_safe(s, s2, release, list)
>  		slab_kmem_cache_release(s);
> -#endif
> -	}
>  }
>  
>  #if defined(CONFIG_MEMCG) && !defined(CONFIG_SLOB)
> @@ -614,6 +609,9 @@ void memcg_destroy_kmem_caches(struct mem_cgroup *memcg)
>  	list_for_each_entry_safe(s, s2, &slab_caches, list) {
>  		if (is_root_cache(s) || s->memcg_params.memcg != memcg)
>  			continue;
> +
> +		sysfs_slab_remove(s);
> +

I would have expected to have seen this added to shutdown_cache() instead.

>  		/*
>  		 * The cgroup is about to be freed and therefore has no charges
>  		 * left. Hence, all its caches must be empty by now.
> diff --git a/mm/slub.c b/mm/slub.c
> index 2e1355a..b6a68b7 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -5296,11 +5296,6 @@ static void memcg_propagate_slab_attrs(struct kmem_cache *s)
>  #endif
>  }
>  
> -static void kmem_cache_release(struct kobject *k)
> -{
> -	slab_kmem_cache_release(to_slab(k));
> -}
> -
>  static const struct sysfs_ops slab_sysfs_ops = {
>  	.show = slab_attr_show,
>  	.store = slab_attr_store,
> @@ -5308,7 +5303,6 @@ static const struct sysfs_ops slab_sysfs_ops = {
>  
>  static struct kobj_type slab_ktype = {
>  	.sysfs_ops = &slab_sysfs_ops,
> -	.release = kmem_cache_release,
>  };
>  
>  static int uevent_filter(struct kset *kset, struct kobject *kobj)