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

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

[Zhihao Cheng]

There is a potential space leak problem while linking tmpfile, in which
case, inode node (with nlink=0) is valid in tnc (on flash), which leads
to space leak. Meanwhile, the corresponding data nodes won't be released
from tnc. For example, (A reproducer can be found in Link):

$ mount UBIFS
  [process A]            [process B]         [TNC]         [orphan area]

 ubifs_tmpfile                          inode_A (nlink=0)     inode_A
                          do_commit     inode_A (nlink=0)     inode_A
			       ↑
      (comment: It makes sure not replay inode_A in next mount)
 ubifs_link                             inode_A (nlink=0)     inode_A
   ubifs_delete_orphan                  inode_A (nlink=0)
                          do_commit     inode_A (nlink=0)
                           ---> POWERCUT <---
   (ubifs_jnl_update)

$ mount UBIFS
  inode_A will neither be replayed in ubifs_replay_journal() nor
  ubifs_mount_orphans(). inode_A (nlink=0) with its data nodes will
  always on tnc, it occupy space but is non-visable for users.

Commit ee1438ce5dc4d ("ubifs: Check link count of inodes when killing
orphans.") handles problem in mistakenly deleting relinked tmpfile
while replaying orphan area. Since that, tmpfile inode should always
live in orphan area even it is linked. Fix it by reverting commit
32fe905c17f001 ("ubifs: Fix O_TMPFILE corner case in ubifs_link()").

Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
Cc: <stable@vger.kernel.org>  # v5.3+
Fixes: 32fe905c17f001 ("ubifs: Fix O_TMPFILE corner case in ubifs_link()")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=208405
---
 fs/ubifs/dir.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index ef85ec167a84..9534c4bb598f 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);
@@ -747,8 +742,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);
-- 
2.25.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[Richard Weinberger]

