Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1325243

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

From Dmitry Safonov <dsafonov@virtuozzo.com>
Newsgroups linux.kernel
Subject Re: [PATCHv3] mm/slab: fix race with dereferencing NULL ptr in alloc_calls_show
Date 2016-02-03 12:40 +0100
Message-ID <qY3WW-1lI-29@gated-at.bofh.it> (permalink)
References <qY0w3-7rK-21@gated-at.bofh.it> <qY2ev-ew-29@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 02/03/2016 12:44 PM, Vladimir Davydov wrote:
> 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").
So, I move __kmem_cache_shutdown into slab_kmem_cache_release,
release list will still be collected under slab_mutex.
Seems like no other way to do this.
Will resend mended version.
>
> Thanks,
> Vladimir


-- 
Regards,
Dmitry Safonov

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


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