On Tue, Jul 16, 2019 at 05:00:32PM +0800, Qu Wenruo wrote: > There is one report of fuzzed image which leads to BUG_ON() in > btrfs_delete_delayed_dir_index(). > > Although that fuzzed image can already be addressed by enhanced > extent-tree error handler, it's still better to hunt down more BUG_ON(). > > This patch will hunt down two BUG_ON()s in > btrfs_delete_delayed_dir_index(): > - One for error from btrfs_delayed_item_reserve_metadata() > Instead of BUG_ON(), we output an error message and free the item. > And return the error. > All callers of this function handles the error by aborting current > trasaction. > > - One for possible EEXIST from __btrfs_add_delayed_deletion_item() > That function can return -EEXIST. > We already have a good enough error message for that, only need to > clean up the reserved metadata space and allocated item. > > To help above cleanup, also modifiy __btrfs_remove_delayed_item() called > in btrfs_release_delayed_item(), to skip unassociated item. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=203253 > Signed-off-by: Qu Wenruo <w...@suse.com> > --- > fs/btrfs/delayed-inode.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c > index 43fdb2992956..c4946d58f49b 100644 > --- a/fs/btrfs/delayed-inode.c > +++ b/fs/btrfs/delayed-inode.c > @@ -474,6 +474,9 @@ static void __btrfs_remove_delayed_item(struct > btrfs_delayed_item *delayed_item) > struct rb_root_cached *root; > struct btrfs_delayed_root *delayed_root; > > + /* Not associated with any delayed_node */ > + if (!delayed_item->delayed_node) > + return; > delayed_root = delayed_item->delayed_node->root->fs_info->delayed_root; > > BUG_ON(!delayed_root); > @@ -1525,7 +1528,13 @@ int btrfs_delete_delayed_dir_index(struct > btrfs_trans_handle *trans, > * we have reserved enough space when we start a new transaction, > * so reserving metadata failure is impossible. > */ > - BUG_ON(ret); > + if (ret < 0) { > + btrfs_err(trans->fs_info, > +"metadata reserve fail at %s, should have already reserved space, ret=%d",
I'd rather avoid using the function name in non-debugging messages, let's use eg "metadata reservation failed for delayed dir item deltion", and printing ret here is not necessary as it will be printed in the transaction abort message.
