]> www.infradead.org Git - users/jedix/linux-maple.git/commitdiff
btrfs: use refcount_inc_not_zero in kill_all_nodes
authorJosef Bacik <josef@toxicpanda.com>
Thu, 26 Sep 2019 12:29:32 +0000 (08:29 -0400)
committerDavid Sterba <dsterba@suse.com>
Mon, 18 Nov 2019 11:46:51 +0000 (12:46 +0100)
We hit the following warning while running down a different problem

[ 6197.175850] ------------[ cut here ]------------
[ 6197.185082] refcount_t: underflow; use-after-free.
[ 6197.194704] WARNING: CPU: 47 PID: 966 at lib/refcount.c:190 refcount_sub_and_test_checked+0x53/0x60
[ 6197.521792] Call Trace:
[ 6197.526687]  __btrfs_release_delayed_node+0x76/0x1c0
[ 6197.536615]  btrfs_kill_all_delayed_nodes+0xec/0x130
[ 6197.546532]  ? __btrfs_btree_balance_dirty+0x60/0x60
[ 6197.556482]  btrfs_clean_one_deleted_snapshot+0x71/0xd0
[ 6197.566910]  cleaner_kthread+0xfa/0x120
[ 6197.574573]  kthread+0x111/0x130
[ 6197.581022]  ? kthread_create_on_node+0x60/0x60
[ 6197.590086]  ret_from_fork+0x1f/0x30
[ 6197.597228] ---[ end trace 424bb7ae00509f56 ]---

This is because the free side drops the ref without the lock, and then
takes the lock if our refcount is 0.  So you can have nodes on the tree
that have a refcount of 0.  Fix this by zero'ing out that element in our
temporary array so we don't try to kill it again.

CC: stable@vger.kernel.org # 4.14+
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: David Sterba <dsterba@suse.com>
[ add comment ]
Signed-off-by: David Sterba <dsterba@suse.com>
fs/btrfs/delayed-inode.c

index 942cc2a59164280e7f89bbef566ffdc771359c5c..d3e15e1d4a91d7a65e55622c9645faa28930ed68 100644 (file)
@@ -1950,12 +1950,19 @@ void btrfs_kill_all_delayed_nodes(struct btrfs_root *root)
                }
 
                inode_id = delayed_nodes[n - 1]->inode_id + 1;
-
-               for (i = 0; i < n; i++)
-                       refcount_inc(&delayed_nodes[i]->refs);
+               for (i = 0; i < n; i++) {
+                       /*
+                        * Don't increase refs in case the node is dead and
+                        * about to be removed from the tree in the loop below
+                        */
+                       if (!refcount_inc_not_zero(&delayed_nodes[i]->refs))
+                               delayed_nodes[i] = NULL;
+               }
                spin_unlock(&root->inode_lock);
 
                for (i = 0; i < n; i++) {
+                       if (!delayed_nodes[i])
+                               continue;
                        __btrfs_kill_delayed_node(delayed_nodes[i]);
                        btrfs_release_delayed_node(delayed_nodes[i]);
                }