Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   bingjingc <bingjingc@synology.com>
To:     josef@toxicpanda.com, dsterba@suse.com, quwenruo@cn.fujitsu.com,
        clm@fb.com, linux-btrfs@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     bingjingc@synology.com, cccheng@synology.com, robbieko@synology.com
Subject: [PATCH] btrfs: fix a potential hole-punching failure
Date:   Wed, 24 Mar 2021 17:31:10 +0800
Message-Id: <1616578270-7365-1-git-send-email-bingjingc@synology.com>
Precedence: bulk

From: BingJing Chang <bingjingc@synology.com>

In commit d77815461f04 ("btrfs: Avoid trucating page or punching hole in
a already existed hole."), existed holes can be skipped by calling
find_first_non_hole() to adjust *start and *len. However, if the given
len is invalid and large, when an EXTENT_MAP_HOLE extent is found, the
*len will not be set to zero because (em->start + em->len) is less than
(*start + *len). Then the ret will be 1 but the *len will not be set to
0. The propagated non-zero ret will result in fallocate failure.

In the while-loop of btrfs_replace_file_extents(), len is not updated
every time before it calls find_first_non_hole(). That is, if the last
file extent in the given hole-punching range has been dropped but
btrfs_drop_extents() fails with -ENOSPC (btrfs_drop_extents() runs out
of reserved space of the given transaction), the problem can happen.
After it calls find_first_non_hole(), the cur_offset will be adjusted
to be larger than or equal to end. However, since the len is not set to
zero. The break-loop condition (ret && !len) will not meet. After it
leaves the while-loop, uncleared ret will result in fallocate failure.

We're not able to construct a reproducible way to let
btrfs_drop_extents() fails with -ENOSPC after it drops the last file
extent but with remaining holes. However, it's quite easy to fix. We
just need to update and check the len every time before we call
find_first_non_hole(). To make the while loop more readable, we also
pull the variable updates to the bottom of loop like this:
while (cur_offset < end) {
        ...
        // update cur_offset & len
        // advance cur_offset & len in hole-punching case if needed
}

Reported-by: Robbie Ko <robbieko@synology.com>
Fixes: d77815461f04 ("btrfs: Avoid trucating page or punching hole in a
already existed hole.")
Reviewed-by: Robbie Ko <robbieko@synology.com>
Reviewed-by: Chung-Chiang Cheng <cccheng@synology.com>
Signed-off-by: BingJing Chang <bingjingc@synology.com>
---
 fs/btrfs/file.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 0e155f0..dccb017 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -2735,8 +2735,6 @@ int btrfs_replace_file_extents(struct inode *inode, struct btrfs_path *path,
 			extent_info->file_offset += replace_len;
 		}
 
-		cur_offset = drop_args.drop_end;
-
 		ret = btrfs_update_inode(trans, root, BTRFS_I(inode));
 		if (ret)
 			break;
@@ -2756,7 +2754,9 @@ int btrfs_replace_file_extents(struct inode *inode, struct btrfs_path *path,
 		BUG_ON(ret);	/* shouldn't happen */
 		trans->block_rsv = rsv;
 
-		if (!extent_info) {
+		cur_offset = drop_args.drop_end;
+		len = end - cur_offset;
+		if (!extent_info && len) {
 			ret = find_first_non_hole(BTRFS_I(inode), &cur_offset,
 						  &len);
 			if (unlikely(ret < 0))
-- 
2.7.4