linux-kernel - Re: [PATCH 01/14] iomap: iomap that extends beyond EOF should be marked dirty

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: <20191017215613.GN16973@dread.disaster.area>
Date:   Fri, 18 Oct 2019 08:56:13 +1100
From:   Dave Chinner <david@...morbit.com>
To:     "Darrick J. Wong" <darrick.wong@...cle.com>
Cc:     Christoph Hellwig <hch@....de>,
        Damien Le Moal <Damien.LeMoal@....com>,
        Andreas Gruenbacher <agruenba@...hat.com>,
        linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, Dave Chinner <dchinner@...hat.com>
Subject: Re: [PATCH 01/14] iomap: iomap that extends beyond EOF should be
 marked dirty

On Thu, Oct 17, 2019 at 11:39:17AM -0700, Darrick J. Wong wrote:
> On Thu, Oct 17, 2019 at 07:56:11PM +0200, Christoph Hellwig wrote:
> > From: Dave Chinner <dchinner@...hat.com>
> > 
> > When doing a direct IO that spans the current EOF, and there are
> > written blocks beyond EOF that extend beyond the current write, the
> > only metadata update that needs to be done is a file size extension.
> > 
> > However, we don't mark such iomaps as IOMAP_F_DIRTY to indicate that
> > there is IO completion metadata updates required, and hence we may
> > fail to correctly sync file size extensions made in IO completion
> > when O_DSYNC writes are being used and the hardware supports FUA.
> > 
> > Hence when setting IOMAP_F_DIRTY, we need to also take into account
> > whether the iomap spans the current EOF. If it does, then we need to
> > mark it dirty so that IO completion will call generic_write_sync()
> > to flush the inode size update to stable storage correctly.
> > 
> > Signed-off-by: Dave Chinner <dchinner@...hat.com>
> > Signed-off-by: Christoph Hellwig <hch@....de>
> 
> Looks ok, but need fixes tag.  Also, might it be wise to split off the
> ext4 section into a separate patch so that it can be backported
> separately?

I 've done a bit more digging on this, and the ext4 part is not
needed for DAX as IOMAP_F_DIRTY is only used in the page fault path
and hence can't change the file size. As such, this only affects
direct IO. Hence the ext4 hunk can be added to the ext4 iomap-dio
patchset that is being developed rather than being in this patch.

Fixes: 3460cac1ca76 ("iomap: Use FUA for pure data O_DSYNC DIO writes")

Cheers,

Dave.

-- 
Dave Chinner
david@...morbit.com