linux-kernel - Re: [PATCH] slab: add a check for the first kmem

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]

Message-Id: <380AA0F8-58C6-4AC7-AE06-D3A326E5B396@toanyone.net>
Date:   Wed, 18 Jan 2017 11:36:15 +0900
From:   kwon <kwon@...nyone.net>
To:     David Rientjes <rientjes@...gle.com>
Cc:     Joonsoo Kim <iamjoonsoo.kim@....com>,
        Christoph Lameter <cl@...ux.com>,
        Pekka Enberg <penberg@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] slab: add a check for the first kmem_cache not to be destroyed


> On Jan 18, 2017, at 7:54 AM, David Rientjes <rientjes@...gle.com> wrote:
> 
> On Tue, 17 Jan 2017, kwon wrote:
> 
>>>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>>>> index 1dfc209..2d30ace 100644
>>>> --- a/mm/slab_common.c
>>>> +++ b/mm/slab_common.c
>>>> @@ -744,7 +744,7 @@ void kmem_cache_destroy(struct kmem_cache *s)
>>>> 	bool need_rcu_barrier = false;
>>>> 	int err;
>>>> 
>>>> -	if (unlikely(!s))
>>>> +	if (unlikely(!s) || s->refcount == -1)
>>>> 		return;
>>> 
>>> Hello, Kyunghwan.
>>> 
>>> Few lines below, s->refcount is checked.
>>> 
>>> if (s->refcount)
>>>       goto unlock;
>>> 
>>> Am I missing something?
>>> 
>>> Thanks.
>> 
>> Hello, Joonsoo.
>> 
>> In case it is called the number of int size times. refcount would finally reach
>> to 0 since decreased every time the function called.
>> 
> 
> The only thing using create_boot_cache() should be the slab implementation 
> itself, so I don't think we need to protect ourselves from doing something 
> like kmem_cache_destroy(kmem_cache) or 
> kmem_cache_destroy(kmem_cache_node) even a single time.

Agreed. I was aware of that though, I thought it would make its logic firm not
giving performance disadvantages. Sorry for distraction.