[PATCH 0/6] ubifs: Fix wrong space calculation while doing budget

[PATCH 0/6] ubifs: Fix wrong space calculation while doing budget

[Zhihao Cheng]

This series of patches fix ubifs wrong budget space calculations,
which could make make_reservation() failed with ENOSPC error code
and let ubifs become read-only.

Zhihao Cheng (6):
  ubifs: Rectify space budget for ubifs_symlink() if symlink is
    encrypted
  ubifs: Rectify space budget for ubifs_xrename()
  ubifs: Add comments and debug info for ubifs_xrename()
  ubifs: Fix wrong dirty space budget for dirty inode
  ubifs: do_rename: Fix wrong space budget when target inode's nlink > 1
  ubifs: Reserve one leb for each journal head while doing budget

 fs/ubifs/budget.c |  9 ++++-----
 fs/ubifs/dir.c    | 18 +++++++++++++++++-
 2 files changed, 21 insertions(+), 6 deletions(-)

-- 
2.31.1


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

[PATCH 1/6] ubifs: Rectify space budget for ubifs_symlink() if symlink is encrypted

[Zhihao Cheng]

Fix bad space budget when symlink file is encrypted. Bad space budget
may let make_reservation() return with -ENOSPC, which could turn ubifs
to read-only mode in do_writepage() process.

Fetch a reproducer in [Link].

