linux-kernel - [PATCH] btrfs: fix a potential double put bug and some related use-after-free bugs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210818091518.4825-1-Wentao_Liang_g@163.com>
Date:   Wed, 18 Aug 2021 17:15:18 +0800
From:   Wentao_Liang <Wentao_Liang_g@....com>
To:     clm@...com
Cc:     josef@...icpanda.com, dsterba@...e.com,
        linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org,
        Wentao_Liang <Wentao_Liang_g@....com>
Subject: [PATCH] btrfs: fix a potential double put bug and some related use-after-free bugs

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 <Wentao_Liang_g@....com>
---
 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