Re: [PATCH 1/6] hfsplus: prevent btree data loss on root split

[Viacheslav Dubeyko <slava@dubeyko.com>]

On Fri, 2018-08-31 at 00:58 -0300, Ernesto A. Fernández wrote:
> Creating, renaming or deleting a file may cause catalog corruption and
> data loss.  This bug is randomly triggered by xfstests generic/027, but
> here is a faster reproducer:
> 
>   truncate -s 50M fs.iso
>   mkfs.hfsplus fs.iso
>   mount fs.iso /mnt
>   i=100
>   while [ $i -le 150 ]; do
>     touch /mnt/$i &>/dev/null
>     ((++i))
>   done
>   i=100
>   while [ $i -le 150 ]; do
>     mv /mnt/$i /mnt/$(perl -e "print $i x82") &>/dev/null
>     ((++i))
>   done
>   umount /mnt
>   fsck.hfsplus -n fs.iso
> 
> The bug is triggered whenever hfs_brec_update_parent() needs to split
> the root node.  The height of the btree is not increased, which leaves
> the new node orphaned and its records lost.
> 
> Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>
> ---
>  fs/hfsplus/brec.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/hfsplus/brec.c b/fs/hfsplus/brec.c
> index ed8eacb34452..aa17a392b414 100644
> --- a/fs/hfsplus/brec.c
> +++ b/fs/hfsplus/brec.c
> @@ -429,6 +429,10 @@ static int hfs_brec_update_parent(struct hfs_find_data *fd)
>  	if (new_node) {
>  		__be32 cnid;
>  
> +		if (!new_node->parent) {
> +			hfs_btree_inc_height(tree);
> +			new_node->parent = tree->root;

I worry about the case when we are adding the node on intermediate (not
root) level. As far as I can see, we will be in trouble here because I
don't see any processing of two possible cases: (1) root node; (2) node
of intermediate level. Do I miss something here?

Thanks,
Vyacheslav Dubeyko.

> +		}
>  		fd->bnode = hfs_bnode_find(tree, new_node->parent);
>  		/* create index key and entry */
>  		hfs_bnode_read_key(new_node, fd->search_key, 14);