linux-kernel - Re: [PATCH 4.4 13/16] ocfs2: should wait dio before inode lock in ocfs2

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <1512488994.18523.173.camel@codethink.co.uk>
Date:   Tue, 05 Dec 2017 15:49:54 +0000
From:   Ben Hutchings <ben.hutchings@...ethink.co.uk>
To:     Alex Chen <alex.chen@...wei.com>, Jun Piao <piaojun@...wei.com>,
        Joseph Qi <jiangqi903@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     stable@...r.kernel.org, Changwei Ge <ge.changwei@....com>,
        Mark Fasheh <mfasheh@...sity.com>,
        Joel Becker <jlbec@...lplan.org>,
        Junxiao Bi <junxiao.bi@...cle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4.4 13/16] ocfs2: should wait dio before inode lock in
 ocfs2_setattr()

On Wed, 2017-11-22 at 11:12 +0100, Greg Kroah-Hartman wrote:
> 4.4-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: alex chen <alex.chen@...wei.com>
> 
> commit 28f5a8a7c033cbf3e32277f4cc9c6afd74f05300 upstream.
> 
> we should wait dio requests to finish before inode lock in
> ocfs2_setattr(), otherwise the following deadlock will happen:
[...]

I looked at the kernel-doc for inode_dio_wait():

/**
 * inode_dio_wait - wait for outstanding DIO requests to finish
 * @inode: inode to wait for
 *
 * Waits for all pending direct I/O requests to finish so that we can
 * proceed with a truncate or equivalent operation.
 *
 * Must be called under a lock that serializes taking new references
 * to i_dio_count, usually by inode->i_mutex.
 */

Now that ocfs2_setattr() calls this outside of the inode locked region,
what prevents another task adding a new dio request immediately
afterward?

Also, ocfs2_dio_end_io_write() was introduced in 4.6 and it looks like
the dio completion path didn't previously take the inode lock.  So it
doesn't look this fix is needed in 3.18 or 4.4.

Ben.

-- 
Ben Hutchings
Software Developer, Codethink Ltd.