linux-kernel - [PATCH] btrfs: fix a potential hole-punching failure

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1616578270-7365-1-git-send-email-bingjingc@synology.com>
Date:   Wed, 24 Mar 2021 17:31:10 +0800
From:   bingjingc <bingjingc@...ology.com>
To:     josef@...icpanda.com, dsterba@...e.com, quwenruo@...fujitsu.com,
        clm@...com, linux-btrfs@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     bingjingc@...ology.com, cccheng@...ology.com, robbieko@...ology.com
Subject: [PATCH] btrfs: fix a potential hole-punching failure

From: BingJing Chang <bingjingc@...ology.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@...ology.com>
Fixes: d77815461f04 ("btrfs: Avoid trucating page or punching hole in a
already existed hole.")
Reviewed-by: Robbie Ko <robbieko@...ology.com>
Reviewed-by: Chung-Chiang Cheng <cccheng@...ology.com>
Signed-off-by: BingJing Chang <bingjingc@...ology.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