Re: [PATCH] btrfs: fix a potential double put bug and some related use-after-free bugs
From: David Sterba <hidden>
Date: 2021-08-18 10:49:09
Also in:
lkml
On Wed, Aug 18, 2021 at 05:15:18PM +0800, Wentao_Liang wrote:
In line 2955 (#1), "btrfs_put_block_group(cache);" drops the reference to
cache and may cause the cache to be released. However, in line 3014, the
cache is dropped again by the same put function (#4). Double putting the
cache can lead to an incorrect reference count.
Furthermore, according to the definition of btrfs_put_block_group() in fs/
btrfs/block-group.c, if the reference count of the cache is one at the
first put, it will be freed by kfree(). Using it again may result in the
use-after-free flaw. In fact, after the first put (line 2955), the cache
is also accessed in a few places (#2, #3), e.g., lines 2967, 2973, 2974,
….
We believe that the first put of the cache is unnecessary (#1).
We can fix the above bugs by removing the redundant
"btrfs_put_block_group(cache);" in line 2955 (#1).
2951 if (!list_empty(&cache->io_list)) {
...
2955 btrfs_put_block_group(cache);
//#1 the first drop to cache (unnecessary)
...
2957 }
...
2967 cache_save_setup(cache, trans, path); //#2 use the cache
...
2972 //#3 use the cache several times
2973 if (!ret && cache->disk_cache_state == BTRFS_DC_SETUP) {
2974 cache->io_ctl.inode = NULL;
2975 ret = btrfs_write_out_cache(trans, cache, path);
2976 if (ret == 0 && cache->io_ctl.inode) {
2977 num_started++;
2978 should_put = 0;
2979 list_add_tail(&cache->io_list, io);
2980 } else {
...
2985 ret = 0;
2986 }
2987 }
2988 if (!ret) {
2989 ret = update_block_group_item(trans, path, cache);
...
3003 if (ret == -ENOENT) {
...
3006 ret = update_block_group_item(trans, path, cache);
3007 }
...
3010 }
3011
...
3013 if (should_put)
3014 btrfs_put_block_group(cache);
//#4 the second drop to cache
Signed-off-by: Wentao_Liang <redacted>Have you tested the patch? It hits and assertion in the first test btrfs/001: 3856 btrfs_remove_free_space_cache(block_group); 3857 ASSERT(block_group->cached != BTRFS_CACHE_STARTED); 3858 ASSERT(list_empty(&block_group->dirty_list)); 3859 ASSERT(list_empty(&block_group->io_list)); 3860 ASSERT(list_empty(&block_group->bg_list)); 3861 ASSERT(refcount_read(&block_group->refs) == 1); [ 23.884253] BTRFS: device fsid e3a2f9ca-8015-49d5-a97b-efe96f575b17 devid 1 transid 5 /dev/vdb scanned by mkfs.btrfs (410) [ 23.934083] BTRFS info (device vdb): disk space caching is enabled [ 23.936108] BTRFS info (device vdb): has skinny extents [ 23.937813] BTRFS info (device vdb): flagging fs with big metadata feature [ 23.946471] BTRFS info (device vdb): checking UUID tree [ 23.971424] assertion failed: refcount_read(&block_group->refs) == 1, in fs/btrfs/block-group.c:3861 [ 23.974909] ------------[ cut here ]------------ [ 23.976185] kernel BUG at fs/btrfs/ctree.h:3451! [ 23.977357] invalid opcode: 0000 [#1] PREEMPT SMP [ 23.978532] CPU: 2 PID: 440 Comm: umount Not tainted 5.14.0-rc6-default+ #1544 [ 23.980270] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a-rebuilt.opensuse.org 04/01/2014 [ 23.982832] RIP: 0010:assertfail.constprop.0+0x18/0x1a [btrfs] [ 23.984353] Code: 24 08 44 8b 14 24 45 8b b0 a0 23 00 00 e9 d6 90 fd ff 89 f1 48 c7 c2 a7 b1 56 c0 48 89 fe 48 c7 c7 08 8d 57 c0 e8 7b 70 22 de <0f> 0b be a1 00 00 00 48 c7 c7 cf b1 56 c0 e8 d5 ff ff ff ba 9d 0a [ 23.988793] RSP: 0018:ffffb66f80cd7dc0 EFLAGS: 00010246 [ 23.990122] RAX: 0000000000000058 RBX: ffff95f195660000 RCX: 0000000000000000 [ 23.991810] RDX: 0000000000000000 RSI: ffffffff9eee124c RDI: 00000000ffffffff [ 23.993402] RBP: ffff95f19890a800 R08: 0000000000000001 R09: 0000000000000001 [ 23.995056] R10: 0000000000000000 R11: 0000000000000001 R12: ffff95f195660110 [ 23.996627] R13: ffff95f195660160 R14: ffff95f19890a988 R15: dead000000000100 [ 23.998232] FS: 00007f7b47a70800(0000) GS:ffff95f1fda00000(0000) knlGS:0000000000000000 [ 23.999968] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 24.001233] CR2: 00007f7b47bff000 CR3: 00000000188f8004 CR4: 0000000000170ea0 [ 24.002830] Call Trace: [ 24.003485] btrfs_free_block_groups.cold+0x33/0x66 [btrfs] [ 24.004762] close_ctree+0x319/0x35a [btrfs] [ 24.005756] ? fsnotify_destroy_marks+0x24/0x130 [ 24.006859] generic_shutdown_super+0x69/0x100 [ 24.008030] kill_anon_super+0x14/0x30 [ 24.008943] btrfs_kill_super+0x12/0x20 [btrfs] [ 24.009978] deactivate_locked_super+0x2c/0xa0 [ 24.010971] cleanup_mnt+0x144/0x1b0 [ 24.011816] task_work_run+0x59/0xa0 [ 24.012618] exit_to_user_mode_loop+0xe7/0xf0 [ 24.013640] exit_to_user_mode_prepare+0xaf/0xf0 [ 24.014682] syscall_exit_to_user_mode+0x19/0x50 [ 24.015762] do_syscall_64+0x4a/0x90 [ 24.016649] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 24.017793] RIP: 0033:0x7f7b47c934db [ 24.018634] Code: 29 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 90 f3 0f 1e fa 31 f6 e9 05 00 00 00 0f 1f 44 00 00 f3 0f 1e fa b8 a6 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 05 c3 0f 1f 40 00 48 8b 15 61 29 0c 00 f7 d8 [ 24.022729] RSP: 002b:00007ffde0d8a208 EFLAGS: 00000246 ORIG_RAX: 00000000000000a6 [ 24.024435] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 00007f7b47c934db [ 24.026067] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 000055c86a6b4b90 [ 24.027681] RBP: 000055c86a6b4960 R08: 0000000000000000 R09: 00007ffde0d88f90 [ 24.029220] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 [ 24.030744] R13: 000055c86a6b4b90 R14: 000055c86a6b4a70 R15: 000055c86a6b4960 [ 24.032314] Modules linked in: btrfs blake2b_generic libcrc32c crc32c_intel xor zstd_decompress zstd_compress xxhash lzo_compress lzo_decompress raid6_pq loop [ 24.035509] ---[ end trace 92be60e092599330 ]--- [ 24.036528] RIP: 0010:assertfail.constprop.0+0x18/0x1a [btrfs] [ 24.037856] Code: 24 08 44 8b 14 24 45 8b b0 a0 23 00 00 e9 d6 90 fd ff 89 f1 48 c7 c2 a7 b1 56 c0 48 89 fe 48 c7 c7 08 8d 57 c0 e8 7b 70 22 de <0f> 0b be a1 00 00 00 48 c7 c7 cf b1 56 c0 e8 d5 ff ff ff ba 9d 0a [ 24.042025] RSP: 0018:ffffb66f80cd7dc0 EFLAGS: 00010246 [ 24.043239] RAX: 0000000000000058 RBX: ffff95f195660000 RCX: 0000000000000000 [ 24.044797] RDX: 0000000000000000 RSI: ffffffff9eee124c RDI: 00000000ffffffff [ 24.046335] RBP: ffff95f19890a800 R08: 0000000000000001 R09: 0000000000000001 [ 24.047877] R10: 0000000000000000 R11: 0000000000000001 R12: ffff95f195660110 [ 24.049318] R13: ffff95f195660160 R14: ffff95f19890a988 R15: dead000000000100 [ 24.050829] FS: 00007f7b47a70800(0000) GS:ffff95f1fda00000(0000) knlGS:0000000000000000 [ 24.052660] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 24.053956] CR2: 00007f7b47bff000 CR3: 00000000188f8004 CR4: 0000000000170ea0
quoted hunk ↗ jump to hunk
--- fs/btrfs/block-group.c | 1 - 1 file changed, 1 deletion(-)diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c index 9e7d9d0c763d..c510c821b1be 100644 --- a/fs/btrfs/block-group.c +++ b/fs/btrfs/block-group.c@@ -2953,7 +2953,6 @@ int btrfs_write_dirty_block_groups(struct btrfs_trans_handle *trans) spin_unlock(&cur_trans->dirty_bgs_lock); list_del_init(&cache->io_list); btrfs_wait_cache_io(trans, cache, path); - btrfs_put_block_group(cache); spin_lock(&cur_trans->dirty_bgs_lock); }-- 2.25.1