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

Ernesto A. Fernández
2018-10-24T11:40:19Z

On Tue, Oct 23, 2018 at 06:23:53PM -0700, Viacheslav Dubeyko wrote:
> 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?

If 'new_node' had been the result of splitting a node other than root,
then it would have a parent.

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