From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-f196.google.com ([209.85.210.196]:41088 "EHLO mail-pf1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725826AbeJXLg6 (ORCPT ); Wed, 24 Oct 2018 07:36:58 -0400 Received: by mail-pf1-f196.google.com with SMTP id a19-v6so1682632pfo.8 for ; Tue, 23 Oct 2018 20:10:53 -0700 (PDT) Message-ID: <1540344830.28908.12.camel@slavad-ubuntu-14.04> Subject: Re: [PATCH 2/6] hfsplus: fix BUG on bnode parent update From: Viacheslav Dubeyko To: "Ernesto A." =?ISO-8859-1?Q?Fern=E1ndez?= Cc: linux-fsdevel@vger.kernel.org, Andrew Morton Date: Tue, 23 Oct 2018 18:33:50 -0700 In-Reply-To: <5ee1db09b60373a15890f6a7c835d00e76bf601d.1535682461.git.ernesto.mnd.fernandez@gmail.com> References: <26d882184fc43043a810114258f45277752186c7.1535682461.git.ernesto.mnd.fernandez@gmail.com> <5ee1db09b60373a15890f6a7c835d00e76bf601d.1535682461.git.ernesto.mnd.fernandez@gmail.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, 2018-08-31 at 00:59 -0300, Ernesto A. Fernández wrote: > Creating, renaming or deleting a file may hit BUG_ON() if the first > record of both a leaf node and its parent are changed, and if this > forces the parent to be split. This bug is triggered by xfstests > generic/027, somewhat rarely; here is a more reliable reproducer: > > truncate -s 50M fs.iso > mkfs.hfsplus fs.iso > mount fs.iso /mnt > i=1000 > while [ $i -le 2400 ]; do > touch /mnt/$i &>/dev/null > ((++i)) > done > i=2400 > while [ $i -ge 1000 ]; do > mv /mnt/$i /mnt/$(perl -e "print $i x61") &>/dev/null > ((--i)) > done > > The issue is that a newly created bnode is being put twice. Reset > new_node to NULL in hfs_brec_update_parent() before reaching goto again. > > Signed-off-by: Ernesto A. Fernández > --- > fs/hfsplus/brec.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/hfsplus/brec.c b/fs/hfsplus/brec.c > index aa17a392b414..1918544a7871 100644 > --- a/fs/hfsplus/brec.c > +++ b/fs/hfsplus/brec.c > @@ -449,6 +449,7 @@ static int hfs_brec_update_parent(struct hfs_find_data *fd) > /* restore search_key */ > hfs_bnode_read_key(node, fd->search_key, 14); > } > + new_node = NULL; Sorry, I don't follow where the new_node is put twice. Could you explain in more details? Currently, it looks unclear. I like to assign the NULL value to the pointer. But are you sure that it's proper place? Maybe it makes sense to place this assignment in the beginning of the function? Thanks, Vyacheslav Dubeyko. > } > > if (!rec && node->parent)