Re: [PATCHv3] mm/slab: fix race with dereferencing NULL ptr in alloc_calls_show

Path	csiph.com!goblin2!goblin.stu.neva.ru!gothmog.csi.it!bofh.it!news.nic.it!robomod
From	Vladimir Davydov <vdavydov@virtuozzo.com>
Newsgroups	linux.kernel
Subject	Re: [PATCHv3] mm/slab: fix race with dereferencing NULL ptr in alloc_calls_show
Date	Wed, 03 Feb 2016 10:50:03 +0100
Message-ID	<qY2ev-ew-29@gated-at.bofh.it> (permalink)
References	<qY0w3-7rK-21@gated-at.bofh.it>
X-Original-To	Dmitry Safonov <dsafonov@virtuozzo.com>
MIME-Version	1.0
Content-Type	text/plain; charset="us-ascii"
Content-Disposition	inline
X-Clientproxiedby	US-EXCH2.sw.swsoft.com (10.255.249.46) To US-EXCH.sw.swsoft.com (10.255.249.47)
Sender	robomod@news.nic.it
List-ID	<linux-kernel.vger.kernel.org>
X-Mailing-List	linux-kernel@vger.kernel.org
Approved	robomod@news.nic.it
Lines	73
Organization	linux.* mail to news gateway
X-Original-Cc	<akpm@linux-foundation.org>, <cl@linux.com>, <penberg@kernel.org>, <rientjes@google.com>, <iamjoonsoo.kim@lge.com>, <linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>, <0x7f454c46@gmail.com>
X-Original-Date	Wed, 3 Feb 2016 12:44:20 +0300
X-Original-Message-ID	<20160203094420.GH21016@esperanza>
X-Original-References	<1454485933-762-1-git-send-email-dsafonov@virtuozzo.com>
X-Original-Sender	linux-kernel-owner@vger.kernel.org
Xref	csiph.com linux.kernel:1325101

Show key headers only | View raw

On Wed, Feb 03, 2016 at 10:52:13AM +0300, Dmitry Safonov wrote:
...
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index b50aef0..2bfc0b1 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -451,6 +451,8 @@ EXPORT_SYMBOL(kmem_cache_create);
>  static int shutdown_cache(struct kmem_cache *s,
>  		struct list_head *release, bool *need_rcu_barrier)
>  {
> +	sysfs_slab_remove(s);
> +

shutdown_cache is called with slab_mutex held. slab_attr_store may take
the mutex. So placing sysfs_slab_remove here introduces a potential
deadlock.

>  	if (__kmem_cache_shutdown(s) != 0)
>  		return -EBUSY;
>  
> @@ -468,13 +470,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 +611,7 @@ 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;
> +

Please remove this hunk.

>  		/*
>  		 * 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,

I surmise this will resurrect the bug that was fixed by 41a212859a4dd
("slub: use sysfs'es release mechanism for kmem_cache").

Thanks,
Vladimir

Thread

[PATCHv3] mm/slab: fix race with dereferencing NULL ptr in alloc_calls_show Dmitry Safonov <dsafonov@virtuozzo.com> - 2016-02-03 09:00 +0100
  Re: [PATCHv3] mm/slab: fix race with dereferencing NULL ptr in  alloc_calls_show Vladimir Davydov <vdavydov@virtuozzo.com> - 2016-02-03 10:50 +0100
    Re: [PATCHv3] mm/slab: fix race with dereferencing NULL ptr in  alloc_calls_show Dmitry Safonov <dsafonov@virtuozzo.com> - 2016-02-03 12:40 +0100

csiph-web