Re: [PATCH] ubifs: Fix a potential space leak problem while linking tmpfile

[Zhihao Cheng]

å 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:
>
> Â 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).
Results for testcases generic/530, generic/509, generic/389 and 
generic/004 are still "not run".
> 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);
> --