hfsplus: fix potential race conditions in b-tree functionality

The HFS_BNODE_DELETED flag is checked in hfs_bnode_put()
under locked tree->hash_lock. This patch adds locking
for the case of setting the HFS_BNODE_DELETED flag in
hfs_bnode_unlink() with the goal to avoid potential
race conditions.

The hfs_btree_write() method should be called under
tree->tree_lock. This patch reworks logic by adding
locking the tree->tree_lock for the calls of
hfs_btree_write() in hfsplus_cat_write_inode() and
hfsplus_system_write_inode().

This patch adds also the lockdep_assert_held() in
hfs_bmap_reserve(), hfs_bmap_alloc(), and hfs_bmap_free().

cc: John Paul Adrian Glaubitz <[email protected]>
cc: Yangtao Li <[email protected]>
cc: [email protected]
Signed-off-by: Viacheslav Dubeyko <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Viacheslav Dubeyko <[email protected]>

Author: dubeyko

fs/hfsplus/bnode.c

@@ -420,7 +420,10 @@ void hfs_bnode_unlink(struct hfs_bnode *node)
 		tree->root = 0;
 		tree->depth = 0;
 	}
+
+	spin_lock(&tree->hash_lock);
 	set_bit(HFS_BNODE_DELETED, &node->flags);
+	spin_unlock(&tree->hash_lock);
 }
 
 static inline int hfs_bnode_hash(u32 num)

fs/hfsplus/btree.c

@@ -500,6 +500,8 @@ int hfs_bmap_reserve(struct hfs_btree *tree, u32 rsvd_nodes)
 	u32 count;
 	int res;
 
+	lockdep_assert_held(&tree->tree_lock);
+
 	if (rsvd_nodes <= 0)
 		return 0;
 
@@ -529,6 +531,8 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
 	u8 *data, byte, m;
 	int i, res;
 
+	lockdep_assert_held(&tree->tree_lock);
+
 	res = hfs_bmap_reserve(tree, 1);
 	if (res)
 		return ERR_PTR(res);
@@ -607,6 +611,7 @@ void hfs_bmap_free(struct hfs_bnode *node)
 	hfs_dbg("node %u\n", node->this);
 	BUG_ON(!node->this);
 	tree = node->tree;
+	lockdep_assert_held(&tree->tree_lock);
 	nidx = node->this;
 	node = hfs_bnode_find(tree, 0);
 	if (IS_ERR(node))

fs/hfsplus/inode.c

@@ -720,20 +720,19 @@ int hfsplus_cat_write_inode(struct inode *inode)
 					 sizeof(struct hfsplus_cat_file));
 	}
 
+	res = hfs_btree_write(tree);
+	if (res) {
+		pr_err("b-tree write err: %d, ino %lu\n",
+		       res, inode->i_ino);
+		goto out;
+	}
+
 	set_bit(HFSPLUS_I_CAT_DIRTY,
 		&HFSPLUS_I(HFSPLUS_CAT_TREE_I(inode->i_sb))->flags);
 	set_bit(HFSPLUS_I_CAT_DIRTY, &HFSPLUS_I(inode)->flags);
 out:
 	hfs_find_exit(&fd);
 
-	if (!res) {
-		res = hfs_btree_write(tree);
-		if (res) {
-			pr_err("b-tree write err: %d, ino %lu\n",
-			       res, inode->i_ino);
-		}
-	}
-
 	return res;
 }
 

fs/hfsplus/super.c

@@ -153,7 +153,10 @@ static int hfsplus_system_write_inode(struct inode *inode)
 	}
 	hfsplus_inode_write_fork(inode, fork);
 	if (tree) {
+		mutex_lock_nested(&tree->tree_lock,
+				  hfsplus_btree_lock_class(tree));
 		int err = hfs_btree_write(tree);
+		mutex_unlock(&tree->tree_lock);
 
 		if (err) {
 			pr_err("b-tree write err: %d, ino %lu\n",