linux-ext4 - Re: [RFC PATCH] mark buffer_head mapping preallocate area as new during write

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: <49F730E6.2070904@redhat.com>
Date:	Tue, 28 Apr 2009 11:37:58 -0500
From:	Eric Sandeen <sandeen@...hat.com>
To:	Theodore Tso <tytso@....edu>
CC:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
	Mingming Cao <cmm@...ibm.com>, linux-ext4@...r.kernel.org
Subject: Re: [RFC PATCH] mark buffer_head mapping preallocate area as new
 during write_begin with delayed allocation

Theodore Tso wrote:
> On Tue, Apr 28, 2009 at 03:01:45PM +0530, Aneesh Kumar K.V wrote:
>> Looking at the source again i guess setting just b_dev is not enough.
>> unmap_underlying_metadata looks at the mapping block number, which we
>> don't have in case on unwritten buffer_head. How about the below patch ?
>> It involve vfs changes. But i guess it is correct with respect to the
>> meaning of BH_New (Disk mapping was newly created by get_block). I guess
>> BH_New implies BH_Mapped.
> 
> Argh.  So we have multiple problems going on here.  One is the
> original problem, namely that of a partial write into an preallocated
> block can leave garbage behind in that unitialized block.
> 
> The other problem seems to be in the case of a delayed allocation
> write, where we return a buffer_head which is marked new, and this
> causes block_prepare_write() to call unmap_underlying_metadata(dev, 0).
> 
> In theory this could cause problems if we try installing a new
> bootloader in the filesystem's boot block while there's a delayed
> writes happening in the background, since we could end up discarding
> the write to the boot sector.  We've lived with this for quite a wihle
> though.
> 
> My concern with making the fs/buffer.c changes is that we need to make
> sure it doesn't break any of the other filesystems, so that's going to
> make it hard to try to slip this with 2.6.30-rc4 nearly upon us.
> (Silly question; why doesn't XFS get caught by this?) 

I'm not sure offhand.  All xfs does is this in the get_block path:

         * With sub-block writes into unwritten extents we also need to mark
         * the buffer as new so that the unwritten parts of the buffer gets
         * correctly zeroed.
         */
        if (create &&
            ((!buffer_mapped(bh_result) && !buffer_uptodate(bh_result)) ||
             (offset >= i_size_read(inode)) ||
             (iomap.iomap_flags & (IOMAP_NEW|IOMAP_UNWRITTEN))))
                set_buffer_new(bh_result);

so it returns with BH_New as well.

> So the question is do we try to fix both bugs with one patch, and very
> likely have to wait until 2.6.31 before the patch is incorporated?  Or
> do we fix the second bug using an ext4-only fix, with the knowledge
> that post 2.6.30, we'll need undo most of it and fix it properly with
> a change that involves fs/buffer.c?

I have the sense that this might need a bit more digging around, and I
finally got stuff out of the way to do so :)

-Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html