From: Zhihao Cheng <chengzhihao1@huawei.com>
To: Richard Weinberger <richard@nod.at>
Cc: Richard Weinberger <richard.weinberger@gmail.com>,
linux-mtd <linux-mtd@lists.infradead.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
yi zhang <yi.zhang@huawei.com>
Subject: Re: [PATCH] ubifs: Fix a potential space leak problem while linking tmpfile
Date: Sat, 11 Jul 2020 14:37:18 +0800 [thread overview]
Message-ID: <0c543297-d94f-ad40-7dd0-2198f39336bb@huawei.com> (raw)
In-Reply-To: <1480699627.103583.1594126053947.JavaMail.zimbra@nod.at>
在 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:
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/
next prev parent reply other threads:[~2020-07-11 6:41 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-01 11:26 [PATCH] ubifs: Fix a potential space leak problem while linking tmpfile Zhihao Cheng
2020-07-07 11:26 ` Richard Weinberger
2020-07-07 11:54 ` Zhihao Cheng
2020-07-07 12:09 ` Richard Weinberger
2020-07-07 12:36 ` Zhihao Cheng
2020-07-07 12:47 ` Richard Weinberger
2020-07-11 6:37 ` Zhihao Cheng [this message]
2020-07-11 6:44 ` Zhihao Cheng
2020-07-13 3:30 ` Zhihao Cheng
-- strict thread matches above, loose matches on Subject: below --
2020-07-01 9:32 Zhihao Cheng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0c543297-d94f-ad40-7dd0-2198f39336bb@huawei.com \
--to=chengzhihao1@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=richard.weinberger@gmail.com \
--cc=richard@nod.at \
--cc=yi.zhang@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).