linux-kernel - Re: z3fold: suspicious return with spinlock held

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: <2E74B250-1EE0-483A-ACA0-143C4F1ECD44@gmail.com>
Date:   Fri, 10 Mar 2017 23:08:53 +0100
From:   vitalywool@...il.com
To:     Alexey Khoroshilov <khoroshilov@...ras.ru>
Cc:     Matthew Wilcox <willy@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Dan Streetman <ddstreet@...e.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, ldv-project@...uxtesting.org
Subject: Re: z3fold: suspicious return with spinlock held

Hi Alexey,

> 10 mars 2017 kl. 22:54 skrev Alexey Khoroshilov <khoroshilov@...ras.ru>:
> 
>> On 11.03.2017 00:34, Matthew Wilcox wrote:
>>> On Sat, Mar 11, 2017 at 12:22:12AM +0300, Alexey Khoroshilov wrote:
>>> Hello!
>>> 
>>> z3fold_reclaim_page() contains the only return that may
>>> leave the function with pool->lock spinlock held.
>>> 
>>> 669    spin_lock(&pool->lock);
>>> 670    if (kref_put(&zhdr->refcount, release_z3fold_page)) {
>>> 671        atomic64_dec(&pool->pages_nr);
>>> 672        return 0;
>>> 673    }
>>> 
>>> May be we need spin_unlock(&pool->lock); just before return?
Looks so, thanks for the pointer. I'm currently commuting but will check it thoroughly tomorrow for sure.

~vitaly 

>> 
>> I would tend to agree.  sparse warns about this, and also about two
>> other locking problems ... which I'm not sure are really problems so
>> much as missing annotations?
>> 
>> mm/z3fold.c:467:35: warning: context imbalance in 'z3fold_alloc' - unexpected unlock
>> mm/z3fold.c:519:26: warning: context imbalance in 'z3fold_free' - different lock contexts for basic block
>> mm/z3fold.c:581:12: warning: context imbalance in 'z3fold_reclaim_page' - different lock contexts for basic block
>> 
> 
> I also do not see problems in z3fold_alloc() and z3fold_free().
> But I am unaware of sparse annotations that can help here.
> 
> --
> Alexey