On Wed, Jul 1, 2020 at 1:28 PM Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>
> There is a potential space leak problem while linking tmpfile, in which
> case, inode node (with nlink=0) is valid in tnc (on flash), which leads
> to space leak. Meanwhile, the corresponding data nodes won't be released
> from tnc. For example, (A reproducer can be found in Link):
>
> $ mount UBIFS
>   [process A]            [process B]         [TNC]         [orphan area]
>
>  ubifs_tmpfile                          inode_A (nlink=0)     inode_A
>                           do_commit     inode_A (nlink=0)     inode_A
>                                ↑
>       (comment: It makes sure not replay inode_A in next mount)
>  ubifs_link                             inode_A (nlink=0)     inode_A
>    ubifs_delete_orphan                  inode_A (nlink=0)
>                           do_commit     inode_A (nlink=0)
>                            ---> POWERCUT <---
>    (ubifs_jnl_update)
>
> $ mount UBIFS
>   inode_A will neither be replayed in ubifs_replay_journal() nor
>   ubifs_mount_orphans(). inode_A (nlink=0) with its data nodes will
>   always on tnc, it occupy space but is non-visable for users.
>
> Commit ee1438ce5dc4d ("ubifs: Check link count of inodes when killing
> orphans.") handles problem in mistakenly deleting relinked tmpfile
> while replaying orphan area. Since that, tmpfile inode should always
> live in orphan area even it is linked. Fix it by reverting commit
> 32fe905c17f001 ("ubifs: Fix O_TMPFILE corner case in ubifs_link()").

Well, by reverting this commit you re-introduce the issue it fixes. ;-\

-- 
Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[Zhihao Cheng]

在 2020/7/7 19:26, Richard Weinberger 写道:
> On Wed, Jul 1, 2020 at 1:28 PM Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>> There is a potential space leak problem while linking tmpfile, in which
>> case, inode node (with nlink=0) is valid in tnc (on flash), which leads
>> to space leak. Meanwhile, the corresponding data nodes won't be released
>> from tnc. For example, (A reproducer can be found in Link):
>>
>> $ mount UBIFS
>>    [process A]            [process B]         [TNC]         [orphan area]
>>
>>   ubifs_tmpfile                          inode_A (nlink=0)     inode_A
>>                            do_commit     inode_A (nlink=0)     inode_A
>>                                 ↑
>>        (comment: It makes sure not replay inode_A in next mount)
>>   ubifs_link                             inode_A (nlink=0)     inode_A
>>     ubifs_delete_orphan                  inode_A (nlink=0)
>>                            do_commit     inode_A (nlink=0)
>>                             ---> POWERCUT <---
>>     (ubifs_jnl_update)
>>
>> $ mount UBIFS
>>    inode_A will neither be replayed in ubifs_replay_journal() nor
>>    ubifs_mount_orphans(). inode_A (nlink=0) with its data nodes will
>>    always on tnc, it occupy space but is non-visable for users.
>>
>> Commit ee1438ce5dc4d ("ubifs: Check link count of inodes when killing
>> orphans.") handles problem in mistakenly deleting relinked tmpfile
>> while replaying orphan area. Since that, tmpfile inode should always
>> live in orphan area even it is linked. Fix it by reverting commit
>> 32fe905c17f001 ("ubifs: Fix O_TMPFILE corner case in ubifs_link()").
> Well, by reverting this commit you re-introduce the issue it fixes. ;-\
>
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.



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[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().

When you create a tmpfile it has a link count of 0 and an orphan is
installed. Such that the tmpfile is gone after a reboot but you can
still use it prior to that.
By using linkat() you can raise the link counter to 1 again.
Thus, the orphan needs to be removed.
This is pattern overlayfs uses a lot.

Since UBIFS never supported raising the link counter from 0 to 1
we have many corner cases and fixing all these turned out into a nightmare.
...as you can see from the amount broken patches from me :-(.

Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[Zhihao Cheng]

在 2020/7/7 20:09, 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?
>
> When you create a tmpfile it has a link count of 0 and an orphan is
> installed. Such that the tmpfile is gone after a reboot but you can
> still use it prior to that.
> By using linkat() you can raise the link counter to 1 again.
> Thus, the orphan needs to be removed.
> This is pattern overlayfs uses a lot.
>
> Since UBIFS never supported raising the link counter from 0 to 1
> we have many corner cases and fixing all these turned out into a nightmare.
> ...as you can see from the amount broken patches from me :-(.
>
> Thanks,
> //richard
>
> .




______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[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).

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/

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



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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:
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/




______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[Zhihao Cheng]

There is a potential space leak problem while linking tmpfile, in which
case, inode node (with nlink=0) is valid in tnc (on flash), which leads
to space leak. Meanwhile, the corresponding data nodes won't be released
from tnc. For example, (A reproducer can be found in Link):

$ mount UBIFS
  [process A]            [process B]         [TNC]         [orphan area]

 ubifs_tmpfile                          inode_A (nlink=0)     inode_A
                          do_commit     inode_A (nlink=0)     inode_A
			       ↑
      (comment: It makes sure not replay inode_A in next mount)
 ubifs_link                             inode_A (nlink=0)     inode_A
   ubifs_delete_orphan                  inode_A (nlink=0)
                          do_commit     inode_A (nlink=0)
                           ---> POWERCUT <---
   (ubifs_jnl_update)

$ mount UBIFS
  inode_A will neither be replayed in ubifs_replay_journal() nor
  ubifs_mount_orphans(). inode_A (nlink=0) with its data nodes will
  always on tnc, it occupy space but is non-visable for users.

Commit ee1438ce5dc4d ("ubifs: Check link count of inodes when killing
orphans.") handles problem in mistakenly deleting relinked tmpfile
while replaying orphan area. Since that, tmpfile inode should always
live in orphan area even it is linked. Fix it by reverting commit
32fe905c17f001 ("ubifs: Fix O_TMPFILE corner case in ubifs_link()").

Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
Cc: <stable@vger.kernel.org>
Fixes: 32fe905c17f001 ("ubifs: Fix O_TMPFILE corner case in ubifs_link()")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=208405
---
 fs/ubifs/dir.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index ef85ec167a84..9534c4bb598f 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);
@@ -747,8 +742,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);
-- 
2.25.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/