fsck breaks preallocated files

[antmat]

Hi!
First of all I'm in doubt if this is a ext4 driver issue or fsck issue, but as far as without fsck all works just fine I thought you may be interested in it.
In situation when we have a file with rather small size, which was preallocated via fallocate with FALLOC_FL_KEEP_SIZE flag to rather big value fsck treats its extents as invalid and lost data.

Some details:

• this happens when extent_tree grows in depth to 3 or more. When this happens top node of extent tree contains range that corresponds to 0-file_size_in_blocks rather than 0-number_of_allocated_blocks
• in this case it seems that file is treated correctly by the kernel, but as far fsck is being run on partion:
  • https://github.com/tytso/e2fsprogs/blob/master/e2fsck/pass1.c#L3089 we recursively call scan_extent_node for level 2 of 3 of extent tree and pass last_lblk
  • check on this line https://github.com/tytso/e2fsprogs/blob/master/e2fsck/pass1.c#L2920 fails as child extent is located beyond parent end, this results in removing part of extent tree with some user data in it (sometimes we can hit e2fsck: Error in deleting parent extent #108 this issue, sometimes not)

I created a minimal reproducible example with minimal comments, you can see it here:
https://gist.github.com/antmat/569408754a007f7519a0ce5122e4ffac

Thanks in advance!

[antmat]

It looks like the problem is here

e2fsprogs/lib/ext2fs/extent.c

Line 287 in 25ad8a4

handle->path[0].end_blk =

We assume, that end_blk is located at the size bound of file, but that is not true, as we preallocated more than size blocks.
The check does not fail on extent tree of deth 2, as on the second level there are leaf nodes that are in UNINIT state, but starting from depth 3 we have index nodes, which doesn't have that flag

[antmat]

https://gist.github.com/antmat/9c3ce004a080c50d341e8c3afb22416a
I've created a simple patch, that fixes the problem, but it has a problem itself: if we have a file with holes and preallocated space end_blk should be calculated smarter. Perhaps we could traverse child nodes to determine real bounds or we can leave end_blk zeroed.