linux-kernel - Re: [PATCH] fs,xfs: fix missed wakeup on l_flush

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:   Thu, 09 May 2019 10:27:00 -0400
From:   Rik van Riel <riel@...riel.com>
To:     Dave Chinner <david@...morbit.com>
Cc:     linux-xfs@...r.kernel.org, kernel-team@...com,
        linux-kernel@...r.kernel.org,
        "Darrick J. Wong" <darrick.wong@...cle.com>,
        David Chinner <dchinner@...hat.com>
Subject: Re: [PATCH] fs,xfs: fix missed wakeup on l_flush_wait

On Thu, 2019-05-09 at 07:32 +1000, Dave Chinner wrote:

> Hmmm, the first wakeup in xsdc is this one, right:
> 
> 	       /* wake up threads waiting in xfs_log_force() */
> 	       wake_up_all(&iclog->ic_force_wait);
> 
> At the end of the iclog iteration loop? That one is under the
> ic_loglock - the lock is dropped to run callbacks, then picked up
> again once the callbacks are done and before the ic_callback_lock is
> dropped (about 10 lines above the wakeup). So unless I'm missing
> something (like enough coffee!) that one look fine.

You are right. I failed to spot that the spin_unlock unlocks 
a different lock than the spin_lock above it takes. 

> .....
> 
> > I am not sure about xfs_log_force_umount(). Could the unlock 
> > be moved to after the wake_up_all, or does that create lock
> > ordering issues with the xlog_grant_head_wake_all calls?
> > Could a simple lock + unlock of log->l_icloglock around the
> > wake_up_all do the trick, or is there some other state that
> > also needs to stay locked?
> 
> Need to be careful which lock is used with which wait queue :)
> 
> This one is waking the the xc_commit_wait queue (CIL push commit
> sequencing wait queue), which is protected by the
> log->l_cilp->xc_push_lock. That should nest jsut fine inside any
> locks we are holding at this point, so you should just be able to
> wrap it.  It's not a common code path, though, it'll only hit this
> code when the filesystem is already considered to be in an
> unrecoverable state.

Awesome.

-- 
All Rights Reversed.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)