| Age | Commit message (Collapse) | Author |
|---|
|
During relocation recovery, each fs root gets a reference to its relocation
root. If loading or adding a later root fails, or if the first transaction
commit fails, btrfs_recover_relocation() jumps to out_unset before
merge_reloc_roots() and clean_dirty_subvols().
put_reloc_control() drops the list-owned relocation root references, but it
does not clear fs_root->reloc_root or drop the references owned by those
pointers. Mount cleanup only drops them when BTRFS_FS_ERROR is set, so an
error such as -ENOMEM while processing a later root can leave references
behind.
Keep temporary references to the fs roots associated during recovery. On
failure, clear their reloc_root pointers and drop the corresponding
references. Once the first transaction commit succeeds, drop only the
temporary fs root references and let the normal merge and cleanup paths
handle the relocation roots.
Fault injection on a pending-relocation image confirmed the cleanup gap.
With an injected first-commit failure, 25 fs roots had reloc_root set with
fs_error=0. With this fix, the same failure path drops that count to 0
before mount fails.
Fixes: f44deb7442ed ("btrfs: hold a ref on the root->reloc_root")
CC: stable@vger.kernel.org
Signed-off-by: Guanghui Yang <3497809730@qq.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
On a zoned FS, btrfs_delayed_refs_rsv_refill() returns -EAGAIN whenever
the over-committed metadata plus the zone_unusable bytes exceeds the
usable size in a metadata block-group to avoid heavy over-commit of
metadata and early ENOSPC in one transaction.
If this happens while doing reclaim, the transaction is getting aborted.
Treat -EAGAIN as a soft, retryable condition in case of block-group
reclaim.
Reported-by: Damien Le Moal <dlemoal@kernel.org>
Fixes: 7bcb04de982f ("btrfs: zoned: cap delayed refs metadata reservation to avoid overcommit")
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The loop intends to copy the data in chunks up to 1M but we allocate the
pages array for the entire length and don't cap it to 1M. Fix this by
computing 'nr_pages' using 'copy_len' instead of 'length'.
While at it, also make 'nr_pages' and 'copy_len' const, as they never
change, to make the code more clear.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
When we set a root's reloc_root to NULL, we do it like this:
static void clear_reloc_root(struct btrfs_root *root)
{
root->reloc_root = NULL;
/*
* Need barrier to ensure clear_bit() only happens after
* root->reloc_root = NULL. Pairs with have_reloc_root().
*/
smp_wmb();
clear_bit(BTRFS_ROOT_DEAD_RELOC_TREE, &root->state);
}
So that a NULL reloc_root is always seen before seeing that the bit
BTRFS_ROOT_DEAD_RELOC_TREE was cleared.
But on the read side we have:
static bool reloc_root_is_dead(const struct btrfs_root *root)
{
smp_rmb();
if (test_bit(BTRFS_ROOT_DEAD_RELOC_TREE, &root->state))
return true;
return false;
}
And then callers of reloc_root_is_dead() access root->reloc_root.
Because the read memory barrier is placed before testing the bit, the CPU
is completely free to speculatively reorder those two loads. It can read
root->reloc_root before it actually checks the dead tree bit.
Sashiko reported this as an existing problem in another patch review, see
the link in the Link tag below.
Fix this by moving the read memory barrier to happen after testing the bit
and update the comment to reflect current reality.
Link: https://sashiko.dev/#/patchset/cf84f1a217c719e25b6b69e4298dd7afd36c9427.1781194426.git.fdmanana%40suse.com
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
__add_reloc_root() allocates a mapping_node before inserting it into
rc->reloc_root_tree. If rb_simple_insert() finds an existing entry, it
returns the existing rb_node and leaves the newly allocated node unlinked.
The error path then returns -EEXIST without freeing the new node. Since
the node was never inserted into reloc_root_tree, the later cleanup in
put_reloc_control() cannot find it either.
Free the newly allocated node before returning -EEXIST.
The callers currently assert that -EEXIST should not happen, so this is a
defensive cleanup for an unexpected duplicate insert path. If the path is
ever reached, the local allocation should still be released.
Fixes: 57a304cfd43b ("btrfs: do not panic in __add_reloc_root")
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Guanghui Yang <3497809730@qq.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Commit a6908f88c9da ("btrfs: validate data reloc tree file extent item
members") introduced extra checks on file extent items for data reloc
inodes, but it checked the file extent offset without checking if the file
extent is inlined.
This can lead to either false alerts (as the offset member is inside the
inlined data) or even reading beyond the item range.
This has already triggered a warning in a syzbot report.
Although the root fix is to avoid compression for data reloc inodes, for
the sake of consistency, reject inlined file extents first.
Fixes: a6908f88c9da ("btrfs: validate data reloc tree file extent item members")
CC: stable@vger.kernel.org
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
If the root we got has zero root refs in its root item, we are resetting
the root's ->reloc_root without using barriers like we do everywhere else.
Sashiko complained about this while reviewing another patch, and it's
correct (see the Link tag below).
Also, we should not clear BTRFS_ROOT_DEAD_RELOC_TREE from the root unless
the root points to the reloc root we have.
Fix this by using clear_reloc_root(), which issues the memory barrier
after setting the root's ->reloc_root to NULL and before clearing the bit
BTRFS_ROOT_DEAD_RELOC_TREE from the root.
Link: https://sashiko.dev/#/patchset/cf84f1a217c719e25b6b69e4298dd7afd36c9427.1781194426.git.fdmanana%40suse.com
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
If during relocation we fail in insert_dirty_subvol() because
btrfs_update_reloc_root() returned an error, we will leave a root's
reloc_root field pointing to a reloc root that was freed instead of NULL,
resulting later in a use-after-free, or double free attempt during
unmount.
The sequence of steps is this:
1) During relocation the call to btrfs_update_reloc_root() in
insert_dirty_subvol() fails, so insert_dirty_subvol() returns the
error to merge_reloc_root() without adding the root to the list
rc->dirty_subvol_roots;
2) Then merge_reloc_root() aborts the current transaction because
insert_dirty_subvol() returned an error;
3) Up the call chain, merge_reloc_roots() gets the error, adds the
reloc root for root X to the local reloc_roots list and jumps to the
'out' label, where it calls free_reloc_roots() to free all the reloc
roots in the local reloc_roots list. This frees the reloc root for
root X;
4) We go up the call chain to relocate_block_group() which calls
clean_dirty_subvols() to go over dirty roots and set their
->reloc_root field to NULL, but root X is not in the dirty_subvol_roots
list, so its ->reloc_root still points to a reloc root;
5) Relocation finishes, with an error and a transaction abort, but the
->reloc_root field for root X still points to the reloc root that was
freed in step 3;
6) When unmounting the fs we end up calling:
btrfs_free_fs_roots()
btrfs_drop_and_free_fs_root()
--> calls btrfs_put_root() against root X's ->reloc_root
which is not NULL and points to the already freed
reloc root in step 4 above
Resulting in a use-after-free to a double free attempt.
Syzbot reported this with the following dmesg/syslog:
[ 106.004389][ T5339] BTRFS error (device loop0 state A): Transaction aborted (error -5)
[ 106.014266][ T5339] BTRFS: error (device loop0 state A) in merge_reloc_root:1655: errno=-5 IO failure
[ 106.021891][ T1061] BTRFS error (device loop0 state A): error while writing out transaction: -5
[ 106.026964][ T1061] BTRFS warning (device loop0 state A): Skipping commit of aborted transaction.
[ 106.033807][ T5340] BTRFS error (device loop0 state A): bdev /dev/loop0 errs: wr 3, rd 0, flush 0, corrupt 0, gen 0
[ 106.039265][ T1061] BTRFS: error (device loop0 state A) in cleanup_transaction:2067: errno=-5 IO failure
[ 106.044382][ T5339] BTRFS info (device loop0 state EA): forced readonly
[ 106.074329][ T5339] BTRFS: error (device loop0 state EA) in merge_reloc_roots:1887: errno=-5 IO failure
[ 106.081004][ T5356] BTRFS info (device loop0 state EA): scrub: started on devid 1
[ 106.085611][ T5339] BTRFS info (device loop0 state EA): balance: ended with status: -30
[ 106.089517][ T5356] BTRFS info (device loop0 state EA): scrub: not finished on devid 1 with status: -30
[ 106.662365][ T5338] BTRFS info (device loop0 state EA): last unmount of filesystem 3a375e4e-b156-4d76-a2ad-16e198ce1409
[ 106.682946][ T5338] ==================================================================
[ 106.686574][ T5338] BUG: KASAN: slab-use-after-free in btrfs_put_root+0x2f/0x250
[ 106.690090][ T5338] Write of size 4 at addr ffff88803f978630 by task syz.0.0/5338
[ 106.693173][ T5338]
[ 106.694279][ T5338] CPU: 0 UID: 0 PID: 5338 Comm: syz.0.0 Not tainted syzkaller #0 PREEMPT(full)
[ 106.694293][ T5338] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 106.694300][ T5338] Call Trace:
[ 106.694308][ T5338] <TASK>
[ 106.694314][ T5338] dump_stack_lvl+0xe8/0x150
[ 106.694331][ T5338] print_address_description+0x55/0x1e0
[ 106.694343][ T5338] ? btrfs_put_root+0x2f/0x250
[ 106.694358][ T5338] print_report+0x58/0x70
[ 106.694368][ T5338] kasan_report+0x117/0x150
[ 106.694384][ T5338] ? btrfs_put_root+0x2f/0x250
[ 106.694399][ T5338] kasan_check_range+0x264/0x2c0
[ 106.694416][ T5338] btrfs_put_root+0x2f/0x250
[ 106.694430][ T5338] btrfs_drop_and_free_fs_root+0x160/0x210
[ 106.694447][ T5338] btrfs_free_fs_roots+0x2f9/0x3c0
[ 106.694464][ T5338] ? __pfx_btrfs_free_fs_roots+0x10/0x10
[ 106.694479][ T5338] ? free_root_pointers+0x5bf/0x5f0
[ 106.694494][ T5338] close_ctree+0x798/0x12d0
[ 106.694511][ T5338] ? __pfx_close_ctree+0x10/0x10
[ 106.694526][ T5338] ? _raw_spin_unlock_irqrestore+0x74/0x80
[ 106.694599][ T5338] ? rcu_preempt_deferred_qs_irqrestore+0x906/0xbc0
[ 106.694620][ T5338] ? __rcu_read_unlock+0x83/0xe0
[ 106.694636][ T5338] ? btrfs_put_super+0x48/0x1c0
[ 106.694652][ T5338] ? __pfx_btrfs_put_super+0x10/0x10
[ 106.694667][ T5338] generic_shutdown_super+0x13d/0x2d0
[ 106.694682][ T5338] kill_anon_super+0x3b/0x70
[ 106.694695][ T5338] btrfs_kill_super+0x41/0x50
[ 106.694710][ T5338] deactivate_locked_super+0xbc/0x130
[ 106.694722][ T5338] cleanup_mnt+0x437/0x4d0
[ 106.694736][ T5338] ? _raw_spin_unlock_irq+0x23/0x50
[ 106.694752][ T5338] task_work_run+0x1d9/0x270
[ 106.694769][ T5338] ? __pfx_task_work_run+0x10/0x10
[ 106.694784][ T5338] ? do_raw_spin_unlock+0x4d/0x210
[ 106.694802][ T5338] do_exit+0x70f/0x22c0
[ 106.694817][ T5338] ? trace_irq_disable+0x3b/0x140
[ 106.694835][ T5338] ? __pfx_do_exit+0x10/0x10
[ 106.694848][ T5338] ? preempt_schedule_thunk+0x16/0x30
[ 106.694863][ T5338] ? preempt_schedule_common+0x82/0xd0
[ 106.694878][ T5338] ? preempt_schedule_thunk+0x16/0x30
[ 106.694892][ T5338] do_group_exit+0x21b/0x2d0
[ 106.694906][ T5338] ? entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 106.694918][ T5338] __x64_sys_exit_group+0x3f/0x40
[ 106.694932][ T5338] x64_sys_call+0x221a/0x2240
[ 106.694944][ T5338] do_syscall_64+0x174/0x580
[ 106.694954][ T5338] ? clear_bhb_loop+0x40/0x90
[ 106.694967][ T5338] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 106.694978][ T5338] RIP: 0033:0x7f958ef9ce59
[ 106.694988][ T5338] Code: Unable to access opcode bytes at 0x7f958ef9ce2f.
[ 106.694994][ T5338] RSP: 002b:00007fffd4058318 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
[ 106.695008][ T5338] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f958ef9ce59
[ 106.695015][ T5338] RDX: 00007f958c3f8000 RSI: 0000000000000000 RDI: 0000000000000000
[ 106.695022][ T5338] RBP: 0000000000000003 R08: 0000000000000000 R09: 00007f958f1e73e0
[ 106.695028][ T5338] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
[ 106.695034][ T5338] R13: 00007f958f1e73e0 R14: 0000000000000003 R15: 00007fffd40583d0
[ 106.695046][ T5338] </TASK>
[ 106.695050][ T5338]
[ 106.821635][ T5338] Allocated by task 1061:
[ 106.823446][ T5338] kasan_save_track+0x3e/0x80
[ 106.825498][ T5338] __kasan_kmalloc+0x93/0xb0
[ 106.827381][ T5338] __kmalloc_cache_noprof+0x31c/0x660
[ 106.829525][ T5338] btrfs_alloc_root+0x75/0x930
[ 106.831458][ T5338] read_tree_root_path+0x127/0xb00
[ 106.833556][ T5338] btrfs_read_tree_root+0x34/0x60
[ 106.835553][ T5338] create_reloc_root+0x6b3/0xcb0
[ 106.837556][ T5338] btrfs_init_reloc_root+0x2ec/0x4b0
[ 106.839557][ T5338] record_root_in_trans+0x2ab/0x350
[ 106.841685][ T5338] btrfs_record_root_in_trans+0x15c/0x180
[ 106.844237][ T5338] start_transaction+0x39c/0x1820
[ 106.846638][ T5338] btrfs_finish_one_ordered+0x88e/0x2680
[ 106.849436][ T5338] btrfs_work_helper+0x37b/0xc20
[ 106.851549][ T5338] process_scheduled_works+0xb5d/0x1860
[ 106.853807][ T5338] worker_thread+0xa53/0xfc0
[ 106.855773][ T5338] kthread+0x389/0x470
[ 106.857548][ T5338] ret_from_fork+0x514/0xb70
[ 106.859493][ T5338] ret_from_fork_asm+0x1a/0x30
[ 106.861504][ T5338]
[ 106.862527][ T5338] Freed by task 5339:
[ 106.864224][ T5338] kasan_save_track+0x3e/0x80
[ 106.866180][ T5338] kasan_save_free_info+0x46/0x50
[ 106.868371][ T5338] __kasan_slab_free+0x5c/0x80
[ 106.870462][ T5338] kfree+0x1c5/0x640
[ 106.872180][ T5338] __del_reloc_root+0x341/0x3b0
[ 106.874290][ T5338] free_reloc_roots+0x5f/0x90
[ 106.876282][ T5338] merge_reloc_roots+0x73f/0x8a0
[ 106.878489][ T5338] relocate_block_group+0xbcc/0xe70
[ 106.880742][ T5338] do_nonremap_reloc+0xa8/0x5b0
[ 106.882885][ T5338] btrfs_relocate_block_group+0x7e6/0xc40
[ 106.885336][ T5338] btrfs_relocate_chunk+0x115/0x820
[ 106.887502][ T5338] __btrfs_balance+0x1db0/0x2ae0
[ 106.889543][ T5338] btrfs_balance+0xaf3/0x11b0
[ 106.891456][ T5338] btrfs_ioctl_balance+0x3d3/0x610
[ 106.893672][ T5338] __se_sys_ioctl+0xfc/0x170
[ 106.895530][ T5338] do_syscall_64+0x174/0x580
[ 106.897518][ T5338] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 106.900101][ T5338]
[ 106.901123][ T5338] The buggy address belongs to the object at ffff88803f978000
[ 106.901123][ T5338] which belongs to the cache kmalloc-4k of size 4096
[ 106.906907][ T5338] The buggy address is located 1584 bytes inside of
[ 106.906907][ T5338] freed 4096-byte region [ffff88803f978000, ffff88803f979000)
[ 106.912980][ T5338]
[ 106.914022][ T5338] The buggy address belongs to the physical page:
[ 106.916716][ T5338] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x3f978
[ 106.920390][ T5338] head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
[ 106.923834][ T5338] flags: 0x4fff00000000040(head|node=1|zone=1|lastcpupid=0x7ff)
[ 106.927104][ T5338] page_type: f5(slab)
[ 106.928898][ T5338] raw: 04fff00000000040 ffff88801ac42140 dead000000000122 0000000000000000
[ 106.932507][ T5338] raw: 0000000000000000 0000000800040004 00000000f5000000 0000000000000000
[ 106.936193][ T5338] head: 04fff00000000040 ffff88801ac42140 dead000000000122 0000000000000000
[ 106.939856][ T5338] head: 0000000000000000 0000000800040004 00000000f5000000 0000000000000000
[ 106.943601][ T5338] head: 04fff00000000003 fffffffffffffe01 00000000ffffffff 00000000ffffffff
[ 106.947268][ T5338] head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000008
[ 106.950988][ T5338] page dumped because: kasan: bad access detected
[ 106.953710][ T5338] page_owner tracks the page as allocated
[ 106.956198][ T5338] page last allocated via order 3, migratetype Unmovable, gfp_mask 0xd2820(GFP_ATOMIC|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC), pid 24, tgid 24 (kworker/u4:2), ts 105728970387, free_ts 29540875453
[ 106.964984][ T5338] post_alloc_hook+0x22d/0x280
[ 106.966956][ T5338] get_page_from_freelist+0x2593/0x2610
[ 106.969307][ T5338] __alloc_frozen_pages_noprof+0x18d/0x380
[ 106.971839][ T5338] allocate_slab+0x77/0x660
[ 106.973709][ T5338] refill_objects+0x339/0x3d0
[ 106.975696][ T5338] __pcs_replace_empty_main+0x321/0x720
[ 106.978136][ T5338] __kmalloc_node_track_caller_noprof+0x572/0x7b0
[ 106.981009][ T5338] __alloc_skb+0x2c1/0x7d0
[ 106.982983][ T5338] nsim_dev_trap_report_work+0x29a/0xb90
[ 106.985356][ T5338] process_scheduled_works+0xb5d/0x1860
[ 106.987710][ T5338] worker_thread+0xa53/0xfc0
[ 106.989847][ T5338] kthread+0x389/0x470
[ 106.991727][ T5338] ret_from_fork+0x514/0xb70
[ 106.993722][ T5338] ret_from_fork_asm+0x1a/0x30
[ 106.995900][ T5338] page last free pid 77 tgid 77 stack trace:
[ 106.998479][ T5338] __free_frozen_pages+0xc1c/0xd30
[ 107.000819][ T5338] vfree+0x1d1/0x2f0
[ 107.002631][ T5338] delayed_vfree_work+0x55/0x80
[ 107.004848][ T5338] process_scheduled_works+0xb5d/0x1860
[ 107.007366][ T5338] worker_thread+0xa53/0xfc0
[ 107.009388][ T5338] kthread+0x389/0x470
[ 107.011177][ T5338] ret_from_fork+0x514/0xb70
[ 107.013313][ T5338] ret_from_fork_asm+0x1a/0x30
[ 107.015454][ T5338]
[ 107.016460][ T5338] Memory state around the buggy address:
[ 107.019052][ T5338] ffff88803f978500: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 107.022691][ T5338] ffff88803f978580: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 107.026264][ T5338] >ffff88803f978600: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 107.029721][ T5338] ^
[ 107.032062][ T5338] ffff88803f978680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 107.035547][ T5338] ffff88803f978700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 107.038865][ T5338] ==================================================================
Fix this by resetting a root's ->reloc_root if we get an error while
trying to merge a reloc root.
Reported-by: syzbot+b3d472d13f9d7bf20669@syzkaller.appspotmail.com
Link: https://lore.kernel.org/linux-btrfs/6a1ebde9.c1435f33.112120.0176.GAE@google.com/
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Decentralize transaction aborts in create_reloc_root(), so that it is
obvious which call failed and what caused the transaction abort.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
If we have an unexpected reloc_root for our root, we jump to the out label
but never drop the reference we obtained for root, resulting in a leak.
Add a missing btrfs_put_root() call.
Fixes: 24213fa46c70 ("btrfs: do proper error handling in merge_reloc_roots")
Reviewed-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
If we get a failure during relocation, before we update all the extent
buffers that have file extent items pointing to extents from the block
group being relocated, we can trigger a user-after-free on the reloc
control structure (fs_info->reloc_control) if we have a concurrent task
that is COWing a subvolume leaf.
This happens like this:
1) Relocation of data block group X starts;
2) Relocation changes its state to UPDATE_DATA_PTRS;
3) A task doing a rename for example, COWs leaf A from a subvolume tree
and ends up at btrfs_reloc_cow_block() and extracts fs_info->reloc_ctl
into a local variable, which then passes to replace_file_extents();
4) The relocation task gets an error and under the label 'out_put_bg' in
btrfs_relocate_block_group() calls free_reloc_control(), which frees
the reloc control structure that the rename task is using;
5) The rename task triggers a use-after-free on the reloc control
structure that was just freed.
Syzbot reported this recently, with the following stack trace:
[ 88.389822][ T5325] BTRFS error (device loop0 state A): Transaction aborted (error -5)
[ 88.389842][ T5325] BTRFS: error (device loop0 state A) in cleanup_transaction:2067: errno=-5 IO failure
[ 88.389864][ T5325] BTRFS info (device loop0 state EA): forced readonly
[ 88.392277][ T5324] BTRFS: error (device loop0 state EA) in btrfs_sync_log:3572: errno=-5 IO failure
[ 88.396630][ T5325] BTRFS info (device loop0 state EA): balance: ended with status: -5
[ 88.400135][ T5346] ==================================================================
[ 88.400148][ T5346] BUG: KASAN: slab-use-after-free in replace_file_extents+0x85f/0x1590
[ 88.400288][ T5346] Read of size 8 at addr ffff888012312010 by task syz.0.0/5346
[ 88.400299][ T5346]
[ 88.400306][ T5346] CPU: 0 UID: 0 PID: 5346 Comm: syz.0.0 Not tainted syzkaller #0 PREEMPT(full)
[ 88.400319][ T5346] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 88.400325][ T5346] Call Trace:
[ 88.400331][ T5346] <TASK>
[ 88.400336][ T5346] dump_stack_lvl+0xe8/0x150
[ 88.400351][ T5346] print_address_description+0x55/0x1e0
[ 88.400364][ T5346] ? replace_file_extents+0x85f/0x1590
[ 88.400378][ T5346] print_report+0x58/0x70
[ 88.400389][ T5346] kasan_report+0x117/0x150
[ 88.400405][ T5346] ? replace_file_extents+0x85f/0x1590
[ 88.400420][ T5346] replace_file_extents+0x85f/0x1590
[ 88.400440][ T5346] ? __pfx_replace_file_extents+0x10/0x10
[ 88.400452][ T5346] ? update_ref_for_cow+0xa71/0x1270
[ 88.400473][ T5346] btrfs_force_cow_block+0xa4d/0x2450
[ 88.400492][ T5346] ? __pfx_btrfs_force_cow_block+0x10/0x10
[ 88.400508][ T5346] ? __pfx_btrfs_get_32+0x10/0x10
[ 88.400523][ T5346] btrfs_cow_block+0x3c4/0xa90
[ 88.400542][ T5346] push_leaf_left+0x2ac/0x4a0
[ 88.400561][ T5346] split_leaf+0xd16/0x12e0
[ 88.400574][ T5346] ? btrfs_bin_search+0x924/0xc70
[ 88.400592][ T5346] ? __pfx_split_leaf+0x10/0x10
[ 88.400602][ T5346] ? leaf_space_used+0x177/0x1e0
[ 88.400618][ T5346] ? btrfs_leaf_free_space+0x14a/0x2f0
[ 88.400634][ T5346] btrfs_search_slot+0x2641/0x2d20
[ 88.400654][ T5346] ? __pfx_btrfs_search_slot+0x10/0x10
[ 88.400669][ T5346] ? rcu_is_watching+0x15/0xb0
[ 88.400681][ T5346] ? trace_kmem_cache_alloc+0x29/0xe0
[ 88.400694][ T5346] btrfs_insert_empty_items+0x9c/0x190
[ 88.400711][ T5346] btrfs_insert_inode_ref+0x229/0xcb0
[ 88.400724][ T5346] ? __pfx_btrfs_insert_inode_ref+0x10/0x10
[ 88.400736][ T5346] ? __pfx_btrfs_qgroup_convert_reserved_meta+0x10/0x10
[ 88.400751][ T5346] ? btrfs_record_root_in_trans+0x124/0x180
[ 88.400767][ T5346] ? start_transaction+0x8a0/0x1820
[ 88.400778][ T5346] ? btrfs_set_inode_index+0x5e/0x100
[ 88.400787][ T5346] btrfs_rename2+0x17bb/0x40d0
[ 88.400800][ T5346] ? check_noncircular+0xda/0x150
[ 88.400814][ T5346] ? add_lock_to_list+0xc7/0x100
[ 88.400828][ T5346] ? __pfx_btrfs_rename2+0x10/0x10
[ 88.400842][ T5346] ? lockdep_hardirqs_on+0x7a/0x110
[ 88.400901][ T5346] ? lock_acquire+0x221/0x350
[ 88.400915][ T5346] ? down_write_nested+0x174/0x210
[ 88.400931][ T5346] ? __pfx_down_write_nested+0x10/0x10
[ 88.400941][ T5346] ? do_raw_spin_unlock+0x4d/0x210
[ 88.400952][ T5346] ? try_break_deleg+0x5b/0x180
[ 88.400963][ T5346] ? __pfx_btrfs_rename2+0x10/0x10
[ 88.400973][ T5346] vfs_rename+0xa96/0xeb0
[ 88.400992][ T5346] ? __pfx_vfs_rename+0x10/0x10
[ 88.401010][ T5346] ovl_fill_super+0x46b7/0x5e20
[ 88.401030][ T5346] ? __pfx_ovl_fill_super+0x10/0x10
[ 88.401042][ T5346] ? xas_create+0x1902/0x1b90
[ 88.401060][ T5346] ? __pfx___mutex_trylock_common+0x10/0x10
[ 88.401076][ T5346] ? trace_contention_end+0x3d/0x140
[ 88.401094][ T5346] ? shrinker_register+0x124/0x230
[ 88.401111][ T5346] ? __mutex_unlock_slowpath+0x1be/0x6f0
[ 88.401127][ T5346] ? shrinker_register+0x61/0x230
[ 88.401143][ T5346] ? __pfx___mutex_lock+0x10/0x10
[ 88.401158][ T5346] ? __pfx___mutex_unlock_slowpath+0x10/0x10
[ 88.401177][ T5346] ? __raw_spin_lock_init+0x45/0x100
[ 88.401196][ T5346] ? sget_fc+0x962/0xa40
[ 88.401208][ T5346] ? __pfx_set_anon_super_fc+0x10/0x10
[ 88.401222][ T5346] ? __pfx_ovl_fill_super+0x10/0x10
[ 88.401241][ T5346] get_tree_nodev+0xbb/0x150
[ 88.401257][ T5346] vfs_get_tree+0x92/0x2a0
[ 88.401272][ T5346] do_new_mount+0x341/0xd30
[ 88.401283][ T5346] ? apparmor_capable+0x126/0x170
[ 88.401301][ T5346] ? __pfx_do_new_mount+0x10/0x10
[ 88.401311][ T5346] ? ns_capable+0x89/0xe0
[ 88.401322][ T5346] ? path_mount+0x690/0x10e0
[ 88.401333][ T5346] ? user_path_at+0xd4/0x160
[ 88.401346][ T5346] __se_sys_mount+0x31d/0x420
[ 88.401358][ T5346] ? __pfx___se_sys_mount+0x10/0x10
[ 88.401370][ T5346] ? __x64_sys_mount+0x20/0xc0
[ 88.401381][ T5346] ? entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 88.401391][ T5346] do_syscall_64+0x15f/0xf80
[ 88.401403][ T5346] ? trace_irq_disable+0x3b/0x140
[ 88.401413][ T5346] ? clear_bhb_loop+0x40/0x90
[ 88.401421][ T5346] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 88.401429][ T5346] RIP: 0033:0x7fa1ff79ce59
[ 88.401436][ T5346] Code: ff c3 66 (...)
[ 88.401443][ T5346] RSP: 002b:00007fa2005affe8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
[ 88.401456][ T5346] RAX: ffffffffffffffda RBX: 00007fa1ffa16180 RCX: 00007fa1ff79ce59
[ 88.401464][ T5346] RDX: 0000200000000100 RSI: 0000200000002240 RDI: 0000000000000000
[ 88.401474][ T5346] RBP: 00007fa1ff832d6f R08: 0000200000000440 R09: 0000000000000000
[ 88.401481][ T5346] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
[ 88.401488][ T5346] R13: 00007fa1ffa16218 R14: 00007fa1ffa16180 R15: 00007ffc734fba78
[ 88.401500][ T5346] </TASK>
[ 88.401506][ T5346]
[ 88.401510][ T5346] Allocated by task 5325:
[ 88.401516][ T5346] kasan_save_track+0x3e/0x80
[ 88.401529][ T5346] __kasan_kmalloc+0x93/0xb0
[ 88.401542][ T5346] __kmalloc_cache_noprof+0x31c/0x660
[ 88.401554][ T5346] btrfs_relocate_block_group+0x217/0xc40
[ 88.401568][ T5346] btrfs_relocate_chunk+0x115/0x820
[ 88.401577][ T5346] __btrfs_balance+0x1db0/0x2ae0
[ 88.401587][ T5346] btrfs_balance+0xaf3/0x11b0
[ 88.401596][ T5346] btrfs_ioctl_balance+0x3d3/0x610
[ 88.401612][ T5346] __se_sys_ioctl+0xfc/0x170
[ 88.401626][ T5346] do_syscall_64+0x15f/0xf80
[ 88.401640][ T5346] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 88.401650][ T5346]
[ 88.401653][ T5346] Freed by task 5325:
[ 88.401659][ T5346] kasan_save_track+0x3e/0x80
[ 88.401671][ T5346] kasan_save_free_info+0x46/0x50
[ 88.401680][ T5346] __kasan_slab_free+0x5c/0x80
[ 88.401692][ T5346] kfree+0x1c5/0x640
[ 88.401703][ T5346] btrfs_relocate_block_group+0x95d/0xc40
[ 88.401715][ T5346] btrfs_relocate_chunk+0x115/0x820
[ 88.401724][ T5346] __btrfs_balance+0x1db0/0x2ae0
[ 88.401733][ T5346] btrfs_balance+0xaf3/0x11b0
[ 88.401742][ T5346] btrfs_ioctl_balance+0x3d3/0x610
[ 88.401757][ T5346] __se_sys_ioctl+0xfc/0x170
[ 88.401770][ T5346] do_syscall_64+0x15f/0xf80
[ 88.401785][ T5346] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 88.401795][ T5346]
[ 88.401798][ T5346] The buggy address belongs to the object at ffff888012312000
[ 88.401798][ T5346] which belongs to the cache kmalloc-2k of size 2048
[ 88.401807][ T5346] The buggy address is located 16 bytes inside of
[ 88.401807][ T5346] freed 2048-byte region [ffff888012312000, ffff888012312800)
[ 88.401819][ T5346]
[ 88.401822][ T5346] The buggy address belongs to the physical page:
[ 88.401829][ T5346] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x12310
[ 88.401840][ T5346] head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
[ 88.401849][ T5346] flags: 0xfff00000000040(head|node=0|zone=1|lastcpupid=0x7ff)
[ 88.401860][ T5346] page_type: f5(slab)
[ 88.401871][ T5346] raw: 00fff00000000040 ffff88801ac42000 dead000000000100 dead000000000122
[ 88.401881][ T5346] raw: 0000000000000000 0000000800080008 00000000f5000000 0000000000000000
[ 88.401892][ T5346] head: 00fff00000000040 ffff88801ac42000 dead000000000100 dead000000000122
[ 88.401902][ T5346] head: 0000000000000000 0000000800080008 00000000f5000000 0000000000000000
[ 88.401913][ T5346] head: 00fff00000000003 fffffffffffffe01 00000000ffffffff 00000000ffffffff
[ 88.401923][ T5346] head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000008
[ 88.401929][ T5346] page dumped because: kasan: bad access detected
[ 88.401935][ T5346] page_owner tracks the page as allocated
[ 88.401941][ T5346] page last allocated via order 3, migratetype Unmovable, gfp_mask 0xd20c0(__GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC), pid 9, tgid 9 (kworker/0:0), ts 83905464494, free_ts 83674944822
[ 88.401961][ T5346] post_alloc_hook+0x231/0x280
[ 88.401975][ T5346] get_page_from_freelist+0x24ba/0x2540
[ 88.401990][ T5346] __alloc_frozen_pages_noprof+0x18d/0x380
[ 88.402004][ T5346] allocate_slab+0x77/0x660
[ 88.402019][ T5346] refill_objects+0x339/0x3d0
[ 88.402033][ T5346] __pcs_replace_empty_main+0x321/0x720
[ 88.402043][ T5346] __kmalloc_node_track_caller_noprof+0x572/0x7b0
[ 88.402055][ T5346] __alloc_skb+0x2c1/0x7d0
[ 88.402067][ T5346] mld_newpack+0x14c/0xc90
[ 88.402080][ T5346] add_grhead+0x5a/0x2a0
[ 88.402093][ T5346] add_grec+0x1452/0x1740
[ 88.402105][ T5346] mld_ifc_work+0x6e6/0xe70
[ 88.402116][ T5346] process_scheduled_works+0xb5d/0x1860
[ 88.402127][ T5346] worker_thread+0xa53/0xfc0
[ 88.402138][ T5346] kthread+0x389/0x470
[ 88.402150][ T5346] ret_from_fork+0x514/0xb70
[ 88.402161][ T5346] page last free pid 5282 tgid 5282 stack trace:
[ 88.402168][ T5346] __free_frozen_pages+0xbc7/0xd30
[ 88.402180][ T5346] __slab_free+0x274/0x2c0
[ 88.402191][ T5346] qlist_free_all+0x99/0x100
[ 88.402201][ T5346] kasan_quarantine_reduce+0x148/0x160
[ 88.402211][ T5346] __kasan_slab_alloc+0x22/0x80
[ 88.402221][ T5346] __kmalloc_cache_noprof+0x2ba/0x660
[ 88.402231][ T5346] kernfs_fop_open+0x3f0/0xda0
[ 88.402253][ T5346] do_dentry_open+0x785/0x14e0
[ 88.402262][ T5346] vfs_open+0x3b/0x340
[ 88.402270][ T5346] path_openat+0x2e08/0x3860
[ 88.402281][ T5346] do_file_open+0x23e/0x4a0
[ 88.402292][ T5346] do_sys_openat2+0x113/0x200
[ 88.402300][ T5346] __x64_sys_openat+0x138/0x170
[ 88.402309][ T5346] do_syscall_64+0x15f/0xf80
[ 88.402326][ T5346] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 88.402336][ T5346]
[ 88.402339][ T5346] Memory state around the buggy address:
[ 88.402345][ T5346] ffff888012311f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 88.402352][ T5346] ffff888012311f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 88.402359][ T5346] >ffff888012312000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 88.402365][ T5346] ^
[ 88.402370][ T5346] ffff888012312080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 88.402380][ T5346] ffff888012312100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 88.402385][ T5346] ==================================================================
Fix this by:
1) Making the reloc control structure ref counted;
2) Make revery place that access fs_info->reloc_ctl outside the relocation
code, which at the moment it's only replace_file_extents() and
btrfs_init_reloc_root(), get a reference count on the structure.
There's also btrfs_update_reloc_root() that is called outside the
relocation code, but this case is safe because it's only called in
the transaction commit path while under the fs_info->reloc_mutex
protection, but nevertheless grab a reference to make the code more
consistent and avoid false alerts from AI reviews;
3) Add a spinlock to protect fs_info->reloc_ctl, since we can not take the
fs_info->reloc_mutex as that would cause a deadlock since that lock is
taken in the transaction commit path. That spinlock is taken before
setting fs_info->reloc_ctl to an allocated structure, setting it to
NULL and reading fs_info->reloc_ctl;
4) Make sure the structure is freed only when its reference count drops to
zero.
Reported-by: syzbot+0eea49bba18051dea35e@syzkaller.appspotmail.com
Link: https://lore.kernel.org/linux-btrfs/6a1df323.bb0696ed.125a22.000a.GAE@google.com/
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
There's no point in having the WARN_ON(1) inside the if statement for the
unexpected error. Move it into the if statement's condition, which brings
a couple benefits:
1) It marks the branch as unlikely, hinting the compiler to generate
better code;
2) The WARN_ON() produces a stack trace after the dumped leaf and error
message which can hide that more important information in case we get
a truncated dmesg/syslog.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
It does not make sense for the single caller to have the responsability
to lock the relocation mutex before calling the function and then have
the function to assert the lock is held. As this is a function in
relocation.c, move the locking details into it.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
For all local indicator variables do simple switch to bool, done on all
files.
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Convert more multiplications of sectorsize or nodesize to use the
shifts. The remaining cases are multiplications by constants that
compiler can optimize by itself, and in tests.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Extent buffer pages allocated by alloc_extent_buffer() are attached to
btree_inode->i_mapping (the buffer_tree path), reach the LRU, and are
served by the btree_migrate_folio aops in fs/btrfs/disk-io.c. They are
migratable in practice once their owning extent buffer hits refs == 1,
which happens naturally. The buddy allocator classifies them by GFP,
however, and bare GFP_NOFS lands them in MIGRATE_UNMOVABLE pageblocks.
The result: every btree_inode page we read in pins an unmovable pageblock
from the page-superblock allocator's perspective, even though the page
itself can be moved.
Have each caller of btrfs_alloc_page_array, btrfs_alloc_folio_array,
and alloc_eb_folio_array pass in the full GFP mask directly, instead
of having the functions calculate it from boolean flags.
The alloc_extent_buffer call site passes GFP_NOFS | __GFP_NOFAIL |
__GFP_MOVABLE. All other call sites pass plain GFP_NOFS.
Three categories of caller stay on bare GFP_NOFS, deliberately:
- alloc_dummy_extent_buffer / btrfs_clone_extent_buffer: the
resulting eb is EXTENT_BUFFER_UNMAPPED, folio->mapping stays NULL,
the folios never enter LRU, never get migrate_folio aops. Tagging
them __GFP_MOVABLE would violate the page allocator's migrability
contract and they would defeat compaction in MOVABLE pageblocks
where isolate_migratepages_block skips non-LRU non-movable_ops
pages outright.
- btrfs_alloc_page_array callers in fs/btrfs/raid56.c (stripe
pages), fs/btrfs/inode.c (encoded reads), fs/btrfs/ioctl.c (io_uring
encoded reads), fs/btrfs/relocation.c (relocation buffers): same
contract violation. raid56 stripe_pages additionally persist in
the stripe cache (RBIO_CACHE_SIZE=1024) well beyond a single I/O,
so they are not transient enough to hand-wave the contract.
- btrfs_alloc_folio_array caller in fs/btrfs/scrub.c (stripe
folios): same -- stripe->folios[] are private buffers freed via
folio_put in release_scrub_stripe.
This change targets the dominant fragmentation source observed on the
page-superblock series: ~28 GB of btree_inode pages parked across
many tainted superpageblocks on a 250 GB test system with btrfs root,
preventing 1 GiB hugepage allocation from those regions. With the
movable hint, those pages now land in MOVABLE pageblocks where the
existing background defragger drains them through the standard
PB_has_movable gate, no LRU-sample fallback needed.
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Rik van Riel <riel@surriel.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
get_new_location() uses BUG_ON() to crash the kernel if the file extent
item it looks up has any of offset, compression, encryption, or
other_encoding set non-zero. The data reloc inode is only written by
relocation's own paths and the four fields are always 0 in what the
kernel writes:
- insert_prealloc_file_extent() memsets the stack item to zero and
only fills in type, disk_bytenr, disk_num_bytes and num_bytes, so
offset/compression/encryption/other_encoding stay 0.
- insert_ordered_extent_file_extent() copies oe->compress_type into
the file extent's compression field, but the data reloc inode is
created with BTRFS_INODE_NOCOMPRESS so compress_type is always 0;
encryption and other_encoding are reserved-and-zero in btrfs.
A non-zero value here means the leaf decoded from disk does not match
what the kernel wrote, i.e. on-disk corruption. A malformed image
reaches this code via balance and panics the kernel.
A previous attempt to enforce all four constraints in tree-checker's
check_extent_data_item() was merged as commit 7d0ee95979e9 ("btrfs:
validate data reloc tree file extent item members in tree-checker")
and then reverted by commit 1c034697fcaa after btrfs/061 produced
false positives on arm64 with 64K pages. The reason: relocation
writeback legitimately produces REG file_extent_items with offset != 0
in the data reloc tree. When an ordered extent covers only the back
portion of an underlying PREALLOC (num_bytes < ram_bytes on the input
file_extent), insert_ordered_extent_file_extent() inserts a REG with
offset = oe->offset
num_bytes = oe->num_bytes
ram_bytes preserved from the original PREALLOC,
and this item can reach disk if a transaction commit fires while it
is present in the leaf.
The four fields belong in different layers:
- compression, encryption and other_encoding are universal
invariants for every item in the data reloc tree, regardless of
cluster geometry. Enforce them in tree-checker's
check_extent_data_item() so a corrupt leaf is rejected at read
time.
- offset is only an invariant at the cluster-boundary keys that
get_new_location() searches (the key is computed as
src_disk_bytenr - reloc_block_group_start). Partial-PREALLOC
writebacks legitimately place REG items at non-boundary keys with
offset != 0; tree-checker cannot reject these. The cluster-
boundary item is always written by either
insert_prealloc_file_extent() (offset=0 by memset) or by the
front portion of a partial writeback (offset=0 by construction),
so a non-zero offset there is corruption.
Enforce the universal invariants in check_extent_data_item() with a
file_extent_err() rejection. Convert the BUG_ON() in
get_new_location() to a -EUCLEAN return paired with btrfs_print_leaf()
and btrfs_err() so the offending leaf is logged. The caller in
replace_file_extents() already handles non-zero returns from
get_new_location() by breaking out of the loop without aborting the
transaction.
Suggested-by: Qu Wenruo <wqu@suse.com>
Suggested-by: David Sterba <dsterba@suse.com>
Reported-by: syzbot+3e20d8f3d41bac5dc9a2@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3e20d8f3d41bac5dc9a2
Signed-off-by: Teng Liu <27rabbitlt@gmail.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The iterator is used only once and within build_backref_tree() so we can
avoid one allocation and place it on stack.
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The fs_info is available everywhere and we don't need to store it inside
a structure that is used within one function only, which is
build_backref_tree(). The size of btrfs_backref_iter is now 48 bytes.
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Do a few more (trivial) conversions that started in commit 69050f8d6d075d
("treewide: Replace kmalloc with kmalloc_obj for non-scalar types").
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
In add_remap_tree_entries(), we only process a certain number of entries
at a time, meaning we may need to loop.
But because we weren't checking the return value of btrfs_insert_empty_items()
within the loop, this meant that if the last iteration of the loop
succeeded but a previous iteration failed, we were erroneously returning
0.
Fix this by breaking the loop early if btrfs_insert_empty_items() fails.
Fixes: b56f35560b82 ("btrfs: handle setting up relocation of block group with remap-tree")
Signed-off-by: Mark Harmstone <mark@harmstone.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Currently for tree block readahead we never pass a
btrfs_tree_parent_check with @has_first_key set.
Without @has_first_key set, btrfs will skip the following extra
checks:
- Header generation check
This is a minor one.
- Empty leaf/node checks
This is more serious, for certain trees like the csum tree, they are
allowed to be empty, thus an empty leaf can pass the tree checker.
But if there is a parent node for such an empty leaf, it indicates
corruption.
Without @has_first_key set, we can no longer detect such a problem.
In fact there is already a fuzzed image report that a corrupted csum
leaf which has zero nritems but still has a parent node can trigger
a BUG_ON() during csum deletion.
However there are only two call sites of btrfs_readahead_tree_block():
- Inside relocate_tree_blocks()
At this call site we are trying to grab the first key of the tree
block, thus we are not able to pass a @first_key parameter.
- Inside btrfs_readahead_node_child()
This is the more common call site, where we have the parent node and
want to readahead the child tree blocks.
In this case we can easily grab the node key and pass it for checks.
Add a new parameter @first_key to btrfs_readahead_tree_block() and pass
the node key to it inside btrfs_readahead_node_child().
This should plug the gap in empty leaf detection during readahead.
Link: https://lore.kernel.org/linux-btrfs/20260409071255.3358044-1-gality369@gmail.com/
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
If one of the calls made by do_remap_reloc_trans() fails, we can leave
the remap tree in an inconsistent state. Abort the transaction if this
happens, to prevent the corrupt state from reaching the disk.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Mark Harmstone <mark@harmstone.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
If the call to btrfs_reserve_extent() in do_remap_reloc_trans() returns
a smaller extent than we asked for, currently we're not undoing the
bytes_may_use change that we made. Fix this by calling
btrfs_space_info_update_bytes_may_use() again for the difference.
Fixes: fd6594b1446c ("btrfs: replace identity remaps with actual remaps when doing relocations")
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Mark Harmstone <mark@harmstone.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
If the call to btrfs_reserve_extent() in move_existing_remap() returns a
smaller extent than we asked for, currently we're not undoing the
bytes_may_use change that we made. Fix this by calling
btrfs_space_info_update_bytes_may_use() again for the difference.
Fixes: bbea42dfb91f ("btrfs: move existing remaps before relocating block group")
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Mark Harmstone <mark@harmstone.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We join a transaction with the goal of catching the current transaction
and then commit it to get rid of pinned extents and reclaim free space,
but a join can create a new transaction if there isn't any running, and if
right before we did the join the current transaction happened to be
committed by someone else (like the transaction kthread for example),
we end up starting and committing a new transaction, causing rotation of
the super block backup roots besides extra and useless IO.
So instead of doing a transaction join followed by a commit, use the
helper btrfs_commit_current_transaction() which ensures no transaction is
created if there isn't any running.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We have several places that call extent_buffer_uptodate() after reading a
tree block with read_tree_block(), but that is redundant since we already
call extent_buffer_uptodate() in the call chain of read_tree_block():
read_tree_block()
btrfs_read_extent_buffer()
read_extent_buffer_pages()
returns -EIO if extent_buffer_uptodate() returns false
So remove those redundant checks.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux
Pull btrfs fixes from David Sterba:
"Another batch of fixes for problems that have been identified by tools
analyzing code or by fuzzing. Most of them are short, two patches fix
the same thing in many places so the diffs are bigger.
- handle potential NULL pointer errors after attempting to read
extent and checksum trees
- prevent ENOSPC when creating many qgroups by ioctls in the same
transaction
- encoded write ioctl fixes (with 64K page and 4K block size):
- fix unexpected bio length
- do not let compressed bios and pages interfere with page cache
- compression fixes on setups with 64K page and 4K block size: fix
folio length assertions (zstd and lzo)
- remap tree fixes:
- make sure to hold block group reference while moving it
- handle early exit when moving block group to unused list
- handle deleted subvolumes with inconsistent state of deletion
progress"
* tag 'for-7.0-rc4-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux:
btrfs: reject root items with drop_progress and zero drop_level
btrfs: check block group before marking it unused in balance_remap_chunks()
btrfs: hold block group reference during entire move_existing_remap()
btrfs: fix an incorrect ASSERT() condition inside lzo_decompress_bio()
btrfs: fix an incorrect ASSERT() condition inside zstd_decompress_bio()
btrfs: do not touch page cache for encoded writes
btrfs: fix a bug that makes encoded write bio larger than expected
btrfs: reserve enough transaction items for qgroup ioctls
btrfs: check for NULL root after calls to btrfs_csum_root()
btrfs: check for NULL root after calls to btrfs_extent_root()
|
|
There is a potential use-after-free in move_existing_remap(): we're calling
btrfs_put_block_group() on dest_bg, then passing it to
btrfs_add_block_group_free_space() a few lines later.
Fix this by getting the BG at the start of the function and putting it
near the end. This also means we're not doing a lookup twice for the
same thing.
Reported-by: Chris Mason <clm@fb.com>
Link: https://lore.kernel.org/linux-btrfs/20260125123908.2096548-1-clm@meta.com/
Fixes: bbea42dfb91f ("btrfs: move existing remaps before relocating block group")
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Mark Harmstone <mark@harmstone.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
btrfs_csum_root() can return a NULL pointer in case the root we are
looking for is not in the rb tree that tracks roots. So add checks to
every caller that is missing such check to log a message and return
an error.
Reported-by: Chris Mason <clm@meta.com>
Link: https://lore.kernel.org/linux-btrfs/20260208161657.3972997-1-clm@meta.com/
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
btrfs_extent_root() can return a NULL pointer in case the root we are
looking for is not in the rb tree that tracks roots. So add checks to
every caller that is missing such check to log a message and return
an error. The same applies to callers of btrfs_block_group_root(),
since it calls btrfs_extent_root().
Reported-by: Chris Mason <clm@meta.com>
Link: https://lore.kernel.org/linux-btrfs/20260208161657.3972997-1-clm@meta.com/
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux
Pull btrfs fixes from David Sterba:
- detect possible file name hash collision earlier so it does not lead
to transaction abort
- handle b-tree leaf overflows when snapshotting a subvolume with set
received UUID, leading to transaction abort
- in zoned mode, reorder relocation block group initialization after
the transaction kthread start
- fix orphan cleanup state tracking of subvolume, this could lead to
invalid dentries under some conditions
- add locking around updates of dynamic reclain state update
- in subpage mode, add missing RCU unlock when trying to releae extent
buffer
- remap tree fixes:
- add missing description strings for the newly added remap tree
- properly update search key when iterating backrefs
* tag 'for-7.0-rc3-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux:
btrfs: remove duplicated definition of btrfs_printk_in_rcu()
btrfs: remove unnecessary transaction abort in the received subvol ioctl
btrfs: abort transaction on failure to update root in the received subvol ioctl
btrfs: fix transaction abort on set received ioctl due to item overflow
btrfs: fix transaction abort when snapshotting received subvolumes
btrfs: fix transaction abort on file creation due to name hash collision
btrfs: read key again after incrementing slot in move_existing_remaps()
btrfs: add missing RCU unlock in error path in try_release_subpage_extent_buffer()
btrfs: set BTRFS_ROOT_ORPHAN_CLEANUP during subvol create
btrfs: zoned: move btrfs_zoned_reserve_data_reloc_bg() after kthread start
btrfs: hold space_info->lock when clearing periodic reclaim ready
btrfs: print-tree: add remap tree definitions
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux
Pull btrfs fixes from David Sterba:
"One-liner or short fixes for minor/moderate problems reported recently:
- fixes or level adjustments of error messages
- fix leaked transaction handles after aborted transactions, when
using the remap tree feature
- fix a few leaked chunk maps after errors
- fix leaked page array in io_uring encoded read if an error occurs
and the 'finished' is not called
- fix double release of reserved extents when doing a range COW
- don't commit super block when the filesystem is in shutdown state
- fix squota accounting condition when checking members vs parent
usage
- other error handling fixes"
* tag 'for-7.0-rc2-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux:
btrfs: check block group lookup in remove_range_from_remap_tree()
btrfs: fix transaction handle leaks in btrfs_last_identity_remap_gone()
btrfs: fix chunk map leak in btrfs_map_block() after btrfs_translate_remap()
btrfs: fix chunk map leak in btrfs_map_block() after btrfs_chunk_map_num_copies()
btrfs: fix compat mask in error messages in btrfs_check_features()
btrfs: print correct subvol num if active swapfile prevents deletion
btrfs: fix warning in scrub_verify_one_metadata()
btrfs: fix objectid value in error message in check_extent_data_ref()
btrfs: fix incorrect key offset in error message in check_dev_extent_item()
btrfs: fix error message order of parameters in btrfs_delete_delayed_dir_index()
btrfs: don't commit the super block when unmounting a shutdown filesystem
btrfs: free pages on error in btrfs_uring_read_extent()
btrfs: fix referenced/exclusive check in squota_check_parent_usage()
btrfs: remove pointless WARN_ON() in cache_save_setup()
btrfs: convert log messages to error level in btrfs_replay_log()
btrfs: remove btrfs_handle_fs_error() after failure to recover log trees
btrfs: remove redundant warning message in btrfs_check_uuid_tree()
btrfs: change warning messages to error level in open_ctree()
btrfs: fix a double release on reserved extents in cow_one_range()
btrfs: handle discard errors in in btrfs_finish_extent_commit()
|
|
Fix move_existing_remaps() so that if we increment the slot because the
key we encounter isn't a REMAP_BACKREF, we don't reuse the objectid and
offset of the old item.
Link: https://lore.kernel.org/linux-btrfs/20260125123908.2096548-1-clm@meta.com/
Reported-by: Chris Mason <clm@fb.com>
Fixes: bbea42dfb91f ("btrfs: move existing remaps before relocating block group")
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Mark Harmstone <mark@harmstone.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Add a check in remove_range_from_remap_tree() after we call
btrfs_lookup_block_group(), to check if it is NULL. This shouldn't
happen, but if it does we at least get an error rather than a segfault.
Reported-by: Chris Mason <clm@fb.com>
Link: https://lore.kernel.org/linux-btrfs/20260125125129.2245240-1-clm@meta.com/
Fixes: 979e1dc3d69e ("btrfs: handle deletions from remapped block group")
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Mark Harmstone <mark@harmstone.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
btrfs_abort_transaction(), unlike btrfs_commit_transaction(), doesn't
also free the transaction handle. Fix the instances in
btrfs_last_identity_remap_gone() where we're also leaking the
transaction on abort.
Reported-by: Chris Mason <clm@fb.com>
Link: https://lore.kernel.org/linux-btrfs/20260125125129.2245240-1-clm@meta.com/
Fixes: 979e1dc3d69e ("btrfs: handle deletions from remapped block group")
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Mark Harmstone <mark@harmstone.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
This is the result of running the Coccinelle script from
scripts/coccinelle/api/kmalloc_objs.cocci. The script is designed to
avoid scalar types (which need careful case-by-case checking), and
instead replace kmalloc-family calls that allocate struct or union
object instances:
Single allocations: kmalloc(sizeof(TYPE), ...)
are replaced with: kmalloc_obj(TYPE, ...)
Array allocations: kmalloc_array(COUNT, sizeof(TYPE), ...)
are replaced with: kmalloc_objs(TYPE, COUNT, ...)
Flex array allocations: kmalloc(struct_size(PTR, FAM, COUNT), ...)
are replaced with: kmalloc_flex(*PTR, FAM, COUNT, ...)
(where TYPE may also be *VAR)
The resulting allocations no longer return "void *", instead returning
"TYPE *".
Signed-off-by: Kees Cook <kees@kernel.org>
|
|
Add a function btrfs_populate_fully_remapped_bgs_list() which gets
called on mount, which looks for fully remapped block groups
(i.e. identity_remap_count == 0) which haven't yet had their chunk
stripes and device extents removed.
This happens when a filesystem is unmounted while async discard has not
yet finished, as otherwise the data range occupied by the chunk stripes
would be permanently unusable.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Mark Harmstone <mark@harmstone.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Add a function do_remap_tree_reloc(), which does the actual work of
doing a relocation using the remap tree.
In a loop we call do_remap_reloc_trans(), which searches for the first
identity remap for the block group. We call btrfs_reserve_extent() to
find space elsewhere for it, and read the data into memory and write it
to the new location. We then carve out the identity remap and replace it
with an actual remap, which points to the new location in which to look.
Once the last identity remap has been removed we call
last_identity_remap_gone(), which, as with deletions, removes the
chunk's stripes and device extents.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Mark Harmstone <mark@harmstone.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
If when relocating a block group we find that `remap_bytes` > 0 in its
block group item, that means that it has been the destination block
group for another that has been remapped.
We need to search the remap tree for any remap backrefs within this
range, and move the data to a third block group. This is because
otherwise btrfs_translate_remap() could end up following an unbounded
chain of remaps, which would only get worse over time.
We only relocate one block group at a time, so `remap_bytes` will only
ever go down while we are doing this. Once we're finished we set the
REMAPPED flag on the block group, which will permanently prevent any
other data from being moved to within it.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Mark Harmstone <mark@harmstone.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Handle the preliminary work for relocating a block group in a filesystem
with the remap-tree flag set.
If the block group is SYSTEM btrfs_relocate_block_group() proceeds as it
does already, as bootstrapping issues mean that these block groups have
to be processed the existing way. Similarly with METADATA_REMAP blocks, which
are dealt with in a later patch.
Otherwise we walk the free-space tree for the block group in question,
recording any holes. These get converted into identity remaps and placed
in the remap tree, and the block group's REMAPPED flag is set. From now
on no new allocations are possible within this block group, and any I/O
to it will be funnelled through btrfs_translate_remap(). We store the
number of identity remaps in `identity_remap_count`, so that we know
when we've removed the last one and the block group is fully remapped.
The change in btrfs_read_roots() is because data relocations no longer
rely on the data reloc tree as a hidden subvolume in which to do
snapshots.
(Thanks to Sun YangKai for his suggestions.)
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Mark Harmstone <mark@harmstone.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Handle the case where we free an extent from a block group that has the
REMAPPED flag set. Because the remap tree is orthogonal to the extent
tree, for data this may be within any number of identity remaps or
actual remaps. If we're freeing a metadata node, this will be wholly
inside one or the other.
btrfs_remove_extent_from_remap_tree() searches the remap tree for the
remaps that cover the range in question, then calls
remove_range_from_remap_tree() for each one, to punch a hole in the
remap and adjust the free-space tree.
For an identity remap, remove_range_from_remap_tree() will adjust the
block group's `identity_remap_count` if this changes. If it reaches
zero we mark the block group as fully remapped.
For an identity remap, remove_range_from_remap_tree() will adjust the
block group's `identity_remap_count` if this changes. If it reaches
zero we mark the block group as fully remapped.
Fully remapped block groups have their chunk stripes removed and their
device extents freed, which makes the disk space available again to the
chunk allocator. This happens asynchronously: in the cleaner thread for
sync discard and nodiscard, and (in a later patch) in the discard worker
for async discard.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Mark Harmstone <mark@harmstone.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Change btrfs_map_block() so that if the block group has the REMAPPED
flag set, we call btrfs_translate_remap() to obtain a new address.
btrfs_translate_remap() searches the remap tree for a range
corresponding to the logical address passed to btrfs_map_block(). If it
is within an identity remap, this part of the block group hasn't yet
been relocated, and so we use the existing address.
If it is within an actual remap, we subtract the start of the remap
range and add the address of its destination, contained in the item's
payload.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Mark Harmstone <mark@harmstone.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
In many places we have pattern:
ret = ...;
return ret;
This can be simplified to a direct return, removing 'ret' if not
otherwise needed. The places in self tests are not converted so we can
add more test cases without changing surrounding code
(extent-map-tests.c:test_case_4()).
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Many fields of struct btrfs_path are used as booleans but their type is
an unsigned int (of one 1 bit width to save space). Change the type to
bool keeping the :1 suffix so that they combine with the previous u8
fields in order to save space. This makes the code more clear by using
explicit true/false and more in line with the preferred style, preserving
the size of the structure.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Apply the AUTO_KFREE and AUTO_KVFREE macros wherever it makes
sense. Since this macro is expected to improve code readability, it has
been avoided in places where the lifetime of objects wasn't easy to
follow and a cleanup attribute would've made things worse; or when the
cleanup section of a function involved many other things and thus there
was no readability impact anyways. This change has also not been applied
in extremely short functions where readability was clearly not an issue.
Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Change all locations that print a key to use the new macros to print
them in order to ensure a consistent style and avoid repetitive code.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We are using 'ret' and 'err' variables to track return values and errors,
which is pattern that is error prone and we had quite some bugs due to
this pattern in the past.
Simplify this and use a single variable, named 'ret', to track errors and
the return value.
Also rename the variable 'rw' to 'bg_is_ro' which is more meaningful name,
and change its type from int to bool.
Reviewed-by: Boris Burkov <boris@bur.io>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
When starting relocation, at reloc_chunk_start(), if we happen to find
the flag BTRFS_FS_RELOC_RUNNING is already set we return an error
(-EINPROGRESS) to the callers, however the callers call reloc_chunk_end()
which will clear the flag BTRFS_FS_RELOC_RUNNING, which is wrong since
relocation was started by another task and still running.
Finding the BTRFS_FS_RELOC_RUNNING flag already set is an unexpected
scenario, but still our current behaviour is not correct.
Fix this by never calling reloc_chunk_end() if reloc_chunk_start() has
returned an error, which is what logically makes sense, since the general
widespread pattern is to have end functions called only if the counterpart
start functions succeeded. This requires changing reloc_chunk_start() to
clear BTRFS_FS_RELOC_RUNNING if there's a pending cancel request.
Fixes: 907d2710d727 ("btrfs: add cancellable chunk relocation support")
CC: stable@vger.kernel.org # 5.15+
Reviewed-by: Boris Burkov <boris@bur.io>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The unlikely() annotation is a static prediction hint that compiler may
use to reorder code out of hot path. We use it elsewhere (namely
tree-checker.c) for error branches that almost never happen.
Transaction abort is one such error, the btrfs_abort_transaction()
inlines code to check the state and print a warning, this ought to be
out of the hot path.
The most common pattern is when transaction abort is called after
checking a return value and the control flow leads to a quick return.
In other cases it may not be necessary to add unlikely() e.g. when the
function returns anyway or the control flow is not changed noticeably.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
