Btrfs: fix race between fsync and direct IO writes for prealloc extents

When we do a direct IO write against a preallocated extent (fallocate)
that does not go beyond the i_size of the inode, we do the write operation
without holding the inode's i_mutex (an optimization that landed in
commit 38851cc19adb ("Btrfs: implement unlocked dio write")). This allows
for a very tiny time window where a race can happen with a concurrent
fsync using the fast code path, as the direct IO write path creates first
a new extent map (no longer flagged as a prealloc extent) and then it
creates the ordered extent, while the fast fsync path first collects
ordered extents and then it collects extent maps. This allows for the
possibility of the fast fsync path to collect the new extent map without
collecting the new ordered extent, and therefore logging an extent item
based on the extent map without waiting for the ordered extent to be
created and complete. This can result in a situation where after a log
replay we end up with an extent not marked anymore as prealloc but it was
only partially written (or not written at all), exposing random, stale or
garbage data corresponding to the unwritten pages and without any
checksums in the csum tree covering the extent's range.

This is an extension of what was done in commit de0ee0edb21f ("Btrfs: fix
race between fsync and lockless direct IO writes").

So fix this by creating first the ordered extent and then the extent
map, so that this way if the fast fsync patch collects the new extent
map it also collects the corresponding ordered extent.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Josef Bacik <jbacik@fb.com>

1 files changed, 37 insertions, 6 deletions

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 0921d2b07f1d..45d0dafbbf40 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -7658,6 +7658,25 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
 
 		if (can_nocow_extent(inode, start, &len, &orig_start,
 				     &orig_block_len, &ram_bytes) == 1) {
+
+			/*
+			 * Create the ordered extent before the extent map. This
+			 * is to avoid races with the fast fsync path because it
+			 * collects ordered extents into a local list and then
+			 * collects all the new extent maps, so we must create
+			 * the ordered extent first and make sure the fast fsync
+			 * path collects any new ordered extents after
+			 * collecting new extent maps as well. The fsync path
+			 * simply can not rely on inode_dio_wait() because it
+			 * causes deadlock with AIO.
+			 */
+			ret = btrfs_add_ordered_extent_dio(inode, start,
+					   block_start, len, len, type);
+			if (ret) {
+				free_extent_map(em);
+				goto unlock_err;
+			}
+
 			if (type == BTRFS_ORDERED_PREALLOC) {
 				free_extent_map(em);
 				em = create_pinned_em(inode, start, len,
@@ -7666,17 +7685,29 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
 						       orig_block_len,
 						       ram_bytes, type);
 				if (IS_ERR(em)) {
+					struct btrfs_ordered_extent *oe;
+
 					ret = PTR_ERR(em);
+					oe = btrfs_lookup_ordered_extent(inode,
+									 start);
+					ASSERT(oe);
+					if (WARN_ON(!oe))
+						goto unlock_err;
+					set_bit(BTRFS_ORDERED_IOERR,
+						&oe->flags);
+					set_bit(BTRFS_ORDERED_IO_DONE,
+						&oe->flags);
+					btrfs_remove_ordered_extent(inode, oe);
+					/*
+					 * Once for our lookup and once for the
+					 * ordered extents tree.
+					 */
+					btrfs_put_ordered_extent(oe);
+					btrfs_put_ordered_extent(oe);
 					goto unlock_err;
 				}
 			}
 
-			ret = btrfs_add_ordered_extent_dio(inode, start,
-					   block_start, len, len, type);
-			if (ret) {
-				free_extent_map(em);
-				goto unlock_err;
-			}
 			goto unlock;
 		}
 	}