| Message ID | 59ccc7b41be79e5c3b0f39ad5da6591554927af7.1669647978.git.fdmanana@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: do not BUG_ON() on ENOMEM when dropping extent items for a range | expand |
On Mon, Nov 28, 2022 at 03:07:30PM +0000, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > If we get -ENOMEM while dropping file extent items in a given range, at > btrfs_drop_extents(), due to failure to allocate memory when attempting to > increment the reference count for an extent or drop the reference count, > we handle it with a BUG_ON(). This is excessive, instead we can simply > abort the transaction and return the error to the caller. In fact most > callers of btrfs_drop_extents(), directly or indirectly, already abort > the transaction if btrfs_drop_extents() returns any error. > > Also, we already have error paths at btrfs_drop_extents() that may return > -ENOMEM and in those cases we abort the transaction, like for example > anything that changes the b+tree may return -ENOMEM due to a failure to > allocate a new extent buffer when COWing an existing extent buffer, such > as a call to btrfs_duplicate_item() for example. > > So replace the BUG_ON() calls with proper logic to abort the transaction > and return the error. > > Reported-by: syzbot+0b1fb6b0108c27419f9f@syzkaller.appspotmail.com > Link: https://lore.kernel.org/linux-btrfs/00000000000089773e05ee4b9cb4@google.com/ > Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: Josef Bacik <josef@toxicpanda.com> Thanks, Josef
On Mon, Nov 28, 2022 at 03:07:30PM +0000, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > If we get -ENOMEM while dropping file extent items in a given range, at > btrfs_drop_extents(), due to failure to allocate memory when attempting to > increment the reference count for an extent or drop the reference count, > we handle it with a BUG_ON(). This is excessive, instead we can simply > abort the transaction and return the error to the caller. In fact most > callers of btrfs_drop_extents(), directly or indirectly, already abort > the transaction if btrfs_drop_extents() returns any error. > > Also, we already have error paths at btrfs_drop_extents() that may return > -ENOMEM and in those cases we abort the transaction, like for example > anything that changes the b+tree may return -ENOMEM due to a failure to > allocate a new extent buffer when COWing an existing extent buffer, such > as a call to btrfs_duplicate_item() for example. > > So replace the BUG_ON() calls with proper logic to abort the transaction > and return the error. > > Reported-by: syzbot+0b1fb6b0108c27419f9f@syzkaller.appspotmail.com > Link: https://lore.kernel.org/linux-btrfs/00000000000089773e05ee4b9cb4@google.com/ > Signed-off-by: Filipe Manana <fdmanana@suse.com> Added to misc-next, thanks.
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 448b143a5cb2..91b00eb2440e 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -380,7 +380,10 @@ int btrfs_drop_extents(struct btrfs_trans_handle *trans, args->start - extent_offset, 0, false); ret = btrfs_inc_extent_ref(trans, &ref); - BUG_ON(ret); /* -ENOMEM */ + if (ret) { + btrfs_abort_transaction(trans, ret); + break; + } } key.offset = args->start; } @@ -467,7 +470,10 @@ int btrfs_drop_extents(struct btrfs_trans_handle *trans, key.offset - extent_offset, 0, false); ret = btrfs_free_extent(trans, &ref); - BUG_ON(ret); /* -ENOMEM */ + if (ret) { + btrfs_abort_transaction(trans, ret); + break; + } args->bytes_found += extent_end - key.offset; }