linux-ext4 - Re: Uninitialized extent races

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]

Date:	Fri, 21 Dec 2012 16:34:33 +0400
From:	Dmitry Monakhov <dmonakhov@...nvz.org>
To:	Jan Kara <jack@...e.cz>
Cc:	linux-ext4@...r.kernel.org
Subject: Re: Uninitialized extent races

On Fri, 21 Dec 2012 02:25:26 +0100, Jan Kara <jack@...e.cz> wrote:
>   Hello,
> 
>   I was looking into handling of unwritten extents and I've noticed two
> things:
> 1) Commit 60d4616f added ext4_flush_unwritten_io() call into
>    ext4_fallocate() with a comment that it should avoid a race between
>    AIO DIO and fallocate. But that really doesn't completely fix the issue
>    because writeback can create unwritten extents as well and that is not
>    synchronized with i_mutex unlike AIO DIO.
> 
> 2) Commit dee1f973 actually changed ext4_convert_unwritten_extents_endio()
>    to be able to deal with the situation when extent we want to convert
>    isn't exactly the one in the extent tree (i.e., it was merged with
>    another extent in the mean time). That was the situation 1) tried to
>    prevent so it seems to make ext4_flush_unwritten_io() call from 1)
>    unnecessary?
In fact first patch is still required because dee1f973 may result in
in-depth tree grow so ENOSPC is not unprobable which result in 
complain from ext4_end_io().
> 
> Am I missing something Dmitry? Also I was wondering about one thing: Does
> anybody see a problem with disabling merging of uninitialized extents
> completely?
Yeah, actually this is sound idea. Instead of fighting again weird races
it is reasonable to just prevent it, because merge uninitialized extents
is false optimization anyway. Later it will be merged after if extents
becomes initialized(which is right place for optimization).
> It would simplify the code (end_io conversion doesn't need to
> potentially split extents) and the case when we really want to merge
> extents - i.e., when someone calls fallocate() on small chunks - doesn't
> seem like the case we need to optimize for? Also it would bound the amount of
> transaction credits we need for conversion to 1 block which would make it
> easier for me to change ext4 to clear PageWriteback only after extent
> conversion is done (again code simplification, more uniform handling of
> page writeback).
> 
> 								Honza
> -- 
> Jan Kara <jack@...e.cz>
> SUSE Labs, CR
--
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