| From c9e1ba8ac97698a06ee9af8a973adf900fbe8ea2 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Tue, 1 Dec 2020 09:53:23 -0500 |
| Subject: btrfs: fix error handling in commit_fs_roots |
| |
| From: Josef Bacik <josef@toxicpanda.com> |
| |
| [ Upstream commit 4f4317c13a40194940acf4a71670179c4faca2b5 ] |
| |
| While doing error injection I would sometimes get a corrupt file system. |
| This is because I was injecting errors at btrfs_search_slot, but would |
| only do it one time per stack. This uncovered a problem in |
| commit_fs_roots, where if we get an error we would just break. However |
| we're in a nested loop, the first loop being a loop to find all the |
| dirty fs roots, and then subsequent root updates would succeed clearing |
| the error value. |
| |
| This isn't likely to happen in real scenarios, however we could |
| potentially get a random ENOMEM once and then not again, and we'd end up |
| with a corrupted file system. Fix this by moving the error checking |
| around a bit to the main loop, as this is the only place where something |
| will fail, and return the error as soon as it occurs. |
| |
| With this patch my reproducer no longer corrupts the file system. |
| |
| Signed-off-by: Josef Bacik <josef@toxicpanda.com> |
| Reviewed-by: David Sterba <dsterba@suse.com> |
| Signed-off-by: David Sterba <dsterba@suse.com> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| fs/btrfs/transaction.c | 11 ++++++----- |
| 1 file changed, 6 insertions(+), 5 deletions(-) |
| |
| diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c |
| index 6af7f2bf92de..fbf93067642a 100644 |
| --- a/fs/btrfs/transaction.c |
| +++ b/fs/btrfs/transaction.c |
| @@ -1319,7 +1319,6 @@ static noinline int commit_fs_roots(struct btrfs_trans_handle *trans) |
| struct btrfs_root *gang[8]; |
| int i; |
| int ret; |
| - int err = 0; |
| |
| spin_lock(&fs_info->fs_roots_radix_lock); |
| while (1) { |
| @@ -1331,6 +1330,8 @@ static noinline int commit_fs_roots(struct btrfs_trans_handle *trans) |
| break; |
| for (i = 0; i < ret; i++) { |
| struct btrfs_root *root = gang[i]; |
| + int ret2; |
| + |
| radix_tree_tag_clear(&fs_info->fs_roots_radix, |
| (unsigned long)root->root_key.objectid, |
| BTRFS_ROOT_TRANS_TAG); |
| @@ -1350,17 +1351,17 @@ static noinline int commit_fs_roots(struct btrfs_trans_handle *trans) |
| root->node); |
| } |
| |
| - err = btrfs_update_root(trans, fs_info->tree_root, |
| + ret2 = btrfs_update_root(trans, fs_info->tree_root, |
| &root->root_key, |
| &root->root_item); |
| + if (ret2) |
| + return ret2; |
| spin_lock(&fs_info->fs_roots_radix_lock); |
| - if (err) |
| - break; |
| btrfs_qgroup_free_meta_all_pertrans(root); |
| } |
| } |
| spin_unlock(&fs_info->fs_roots_radix_lock); |
| - return err; |
| + return 0; |
| } |
| |
| /* |
| -- |
| 2.30.1 |
| |