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

Viacheslav Dubeyko
2018-10-24T11:34:31Z

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);