From 659192e668d32f9bfde04ddff6c1914ee24183ce Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Tue, 31 May 2022 16:06:35 +0100 Subject: [PATCH] btrfs: add assertions when deleting batches of delayed items There are a few impossible cases that btrfs_batch_delete_items() tries to deal with: 1) Getting a path pointing to a NULL leaf; 2) The leaf slot is pointing beyond the last item in the leaf; 3) We can't find a single item to delete. The first case is impossible because the given path was returned by a successful call to btrfs_search_slot(). Replace the BUG_ON() with an ASSERT for this. The second case is impossible because we are always called when a delayed item matches an item in the given leaf. So add an ASSERT() for that and if that condition is not satisfied, trigger a warning and return an error. The third case is impossible exactly because of the same reason as the second case. The given delayed item matches one item in the leaf, so we know that our batch always has at least one item. Add an ASSERT to check that, trigger a warning if that expectation fails and return an error. Reviewed-by: Anand Jain Reviewed-by: Nikolay Borisov Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/delayed-inode.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c index 748bf6b0d860..1dc4ebba04f5 100644 --- a/fs/btrfs/delayed-inode.c +++ b/fs/btrfs/delayed-inode.c @@ -797,20 +797,23 @@ static int btrfs_batch_delete_items(struct btrfs_trans_handle *trans, struct btrfs_delayed_item *item) { struct btrfs_delayed_item *curr, *next; - struct extent_buffer *leaf; + struct extent_buffer *leaf = path->nodes[0]; struct btrfs_key key; struct list_head head; int nitems, i, last_item; int ret = 0; - BUG_ON(!path->nodes[0]); - - leaf = path->nodes[0]; + ASSERT(leaf != NULL); i = path->slots[0]; last_item = btrfs_header_nritems(leaf) - 1; - if (i > last_item) - return -ENOENT; /* FIXME: Is errno suitable? */ + /* + * Our caller always gives us a path pointing to an existing item, so + * this can not happen. + */ + ASSERT(i <= last_item); + if (WARN_ON(i > last_item)) + return -ENOENT; next = item; INIT_LIST_HEAD(&head); @@ -837,8 +840,13 @@ static int btrfs_batch_delete_items(struct btrfs_trans_handle *trans, btrfs_item_key_to_cpu(leaf, &key, i); } - if (!nitems) - return 0; + /* + * Our caller always gives us a path pointing to an existing item, so + * this can not happen. + */ + ASSERT(nitems >= 1); + if (nitems < 1) + return -ENOENT; ret = btrfs_del_items(trans, root, path, path->slots[0], nitems); if (ret)