From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 39A17C433DF for ; Mon, 13 Jul 2020 03:30:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 174F7206E2 for ; Mon, 13 Jul 2020 03:30:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728714AbgGMDaX (ORCPT ); Sun, 12 Jul 2020 23:30:23 -0400 Received: from szxga06-in.huawei.com ([45.249.212.32]:33664 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726261AbgGMDaW (ORCPT ); Sun, 12 Jul 2020 23:30:22 -0400 Received: from DGGEMS408-HUB.china.huawei.com (unknown [172.30.72.60]) by Forcepoint Email with ESMTP id C03845CF625A9997552B; Mon, 13 Jul 2020 11:30:17 +0800 (CST) Received: from [127.0.0.1] (10.174.179.214) by DGGEMS408-HUB.china.huawei.com (10.3.19.208) with Microsoft SMTP Server id 14.3.487.0; Mon, 13 Jul 2020 11:30:10 +0800 Subject: Re: [PATCH] ubifs: Fix a potential space leak problem while linking tmpfile From: Zhihao Cheng To: Richard Weinberger CC: Richard Weinberger , linux-mtd , linux-kernel , yi zhang References: <20200701112643.726986-1-chengzhihao1@huawei.com> <082f18e0-d6f0-6389-43af-3159edb244cb@huawei.com> <1463101229.103384.1594123741187.JavaMail.zimbra@nod.at> <963fa5c8-414f-783f-871e-47e751b54d87@huawei.com> <1480699627.103583.1594126053947.JavaMail.zimbra@nod.at> <0c543297-d94f-ad40-7dd0-2198f39336bb@huawei.com> Message-ID: <4b59ffcc-fbbb-e62e-779b-ce4d795f51c7@huawei.com> Date: Mon, 13 Jul 2020 11:30:09 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: <0c543297-d94f-ad40-7dd0-2198f39336bb@huawei.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.174.179.214] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 在 2020/7/11 14:37, Zhihao Cheng 写道: > 在 2020/7/7 20:47, Richard Weinberger 写道: >> ----- Ursprüngliche Mail ----- >>>>> Perhaps I misunderstood what commit 32fe905c17f001 ("ubifs: Fix >>>>> O_TMPFILE corner case in ubifs_link()") wanted to fix. >>>>> I think orphan area is used to remind filesystem don't forget to >>>>> delete >>>>> inodes (whose nlink is 0) in next unclean rebooting. Generally, >>>>> the file >>>>> system is not corrupted caused by replaying orphan nodes. >>>>> Ralph reported a filesystem corruption in combination with overlayfs. >>>>> Can you tell me the details about that problem? Thanks. >>>> On my test bed I didn't see a fs corruption, what I saw was a >>>> failing orphan >>>> self test while playing with O_TMPFILE and linkat(). >>> Do we have a reproducer, or can I get the fail testcase? Is it a >>> xfstest >>> case? >> I think xfstests triggered it, yes. >> Later today I can check. :) >> >> Thanks, >> //richard >> >> . > > I think I have found the testcases, overlay/006 and overlay/041. > > The 'mv' and 'rm' operations will put lowertestfile into orphan list > twice, so we must reseve the orphan deletion operation in > ubifs_link(), otherwise the testcase fails and we will see the > following msg: Sorry, not lowertestfile, it's tempfile which is generated by ovl copy-up (mv operation). The tempfile is linked after copy-up finished. The tempfile is then unlinked by 'rm' operation. > >   overlay/006 2s ... - output mismatch (see > /root/git/xfstests-dev/results//overlay/006.out.bad) >     --- tests/overlay/006.out    2020-07-07 21:42:57.737000000 +0800 >     +++ /root/git/xfstests-dev/results//overlay/006.out.bad 2020-07-11 > 14:31:55.340000000 +0800 >     @@ -1,2 +1,4 @@ >      QA output created by 006 >      Silence is golden >     +rm: cannot remove > '/tmp/scratch/ovl-mnt/uppertestdir/lowertestfile': Invalid argument >     +lowertestfile >     ... > >   [  382.258210] UBIFS error (ubi0:1 pid 11896): orphan_add [ubifs]: > orphaned twice >   [  382.352535] UBIFS error (ubi0:1 pid 11930): free_orphans [ubifs]: > orphan list not empty at unmount > > > So, how about moving ubifs_delete_orphan() after ubifs_jnl_update() in > function ubifs_link(). Following modifications applied in linux-5.8 > has been tested by overlay/041, overlay/006 and  other tmpfile cases > (generic/531, generic/530, generic/509, generic/389, generic/004). > > diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c > index ef85ec167a84..fd4443a5e8c6 100644 > --- a/fs/ubifs/dir.c > +++ b/fs/ubifs/dir.c > @@ -722,11 +722,6 @@ static int ubifs_link(struct dentry *old_dentry, > struct inode *dir, >                 goto out_fname; > >         lock_2_inodes(dir, inode); > - > -       /* Handle O_TMPFILE corner case, it is allowed to link a > O_TMPFILE. */ > -       if (inode->i_nlink == 0) > -               ubifs_delete_orphan(c, inode->i_ino); > - > inc_nlink(inode); > ihold(inode); >         inode->i_ctime = current_time(inode); > @@ -736,6 +731,11 @@ static int ubifs_link(struct dentry *old_dentry, > struct inode *dir, >         err = ubifs_jnl_update(c, dir, &nm, inode, 0, 0); >         if (err) >                 goto out_cancel; > + > +       /* Handle O_TMPFILE corner case, it is allowed to link a > O_TMPFILE. */ > +       if (inode->i_nlink == 1) > +               ubifs_delete_orphan(c, inode->i_ino); > + >         unlock_2_inodes(dir, inode); > >         ubifs_release_budget(c, &req); > @@ -747,8 +747,6 @@ static int ubifs_link(struct dentry *old_dentry, > struct inode *dir, >         dir->i_size -= sz_change; >         dir_ui->ui_size = dir->i_size; > drop_nlink(inode); > -       if (inode->i_nlink == 0) > -               ubifs_add_orphan(c, inode->i_ino); >         unlock_2_inodes(dir, inode); >         ubifs_release_budget(c, &req); > iput(inode); > -- > > > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/