[v2] btrfs: handle failure of add_pending_csums
diff mbox

Message ID 1515401983-28451-1-git-send-email-nborisov@suse.com
State New
Headers show

Commit Message

Nikolay Borisov Jan. 8, 2018, 8:59 a.m. UTC
add_pending_csums was added as part of the new data=ordered implementation in
e6dcd2dc9c48 ("Btrfs: New data=ordered implementation"). Even back then it
called the btrfs_csum_file_blocks which can fail but it never bothered handling
the failure. In ENOMEM situation this could lead to the filesystem failing to
write the checksums for a particular extent and not detect this. On read this
could lead to the filesystem erroring out due to crc mismatch. Fix it by
propagating failure from add_pending_csums and handling them

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---

V2: 
 Moves the if/ret part after setting ->adding_csums to false. 

 fs/btrfs/inode.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Josef Bacik Jan. 26, 2018, 2:28 p.m. UTC | #1
On Mon, Jan 08, 2018 at 10:59:43AM +0200, Nikolay Borisov wrote:
> add_pending_csums was added as part of the new data=ordered implementation in
> e6dcd2dc9c48 ("Btrfs: New data=ordered implementation"). Even back then it
> called the btrfs_csum_file_blocks which can fail but it never bothered handling
> the failure. In ENOMEM situation this could lead to the filesystem failing to
> write the checksums for a particular extent and not detect this. On read this
> could lead to the filesystem erroring out due to crc mismatch. Fix it by
> propagating failure from add_pending_csums and handling them
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Josef Bacik <jbacik@fb.com>

Thanks,

Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba Feb. 5, 2018, 5:56 p.m. UTC | #2
On Fri, Jan 26, 2018 at 09:28:58AM -0500, Josef Bacik wrote:
> On Mon, Jan 08, 2018 at 10:59:43AM +0200, Nikolay Borisov wrote:
> > add_pending_csums was added as part of the new data=ordered implementation in
> > e6dcd2dc9c48 ("Btrfs: New data=ordered implementation"). Even back then it
> > called the btrfs_csum_file_blocks which can fail but it never bothered handling
> > the failure. In ENOMEM situation this could lead to the filesystem failing to
> > write the checksums for a particular extent and not detect this. On read this
> > could lead to the filesystem erroring out due to crc mismatch. Fix it by
> > propagating failure from add_pending_csums and handling them
> > 
> > Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> 
> Reviewed-by: Josef Bacik <jbacik@fb.com>

Reviewed-by: David Sterba <dsterba@suse.com>

and added to next, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index eebfe2615428..029399593049 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2039,12 +2039,15 @@  static noinline int add_pending_csums(struct btrfs_trans_handle *trans,
 			     struct inode *inode, struct list_head *list)
 {
 	struct btrfs_ordered_sum *sum;
+	int ret;
 
 	list_for_each_entry(sum, list, list) {
 		trans->adding_csums = true;
-		btrfs_csum_file_blocks(trans,
+		ret = btrfs_csum_file_blocks(trans,
 		       BTRFS_I(inode)->root->fs_info->csum_root, sum);
 		trans->adding_csums = false;
+		if (ret)
+			return ret;
 	}
 	return 0;
 }
@@ -3058,7 +3061,11 @@  static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
 		goto out;
 	}
 
-	add_pending_csums(trans, inode, &ordered_extent->list);
+	ret = add_pending_csums(trans, inode, &ordered_extent->list);
+	if (ret) {
+		btrfs_abort_transaction(trans, ret);
+		goto out;
+	}
 
 	btrfs_ordered_update_i_size(inode, 0, ordered_extent);
 	ret = btrfs_update_inode_fallback(trans, root, inode);