Link: https://bugzilla.kernel.org/show_bug.cgi?id=216490
Fixes: ca7f85be8d6cf9 ("ubifs: Add support for encrypted symlinks")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index bc11276301fd..4705d9a77357 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1150,7 +1150,6 @@ static int ubifs_symlink(struct user_namespace *mnt_userns, struct inode *dir,
 	int err, sz_change, len = strlen(symname);
 	struct fscrypt_str disk_link;
 	struct ubifs_budget_req req = { .new_ino = 1, .new_dent = 1,
-					.new_ino_d = ALIGN(len, 8),
 					.dirtied_ino = 1 };
 	struct fscrypt_name nm;
 
@@ -1166,6 +1165,7 @@ static int ubifs_symlink(struct user_namespace *mnt_userns, struct inode *dir,
 	 * Budget request settings: new inode, new direntry and changing parent
 	 * directory inode.
 	 */
+	req.new_ino_d = ALIGN(disk_link.len - 1, 8);
 	err = ubifs_budget_space(c, &req);
 	if (err)
 		return err;
-- 
2.31.1


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

[PATCH 2/6] ubifs: Rectify space budget for ubifs_xrename()

[Zhihao Cheng]

There is no space budget for ubifs_xrename(). It may let
make_reservation() return with -ENOSPC, which could turn
ubifs to read-only mode in do_writepage() process.
Fix it by adding space budget for ubifs_xrename().

Fetch a reproducer in [Link].

Link: https://bugzilla.kernel.org/show_bug.cgi?id=216569
Fixes: 9ec64962afb170 ("ubifs: Implement RENAME_EXCHANGE")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/dir.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 4705d9a77357..eeda471f05ee 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1575,6 +1575,10 @@ static int ubifs_xrename(struct inode *old_dir, struct dentry *old_dentry,
 		return err;
 	}
 
+	err = ubifs_budget_space(c, &req);
+	if (err)
+		goto out;
+
 	lock_4_inodes(old_dir, new_dir, NULL, NULL);
 
 	time = current_time(old_dir);
@@ -1600,6 +1604,7 @@ static int ubifs_xrename(struct inode *old_dir, struct dentry *old_dentry,
 	unlock_4_inodes(old_dir, new_dir, NULL, NULL);
 	ubifs_release_budget(c, &req);
 
+out:
 	fscrypt_free_filename(&fst_nm);
 	fscrypt_free_filename(&snd_nm);
 	return err;
-- 
2.31.1


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

[PATCH 3/6] ubifs: Add comments and debug info for ubifs_xrename()

[Zhihao Cheng]

Just like other operations (eg. ubifs_create, do_rename), add comments
and debug information for ubifs_xrename().

Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/dir.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index eeda471f05ee..53c5442f48b7 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1565,6 +1565,15 @@ static int ubifs_xrename(struct inode *old_dir, struct dentry *old_dentry,
 
 	ubifs_assert(c, fst_inode && snd_inode);
 
+	/*
+	 * Budget request settings: changing two direntries, changing the two
+	 * parent directory inodes.
+	 */
+
+	dbg_gen("dent '%pd' ino %lu in dir ino %lu exchange dent '%pd' ino %lu in dir ino %lu",
+		old_dentry, fst_inode->i_ino, old_dir->i_ino,
+		new_dentry, snd_inode->i_ino, new_dir->i_ino);
+
 	err = fscrypt_setup_filename(old_dir, &old_dentry->d_name, 0, &fst_nm);
 	if (err)
 		return err;
-- 
2.31.1


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

[PATCH 4/6] ubifs: Fix wrong dirty space budget for dirty inode

[Zhihao Cheng]

Each dirty inode should reserve 'c->bi.inode_budget' bytes in space
budget calculation. Currently, space budget for dirty inode reports
more space than what UBIFS actually needs to write.

Fixes: 1e51764a3c2ac0 ("UBIFS: add new flash file system")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/budget.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c
index e8b9b756f0ac..986e6e4081c7 100644
--- a/fs/ubifs/budget.c
+++ b/fs/ubifs/budget.c
@@ -400,7 +400,7 @@ static int calc_dd_growth(const struct ubifs_info *c,
 	dd_growth = req->dirtied_page ? c->bi.page_budget : 0;
 
 	if (req->dirtied_ino)
-		dd_growth += c->bi.inode_budget << (req->dirtied_ino - 1);
+		dd_growth += c->bi.inode_budget * req->dirtied_ino;
 	if (req->mod_dent)
 		dd_growth += c->bi.dent_budget;
 	dd_growth += req->dirtied_ino_d;
-- 
2.31.1


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

[PATCH 5/6] ubifs: do_rename: Fix wrong space budget when target inode's nlink > 1

[Zhihao Cheng]

If target inode is a special file (eg. block/char device) with nlink
count greater than 1, the inode with ui->data will be re-written on
disk. However, UBIFS losts target inode's data_len while doing space
budget. Bad space budget may let make_reservation() return with -ENOSPC,
which could turn ubifs to read-only mode in do_writepage() process.

Fetch a reproducer in [Link].

Link: https://bugzilla.kernel.org/show_bug.cgi?id=216494
Fixes: 1e51764a3c2ac0 ("UBIFS: add new flash file system")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/dir.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 53c5442f48b7..a77e859246c3 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1323,6 +1323,8 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 	if (unlink) {
 		ubifs_assert(c, inode_is_locked(new_inode));
 
+		/* Budget for old inode's data when its nlink > 1. */
+		req.dirtied_ino_d = ALIGN(ubifs_inode(new_inode)->data_len, 8);
 		err = ubifs_purge_xattrs(new_inode);
 		if (err)
 			return err;
-- 
2.31.1


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

[PATCH 6/6] ubifs: Reserve one leb for each journal head while doing budget

[Zhihao Cheng]

UBIFS calculates available space by c->main_bytes - c->lst.total_used
(which means non-index lebs' free and dirty space is accounted into
total available), then index lebs and four lebs (one for gc_lnum, one
for deletions, two for journal heads) are deducted.
In following situation, ubifs may get -ENOSPC from make_reservation():
 LEB 84: DATAHD   free 122880 used 1920  dirty 2176  dark 6144
 LEB 110:DELETION free 126976 used 0     dirty 0     dark 6144 (empty)
 LEB 201:gc_lnum  free 126976 used 0     dirty 0     dark 6144
 LEB 272:GCHD     free 77824  used 47672 dirty 1480  dark 6144
 LEB 356:BASEHD   free 0      used 39776 dirty 87200 dark 6144
 OTHERS: index lebs, zero-available non-index lebs

UBIFS calculates the available bytes is 6888 (How to calculate it:
126976 * 5[remain main bytes] - 1920[used] - 47672[used] - 39776[used] -
126976 * 1[deletions] - 126976 * 1[gc_lnum] - 126976 * 2[journal heads]
- 6144 * 5[dark] = 6888) after doing budget, however UBIFS cannot use
BASEHD's dirty space(87200), because UBIFS cannot find next BASEHD to
reclaim current BASEHD. (c->bi.min_idx_lebs equals to c->lst.idx_lebs,
the empty leb won't be found by ubifs_find_free_space(), and dirty index
lebs won't be picked as gced lebs. All non-index lebs has dirty space
less then c->dead_wm, non-index lebs won't be picked as gced lebs
either. So new free lebs won't be produced.). See more details in Link.

To fix it, reserve one leb for each journal head while doing budget.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=216562
Fixes: 1e51764a3c2ac0 ("UBIFS: add new flash file system")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/budget.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c
index 986e6e4081c7..d76eb7b39f56 100644
--- a/fs/ubifs/budget.c
+++ b/fs/ubifs/budget.c
@@ -209,11 +209,10 @@ long long ubifs_calc_available(const struct ubifs_info *c, int min_idx_lebs)
 	subtract_lebs += 1;
 
 	/*
-	 * The GC journal head LEB is not really accessible. And since
-	 * different write types go to different heads, we may count only on
-	 * one head's space.
+	 * Since different write types go to different heads, we should
+	 * reserve one leb for each head.
 	 */
-	subtract_lebs += c->jhead_cnt - 1;
+	subtract_lebs += c->jhead_cnt;
 
 	/* We also reserve one LEB for deletions, which bypass budgeting */
 	subtract_lebs += 1;
-- 
2.31.1


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

Re: [PATCH 0/6] ubifs: Fix wrong space calculation while doing budget

[Zhihao Cheng]

在 2022/10/11 11:47, Zhihao Cheng 写道:

Hi. Richard
Just a reminding. Could you please look through following series(Some of 
them is sent several months ago), I wish these can be merged in 6.1, 
thanks a lot:

ubi infinite loop: 
https://patchwork.ozlabs.org/project/linux-mtd/list/?series=304485
ubi uaf in sysfs: 
https://patchwork.ozlabs.org/project/linux-mtd/list/?series=311956
ubi fastmap: 
https://patchwork.ozlabs.org/project/linux-mtd/list/?series=313063
ubifs assertion failed about writepage: 
https://patchwork.ozlabs.org/project/linux-mtd/list/?series=302776
ubifs space budget: 
https://patchwork.ozlabs.org/project/linux-mtd/list/?series=322220

> This series of patches fix ubifs wrong budget space calculations,
> which could make make_reservation() failed with ENOSPC error code
> and let ubifs become read-only.
>
> Zhihao Cheng (6):
>    ubifs: Rectify space budget for ubifs_symlink() if symlink is
>      encrypted
>    ubifs: Rectify space budget for ubifs_xrename()
>    ubifs: Add comments and debug info for ubifs_xrename()
>    ubifs: Fix wrong dirty space budget for dirty inode
>    ubifs: do_rename: Fix wrong space budget when target inode's nlink > 1
>    ubifs: Reserve one leb for each journal head while doing budget
>
>   fs/ubifs/budget.c |  9 ++++-----
>   fs/ubifs/dir.c    | 18 +++++++++++++++++-
>   2 files changed, 21 insertions(+), 6 deletions(-)
>


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

Re: [PATCH 0/6] ubifs: Fix wrong space calculation while doing budget

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "chengzhihao1" <chengzhihao1@huawei.com>
> An: "richard" <richard@nod.at>, "Sascha Hauer" <s.hauer@pengutronix.de>, "Miquel Raynal" <miquel.raynal@bootlin.com>
> CC: "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel" <linux-kernel@vger.kernel.org>, "yi zhang"
> <yi.zhang@huawei.com>
> Gesendet: Freitag, 18. November 2022 09:53:38
> Betreff: Re: [PATCH 0/6] ubifs: Fix wrong space calculation while doing budget

> 在 2022/10/11 11:47, Zhihao Cheng 写道:
> 
> Hi. Richard
> Just a reminding. Could you please look through following series(Some of
> them is sent several months ago), I wish these can be merged in 6.1,
> thanks a lot:
> 
> ubi infinite loop:
> https://patchwork.ozlabs.org/project/linux-mtd/list/?series=304485
> ubi uaf in sysfs:
> https://patchwork.ozlabs.org/project/linux-mtd/list/?series=311956
> ubi fastmap:
> https://patchwork.ozlabs.org/project/linux-mtd/list/?series=313063
> ubifs assertion failed about writepage:
> https://patchwork.ozlabs.org/project/linux-mtd/list/?series=302776
> ubifs space budget:
> https://patchwork.ozlabs.org/project/linux-mtd/list/?series=322220

Sure. Let me check what happened to them.

Thanks,
//richard

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

Re: [PATCH 0/6] ubifs: Fix wrong space calculation while doing budget

[Zhihao Cheng]

在 2022/11/18 16:56, Richard Weinberger 写道:
>> 在 2022/10/11 11:47, Zhihao Cheng 写道:
>>
>> Hi. Richard
>> Just a reminding. Could you please look through following series(Some of
>> them is sent several months ago), I wish these can be merged in 6.1,
>> thanks a lot:
>>
>> ubi infinite loop:
>> https://patchwork.ozlabs.org/project/linux-mtd/list/?series=304485
>> ubi uaf in sysfs:
>> https://patchwork.ozlabs.org/project/linux-mtd/list/?series=311956
>> ubi fastmap:
>> https://patchwork.ozlabs.org/project/linux-mtd/list/?series=313063
>> ubifs assertion failed about writepage:
>> https://patchwork.ozlabs.org/project/linux-mtd/list/?series=302776
>> ubifs space budget:
>> https://patchwork.ozlabs.org/project/linux-mtd/list/?series=322220
> Sure. Let me check what happened to them.
OK. Thanks again.
> Thanks,
> //richard
> .



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