* [PATCH] ubifs: Fix a potential space leak problem while linking tmpfile
@ 2020-07-01 11:26 Zhihao Cheng
2020-07-07 11:26 ` Richard Weinberger
0 siblings, 1 reply; 10+ messages in thread
From: Zhihao Cheng @ 2020-07-01 11:26 UTC (permalink / raw)
To: linux-mtd, linux-kernel; +Cc: richard, yi.zhang
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] ubifs: Fix a potential space leak problem while linking tmpfile
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
0 siblings, 1 reply; 10+ messages in thread
From: Richard Weinberger @ 2020-07-07 11:26 UTC (permalink / raw)
To: Zhihao Cheng; +Cc: linux-mtd, LKML, Richard Weinberger, zhangyi (F)
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ubifs: Fix a potential space leak problem while linking tmpfile
2020-07-07 11:26 ` Richard Weinberger
@ 2020-07-07 11:54 ` Zhihao Cheng
2020-07-07 12:09 ` Richard Weinberger
0 siblings, 1 reply; 10+ messages in thread
From: Zhihao Cheng @ 2020-07-07 11:54 UTC (permalink / raw)
To: Richard Weinberger; +Cc: linux-mtd, LKML, Richard Weinberger, zhangyi (F)
在 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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ubifs: Fix a potential space leak problem while linking tmpfile
2020-07-07 11:54 ` Zhihao Cheng
@ 2020-07-07 12:09 ` Richard Weinberger
2020-07-07 12:36 ` Zhihao Cheng
0 siblings, 1 reply; 10+ messages in thread
From: Richard Weinberger @ 2020-07-07 12:09 UTC (permalink / raw)
To: chengzhihao1; +Cc: Richard Weinberger, linux-mtd, linux-kernel, yi zhang
----- 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ubifs: Fix a potential space leak problem while linking tmpfile
2020-07-07 12:09 ` Richard Weinberger
@ 2020-07-07 12:36 ` Zhihao Cheng
2020-07-07 12:47 ` Richard Weinberger
0 siblings, 1 reply; 10+ messages in thread
From: Zhihao Cheng @ 2020-07-07 12:36 UTC (permalink / raw)
To: Richard Weinberger; +Cc: Richard Weinberger, linux-mtd, linux-kernel, yi zhang
在 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
>
> .
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ubifs: Fix a potential space leak problem while linking tmpfile
2020-07-07 12:36 ` Zhihao Cheng
@ 2020-07-07 12:47 ` Richard Weinberger
2020-07-11 6:37 ` Zhihao Cheng
0 siblings, 1 reply; 10+ messages in thread
From: Richard Weinberger @ 2020-07-07 12:47 UTC (permalink / raw)
To: chengzhihao1; +Cc: Richard Weinberger, linux-mtd, linux-kernel, yi zhang
----- 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ubifs: Fix a potential space leak problem while linking tmpfile
2020-07-07 12:47 ` Richard Weinberger
@ 2020-07-11 6:37 ` Zhihao Cheng
2020-07-11 6:44 ` Zhihao Cheng
2020-07-13 3:30 ` Zhihao Cheng
0 siblings, 2 replies; 10+ messages in thread
From: Zhihao Cheng @ 2020-07-11 6:37 UTC (permalink / raw)
To: Richard Weinberger; +Cc: Richard Weinberger, linux-mtd, linux-kernel, yi zhang
在 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);
--
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] ubifs: Fix a potential space leak problem while linking tmpfile
2020-07-11 6:37 ` Zhihao Cheng
@ 2020-07-11 6:44 ` Zhihao Cheng
2020-07-13 3:30 ` Zhihao Cheng
1 sibling, 0 replies; 10+ messages in thread
From: Zhihao Cheng @ 2020-07-11 6:44 UTC (permalink / raw)
To: Richard Weinberger; +Cc: Richard Weinberger, linux-mtd, linux-kernel, yi zhang
在 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);
> --
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ubifs: Fix a potential space leak problem while linking tmpfile
2020-07-11 6:37 ` Zhihao Cheng
2020-07-11 6:44 ` Zhihao Cheng
@ 2020-07-13 3:30 ` Zhihao Cheng
1 sibling, 0 replies; 10+ messages in thread
From: Zhihao Cheng @ 2020-07-13 3:30 UTC (permalink / raw)
To: Richard Weinberger; +Cc: Richard Weinberger, linux-mtd, linux-kernel, yi zhang
在 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/
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ubifs: Fix a potential space leak problem while linking tmpfile
@ 2020-07-01 9:32 Zhihao Cheng
0 siblings, 0 replies; 10+ messages in thread
From: Zhihao Cheng @ 2020-07-01 9:32 UTC (permalink / raw)
To: richard, yi.zhang; +Cc: linux-mtd, linux-kernel
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-07-13 3:30 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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
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).