public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH v5 1/3] ubifs: prefer kstrtobool_from_user() over custom helper
@ 2026-04-15  7:18 Dmitry Antipov
  2026-04-15  7:18 ` [PATCH v5 2/3] ubifs: use strscpy() and kmemdup_nul() where appropriate Dmitry Antipov
  2026-04-15  7:18 ` [PATCH v5 3/3] ubifs: avoid redundant calls to memset() Dmitry Antipov
  0 siblings, 2 replies; 7+ messages in thread
From: Dmitry Antipov @ 2026-04-15  7:18 UTC (permalink / raw)
  To: Zhihao Cheng, Richard Weinberger; +Cc: linux-mtd, Dmitry Antipov

Adjust 'dfs_file_write()' and 'dfs_global_file_write()' to prefer generic
'kstrtobool_from_user()' over an ad-hoc 'interpret_user_input()' helper,
thus making the latter not needed anymore.

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v3 and upwards: bump version to match the series
v2: add Reviewed-by: and bump version to match the series
---
 fs/ubifs/debug.c | 44 ++++++++++----------------------------------
 1 file changed, 10 insertions(+), 34 deletions(-)

diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c
index 5794de5a9069..9e0468dc72d3 100644
--- a/fs/ubifs/debug.c
+++ b/fs/ubifs/debug.c
@@ -2732,39 +2732,14 @@ static ssize_t dfs_file_read(struct file *file, char __user *u, size_t count,
 	return provide_user_output(val, u, count, ppos);
 }
 
-/**
- * interpret_user_input - interpret user debugfs file input.
- * @u: user-provided buffer with the input
- * @count: buffer size
- *
- * This is a helper function which interpret user input to a boolean UBIFS
- * debugfs file. Returns %0 or %1 in case of success and a negative error code
- * in case of failure.
- */
-static int interpret_user_input(const char __user *u, size_t count)
-{
-	size_t buf_size;
-	char buf[8];
-
-	buf_size = min_t(size_t, count, (sizeof(buf) - 1));
-	if (copy_from_user(buf, u, buf_size))
-		return -EFAULT;
-
-	if (buf[0] == '1')
-		return 1;
-	else if (buf[0] == '0')
-		return 0;
-
-	return -EINVAL;
-}
-
 static ssize_t dfs_file_write(struct file *file, const char __user *u,
 			      size_t count, loff_t *ppos)
 {
 	struct ubifs_info *c = file->private_data;
 	struct ubifs_debug_info *d = c->dbg;
 	struct dentry *dent = file->f_path.dentry;
-	int val;
+	bool val;
+	int ret;
 
 	if (file->f_path.dentry == d->dfs_dump_lprops) {
 		ubifs_dump_lprops(c);
@@ -2781,9 +2756,9 @@ static ssize_t dfs_file_write(struct file *file, const char __user *u,
 		return count;
 	}
 
-	val = interpret_user_input(u, count);
-	if (val < 0)
-		return val;
+	ret = kstrtobool_from_user(u, count, &val);
+	if (unlikely(ret))
+		return ret;
 
 	if (dent == d->dfs_chk_gen)
 		d->chk_gen = val;
@@ -2926,11 +2901,12 @@ static ssize_t dfs_global_file_write(struct file *file, const char __user *u,
 				     size_t count, loff_t *ppos)
 {
 	struct dentry *dent = file->f_path.dentry;
-	int val;
+	bool val;
+	int ret;
 
-	val = interpret_user_input(u, count);
-	if (val < 0)
-		return val;
+	ret = kstrtobool_from_user(u, count, &val);
+	if (unlikely(ret))
+		return ret;
 
 	if (dent == dfs_chk_gen)
 		ubifs_dbg.chk_gen = val;
-- 
2.53.0


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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 2/3] ubifs: use strscpy() and kmemdup_nul() where appropriate
  2026-04-15  7:18 [PATCH v5 1/3] ubifs: prefer kstrtobool_from_user() over custom helper Dmitry Antipov
@ 2026-04-15  7:18 ` Dmitry Antipov
  2026-04-16  3:37   ` Zhihao Cheng
  2026-04-15  7:18 ` [PATCH v5 3/3] ubifs: avoid redundant calls to memset() Dmitry Antipov
  1 sibling, 1 reply; 7+ messages in thread
From: Dmitry Antipov @ 2026-04-15  7:18 UTC (permalink / raw)
  To: Zhihao Cheng, Richard Weinberger; +Cc: linux-mtd, Dmitry Antipov

Go closer to the modern kernel API and use 'strscpy()' and 'kmemdup_nul()'
over an ad-hoc ensure-to-have-'\0' quirks where appropriate.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v5: switch to fname_len() + 1 to specify strscpy() target buffer size
v4: once again to not forget a filesystem-native (little) to CPU endian swap
v3: fix strscpy() usage as noticed by Zhihao
v2: initial version to join the series
---
 fs/ubifs/journal.c | 18 ++++++------------
 fs/ubifs/replay.c  |  3 +--
 fs/ubifs/super.c   |  8 ++------
 3 files changed, 9 insertions(+), 20 deletions(-)

diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index 40a95a2fad50..8d7f0843d353 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -729,8 +729,7 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
 	dent->inum = deletion ? 0 : cpu_to_le64(inode->i_ino);
 	dent->type = get_dent_type(inode->i_mode);
 	dent->nlen = cpu_to_le16(fname_len(nm));
-	memcpy(dent->name, fname_name(nm), fname_len(nm));
-	dent->name[fname_len(nm)] = '\0';
+	strscpy(dent->name, fname_name(nm), fname_len(nm) + 1);
 	set_dent_cookie(c, dent);
 
 	zero_dent_node_unused(dent);
@@ -1232,8 +1231,7 @@ int ubifs_jnl_xrename(struct ubifs_info *c, const struct inode *fst_dir,
 	dent1->inum = cpu_to_le64(fst_inode->i_ino);
 	dent1->type = get_dent_type(fst_inode->i_mode);
 	dent1->nlen = cpu_to_le16(fname_len(snd_nm));
-	memcpy(dent1->name, fname_name(snd_nm), fname_len(snd_nm));
-	dent1->name[fname_len(snd_nm)] = '\0';
+	strscpy(dent1->name, fname_name(snd_nm), fname_len(snd_nm) + 1);
 	set_dent_cookie(c, dent1);
 	zero_dent_node_unused(dent1);
 	ubifs_prep_grp_node(c, dent1, dlen1, 0);
@@ -1248,8 +1246,7 @@ int ubifs_jnl_xrename(struct ubifs_info *c, const struct inode *fst_dir,
 	dent2->inum = cpu_to_le64(snd_inode->i_ino);
 	dent2->type = get_dent_type(snd_inode->i_mode);
 	dent2->nlen = cpu_to_le16(fname_len(fst_nm));
-	memcpy(dent2->name, fname_name(fst_nm), fname_len(fst_nm));
-	dent2->name[fname_len(fst_nm)] = '\0';
+	strscpy(dent2->name, fname_name(fst_nm), fname_len(fst_nm) + 1);
 	set_dent_cookie(c, dent2);
 	zero_dent_node_unused(dent2);
 	ubifs_prep_grp_node(c, dent2, dlen2, 0);
@@ -1424,8 +1421,7 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 	dent->inum = cpu_to_le64(old_inode->i_ino);
 	dent->type = get_dent_type(old_inode->i_mode);
 	dent->nlen = cpu_to_le16(fname_len(new_nm));
-	memcpy(dent->name, fname_name(new_nm), fname_len(new_nm));
-	dent->name[fname_len(new_nm)] = '\0';
+	strscpy(dent->name, fname_name(new_nm), fname_len(new_nm) + 1);
 	set_dent_cookie(c, dent);
 	zero_dent_node_unused(dent);
 	ubifs_prep_grp_node(c, dent, dlen1, 0);
@@ -1446,8 +1442,7 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 		dent2->type = DT_UNKNOWN;
 	}
 	dent2->nlen = cpu_to_le16(fname_len(old_nm));
-	memcpy(dent2->name, fname_name(old_nm), fname_len(old_nm));
-	dent2->name[fname_len(old_nm)] = '\0';
+	strscpy(dent2->name, fname_name(old_nm), fname_len(old_nm) + 1);
 	set_dent_cookie(c, dent2);
 	zero_dent_node_unused(dent2);
 	ubifs_prep_grp_node(c, dent2, dlen2, 0);
@@ -1897,8 +1892,7 @@ int ubifs_jnl_delete_xattr(struct ubifs_info *c, const struct inode *host,
 	xent->inum = 0;
 	xent->type = get_dent_type(inode->i_mode);
 	xent->nlen = cpu_to_le16(fname_len(nm));
-	memcpy(xent->name, fname_name(nm), fname_len(nm));
-	xent->name[fname_len(nm)] = '\0';
+	strscpy(xent->name, fname_name(nm), fname_len(nm) + 1);
 	zero_dent_node_unused(xent);
 	ubifs_prep_grp_node(c, xent, xlen, 0);
 
diff --git a/fs/ubifs/replay.c b/fs/ubifs/replay.c
index a9a568f4a868..ef6ae63792d1 100644
--- a/fs/ubifs/replay.c
+++ b/fs/ubifs/replay.c
@@ -463,8 +463,7 @@ static int insert_dent(struct ubifs_info *c, int lnum, int offs, int len,
 	r->sqnum = sqnum;
 	key_copy(c, key, &r->key);
 	fname_len(&r->nm) = nlen;
-	memcpy(nbuf, name, nlen);
-	nbuf[nlen] = '\0';
+	strscpy(nbuf, name, nlen + 1);
 	fname_name(&r->nm) = nbuf;
 
 	list_add_tail(&r->list, &c->replay_list);
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 9a77d8b64ffa..048c21271db1 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -168,13 +168,11 @@ struct inode *ubifs_iget(struct super_block *sb, unsigned long inum)
 		inode->i_op = &ubifs_file_inode_operations;
 		inode->i_fop = &ubifs_file_operations;
 		if (ui->xattr) {
-			ui->data = kmalloc(ui->data_len + 1, GFP_NOFS);
+			ui->data = kmemdup_nul(ino->data, ui->data_len, GFP_NOFS);
 			if (!ui->data) {
 				err = -ENOMEM;
 				goto out_ino;
 			}
-			memcpy(ui->data, ino->data, ui->data_len);
-			((char *)ui->data)[ui->data_len] = '\0';
 		} else if (ui->data_len != 0) {
 			err = 10;
 			goto out_invalid;
@@ -194,13 +192,11 @@ struct inode *ubifs_iget(struct super_block *sb, unsigned long inum)
 			err = 12;
 			goto out_invalid;
 		}
-		ui->data = kmalloc(ui->data_len + 1, GFP_NOFS);
+		ui->data = kmemdup_nul(ino->data, ui->data_len, GFP_NOFS);
 		if (!ui->data) {
 			err = -ENOMEM;
 			goto out_ino;
 		}
-		memcpy(ui->data, ino->data, ui->data_len);
-		((char *)ui->data)[ui->data_len] = '\0';
 		break;
 	case S_IFBLK:
 	case S_IFCHR:
-- 
2.53.0


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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 3/3] ubifs: avoid redundant calls to memset()
  2026-04-15  7:18 [PATCH v5 1/3] ubifs: prefer kstrtobool_from_user() over custom helper Dmitry Antipov
  2026-04-15  7:18 ` [PATCH v5 2/3] ubifs: use strscpy() and kmemdup_nul() where appropriate Dmitry Antipov
@ 2026-04-15  7:18 ` Dmitry Antipov
  2026-04-16  3:52   ` Zhihao Cheng
  1 sibling, 1 reply; 7+ messages in thread
From: Dmitry Antipov @ 2026-04-15  7:18 UTC (permalink / raw)
  To: Zhihao Cheng, Richard Weinberger; +Cc: linux-mtd, Dmitry Antipov

Rely on partial implicit initialization of 'struct ubifs_budget_req'
objects and so simplify 'ubifs_release_dirty_inode_budget()', 'do_rename()'
and 'do_truncation()' by dropping explicit calls to 'memset()'.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v5: initial version to join the series
---
 fs/ubifs/budget.c |  9 ++++-----
 fs/ubifs/dir.c    |  4 +---
 fs/ubifs/file.c   | 10 +++++-----
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c
index d76eb7b39f56..a73f1e969f7c 100644
--- a/fs/ubifs/budget.c
+++ b/fs/ubifs/budget.c
@@ -590,11 +590,10 @@ void ubifs_convert_page_budget(struct ubifs_info *c)
 void ubifs_release_dirty_inode_budget(struct ubifs_info *c,
 				      struct ubifs_inode *ui)
 {
-	struct ubifs_budget_req req;
-
-	memset(&req, 0, sizeof(struct ubifs_budget_req));
-	/* The "no space" flags will be cleared because dd_growth is > 0 */
-	req.dd_growth = c->bi.inode_budget + ALIGN(ui->data_len, 8);
+	struct ubifs_budget_req req = {
+		/* The "no space" flags will be cleared because dd_growth is > 0 */
+		.dd_growth = c->bi.inode_budget + ALIGN(ui->data_len, 8)
+	};
 	ubifs_release_budget(c, &req);
 }
 
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 86d41e077e4d..c1602e8aff01 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1335,7 +1335,7 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 					.dirtied_ino = 3 };
 	struct ubifs_budget_req ino_req = { .dirtied_ino = 1,
 			.dirtied_ino_d = ALIGN(old_inode_ui->data_len, 8) };
-	struct ubifs_budget_req wht_req;
+	struct ubifs_budget_req wht_req = { .new_ino = 1 };
 	unsigned int saved_nlink;
 	struct fscrypt_name old_nm, new_nm;
 
@@ -1422,8 +1422,6 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 		whiteout_ui->data_len = ubifs_encode_dev(dev, MKDEV(0, 0));
 		ubifs_assert(c, !whiteout_ui->dirty);
 
-		memset(&wht_req, 0, sizeof(struct ubifs_budget_req));
-		wht_req.new_ino = 1;
 		wht_req.new_ino_d = ALIGN(whiteout_ui->data_len, 8);
 		/*
 		 * To avoid deadlock between space budget (holds ui_mutex and
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index e73c28b12f97..35061a587e3c 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1101,13 +1101,16 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
 			 const struct iattr *attr)
 {
 	int err;
-	struct ubifs_budget_req req;
+	struct ubifs_budget_req req = {
+		.dirtied_ino = 1,
+		/* A funny way to budget for truncation node */
+		.dirtied_ino_d = UBIFS_TRUN_NODE_SZ
+	};
 	loff_t old_size = inode->i_size, new_size = attr->ia_size;
 	int offset = new_size & (UBIFS_BLOCK_SIZE - 1), budgeted = 1;
 	struct ubifs_inode *ui = ubifs_inode(inode);
 
 	dbg_gen("ino %llu, size %lld -> %lld", inode->i_ino, old_size, new_size);
-	memset(&req, 0, sizeof(struct ubifs_budget_req));
 
 	/*
 	 * If this is truncation to a smaller size, and we do not truncate on a
@@ -1117,9 +1120,6 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
 	if (new_size & (UBIFS_BLOCK_SIZE - 1))
 		req.dirtied_page = 1;
 
-	req.dirtied_ino = 1;
-	/* A funny way to budget for truncation node */
-	req.dirtied_ino_d = UBIFS_TRUN_NODE_SZ;
 	err = ubifs_budget_space(c, &req);
 	if (err) {
 		/*
-- 
2.53.0


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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 2/3] ubifs: use strscpy() and kmemdup_nul() where appropriate
  2026-04-15  7:18 ` [PATCH v5 2/3] ubifs: use strscpy() and kmemdup_nul() where appropriate Dmitry Antipov
@ 2026-04-16  3:37   ` Zhihao Cheng
  2026-04-16  5:09     ` Dmitry Antipov
  0 siblings, 1 reply; 7+ messages in thread
From: Zhihao Cheng @ 2026-04-16  3:37 UTC (permalink / raw)
  To: Dmitry Antipov, Richard Weinberger; +Cc: linux-mtd

在 2026/4/15 15:18, Dmitry Antipov 写道:
> Go closer to the modern kernel API and use 'strscpy()' and 'kmemdup_nul()'
> over an ad-hoc ensure-to-have-'\0' quirks where appropriate.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
> v5: switch to fname_len() + 1 to specify strscpy() target buffer size
> v4: once again to not forget a filesystem-native (little) to CPU endian swap
> v3: fix strscpy() usage as noticed by Zhihao
> v2: initial version to join the series
> ---
>   fs/ubifs/journal.c | 18 ++++++------------
>   fs/ubifs/replay.c  |  3 +--
>   fs/ubifs/super.c   |  8 ++------
>   3 files changed, 9 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
> index 40a95a2fad50..8d7f0843d353 100644
> --- a/fs/ubifs/journal.c
> +++ b/fs/ubifs/journal.c
> @@ -729,8 +729,7 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
>   	dent->inum = deletion ? 0 : cpu_to_le64(inode->i_ino);
>   	dent->type = get_dent_type(inode->i_mode);
>   	dent->nlen = cpu_to_le16(fname_len(nm));
> -	memcpy(dent->name, fname_name(nm), fname_len(nm));
> -	dent->name[fname_len(nm)] = '\0';
> +	strscpy(dent->name, fname_name(nm), fname_len(nm) + 1);

Hi Dmitry,
I notice that some architectures can accelerate the implementation of 
memcpy(), and the length of dentry name could be 255, I suggest to keep 
the orignal implementation.
>   	set_dent_cookie(c, dent);
>   
>   	zero_dent_node_unused(dent);
> @@ -1232,8 +1231,7 @@ int ubifs_jnl_xrename(struct ubifs_info *c, const struct inode *fst_dir,
>   	dent1->inum = cpu_to_le64(fst_inode->i_ino);
>   	dent1->type = get_dent_type(fst_inode->i_mode);
>   	dent1->nlen = cpu_to_le16(fname_len(snd_nm));
> -	memcpy(dent1->name, fname_name(snd_nm), fname_len(snd_nm));
> -	dent1->name[fname_len(snd_nm)] = '\0';
> +	strscpy(dent1->name, fname_name(snd_nm), fname_len(snd_nm) + 1);
>   	set_dent_cookie(c, dent1);
>   	zero_dent_node_unused(dent1);
>   	ubifs_prep_grp_node(c, dent1, dlen1, 0);
> @@ -1248,8 +1246,7 @@ int ubifs_jnl_xrename(struct ubifs_info *c, const struct inode *fst_dir,
>   	dent2->inum = cpu_to_le64(snd_inode->i_ino);
>   	dent2->type = get_dent_type(snd_inode->i_mode);
>   	dent2->nlen = cpu_to_le16(fname_len(fst_nm));
> -	memcpy(dent2->name, fname_name(fst_nm), fname_len(fst_nm));
> -	dent2->name[fname_len(fst_nm)] = '\0';
> +	strscpy(dent2->name, fname_name(fst_nm), fname_len(fst_nm) + 1);
>   	set_dent_cookie(c, dent2);
>   	zero_dent_node_unused(dent2);
>   	ubifs_prep_grp_node(c, dent2, dlen2, 0);
> @@ -1424,8 +1421,7 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
>   	dent->inum = cpu_to_le64(old_inode->i_ino);
>   	dent->type = get_dent_type(old_inode->i_mode);
>   	dent->nlen = cpu_to_le16(fname_len(new_nm));
> -	memcpy(dent->name, fname_name(new_nm), fname_len(new_nm));
> -	dent->name[fname_len(new_nm)] = '\0';
> +	strscpy(dent->name, fname_name(new_nm), fname_len(new_nm) + 1);
>   	set_dent_cookie(c, dent);
>   	zero_dent_node_unused(dent);
>   	ubifs_prep_grp_node(c, dent, dlen1, 0);
> @@ -1446,8 +1442,7 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
>   		dent2->type = DT_UNKNOWN;
>   	}
>   	dent2->nlen = cpu_to_le16(fname_len(old_nm));
> -	memcpy(dent2->name, fname_name(old_nm), fname_len(old_nm));
> -	dent2->name[fname_len(old_nm)] = '\0';
> +	strscpy(dent2->name, fname_name(old_nm), fname_len(old_nm) + 1);
>   	set_dent_cookie(c, dent2);
>   	zero_dent_node_unused(dent2);
>   	ubifs_prep_grp_node(c, dent2, dlen2, 0);
> @@ -1897,8 +1892,7 @@ int ubifs_jnl_delete_xattr(struct ubifs_info *c, const struct inode *host,
>   	xent->inum = 0;
>   	xent->type = get_dent_type(inode->i_mode);
>   	xent->nlen = cpu_to_le16(fname_len(nm));
> -	memcpy(xent->name, fname_name(nm), fname_len(nm));
> -	xent->name[fname_len(nm)] = '\0';
> +	strscpy(xent->name, fname_name(nm), fname_len(nm) + 1);
>   	zero_dent_node_unused(xent);
>   	ubifs_prep_grp_node(c, xent, xlen, 0);
>   
> diff --git a/fs/ubifs/replay.c b/fs/ubifs/replay.c
> index a9a568f4a868..ef6ae63792d1 100644
> --- a/fs/ubifs/replay.c
> +++ b/fs/ubifs/replay.c
> @@ -463,8 +463,7 @@ static int insert_dent(struct ubifs_info *c, int lnum, int offs, int len,
>   	r->sqnum = sqnum;
>   	key_copy(c, key, &r->key);
>   	fname_len(&r->nm) = nlen;
> -	memcpy(nbuf, name, nlen);
> -	nbuf[nlen] = '\0';
> +	strscpy(nbuf, name, nlen + 1);
>   	fname_name(&r->nm) = nbuf;
>   
>   	list_add_tail(&r->list, &c->replay_list);
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 9a77d8b64ffa..048c21271db1 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -168,13 +168,11 @@ struct inode *ubifs_iget(struct super_block *sb, unsigned long inum)
>   		inode->i_op = &ubifs_file_inode_operations;
>   		inode->i_fop = &ubifs_file_operations;
>   		if (ui->xattr) {
> -			ui->data = kmalloc(ui->data_len + 1, GFP_NOFS);
> +			ui->data = kmemdup_nul(ino->data, ui->data_len, GFP_NOFS);
>   			if (!ui->data) {
>   				err = -ENOMEM;
>   				goto out_ino;
>   			}
> -			memcpy(ui->data, ino->data, ui->data_len);
> -			((char *)ui->data)[ui->data_len] = '\0';
>   		} else if (ui->data_len != 0) {
>   			err = 10;
>   			goto out_invalid;
> @@ -194,13 +192,11 @@ struct inode *ubifs_iget(struct super_block *sb, unsigned long inum)
>   			err = 12;
>   			goto out_invalid;
>   		}
> -		ui->data = kmalloc(ui->data_len + 1, GFP_NOFS);
> +		ui->data = kmemdup_nul(ino->data, ui->data_len, GFP_NOFS);
>   		if (!ui->data) {
>   			err = -ENOMEM;
>   			goto out_ino;
>   		}
> -		memcpy(ui->data, ino->data, ui->data_len);
> -		((char *)ui->data)[ui->data_len] = '\0';
>   		break;
>   	case S_IFBLK:
>   	case S_IFCHR:
> 


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 3/3] ubifs: avoid redundant calls to memset()
  2026-04-15  7:18 ` [PATCH v5 3/3] ubifs: avoid redundant calls to memset() Dmitry Antipov
@ 2026-04-16  3:52   ` Zhihao Cheng
  0 siblings, 0 replies; 7+ messages in thread
From: Zhihao Cheng @ 2026-04-16  3:52 UTC (permalink / raw)
  To: Dmitry Antipov, Richard Weinberger; +Cc: linux-mtd

在 2026/4/15 15:18, Dmitry Antipov 写道:
> Rely on partial implicit initialization of 'struct ubifs_budget_req'
> objects and so simplify 'ubifs_release_dirty_inode_budget()', 'do_rename()'
> and 'do_truncation()' by dropping explicit calls to 'memset()'.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
> v5: initial version to join the series
> ---
>   fs/ubifs/budget.c |  9 ++++-----
>   fs/ubifs/dir.c    |  4 +---
>   fs/ubifs/file.c   | 10 +++++-----
>   3 files changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c
> index d76eb7b39f56..a73f1e969f7c 100644
> --- a/fs/ubifs/budget.c
> +++ b/fs/ubifs/budget.c
> @@ -590,11 +590,10 @@ void ubifs_convert_page_budget(struct ubifs_info *c)
>   void ubifs_release_dirty_inode_budget(struct ubifs_info *c,
>   				      struct ubifs_inode *ui)
>   {
> -	struct ubifs_budget_req req;
> -
> -	memset(&req, 0, sizeof(struct ubifs_budget_req));
> -	/* The "no space" flags will be cleared because dd_growth is > 0 */
> -	req.dd_growth = c->bi.inode_budget + ALIGN(ui->data_len, 8);
> +	struct ubifs_budget_req req = {
> +		/* The "no space" flags will be cleared because dd_growth is > 0 */
> +		.dd_growth = c->bi.inode_budget + ALIGN(ui->data_len, 8)
> +	};
>   	ubifs_release_budget(c, &req);
>   }
>   
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index 86d41e077e4d..c1602e8aff01 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -1335,7 +1335,7 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
>   					.dirtied_ino = 3 };
>   	struct ubifs_budget_req ino_req = { .dirtied_ino = 1,
>   			.dirtied_ino_d = ALIGN(old_inode_ui->data_len, 8) };
> -	struct ubifs_budget_req wht_req;
> +	struct ubifs_budget_req wht_req = { .new_ino = 1 };
>   	unsigned int saved_nlink;
>   	struct fscrypt_name old_nm, new_nm;
>   
> @@ -1422,8 +1422,6 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
>   		whiteout_ui->data_len = ubifs_encode_dev(dev, MKDEV(0, 0));
>   		ubifs_assert(c, !whiteout_ui->dirty);
>   
> -		memset(&wht_req, 0, sizeof(struct ubifs_budget_req));
> -		wht_req.new_ino = 1;
>   		wht_req.new_ino_d = ALIGN(whiteout_ui->data_len, 8);
>   		/*
>   		 * To avoid deadlock between space budget (holds ui_mutex and

I prefer to keep the orginal logic for do_rename(). it makes 'whiteout' 
and 'wht_req' be initialized in the same code block, which is more readable.
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index e73c28b12f97..35061a587e3c 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -1101,13 +1101,16 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
>   			 const struct iattr *attr)
>   {
>   	int err;
> -	struct ubifs_budget_req req;
> +	struct ubifs_budget_req req = {
> +		.dirtied_ino = 1,
> +		/* A funny way to budget for truncation node */
> +		.dirtied_ino_d = UBIFS_TRUN_NODE_SZ
> +	};
>   	loff_t old_size = inode->i_size, new_size = attr->ia_size;
>   	int offset = new_size & (UBIFS_BLOCK_SIZE - 1), budgeted = 1;
>   	struct ubifs_inode *ui = ubifs_inode(inode);
>   
>   	dbg_gen("ino %llu, size %lld -> %lld", inode->i_ino, old_size, new_size);
> -	memset(&req, 0, sizeof(struct ubifs_budget_req));
>   
>   	/*
>   	 * If this is truncation to a smaller size, and we do not truncate on a
> @@ -1117,9 +1120,6 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
>   	if (new_size & (UBIFS_BLOCK_SIZE - 1))
>   		req.dirtied_page = 1;
>   
> -	req.dirtied_ino = 1;
> -	/* A funny way to budget for truncation node */
> -	req.dirtied_ino_d = UBIFS_TRUN_NODE_SZ;
>   	err = ubifs_budget_space(c, &req);
>   	if (err) {
>   		/*
> 


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 2/3] ubifs: use strscpy() and kmemdup_nul() where appropriate
  2026-04-16  3:37   ` Zhihao Cheng
@ 2026-04-16  5:09     ` Dmitry Antipov
  2026-04-16  6:41       ` Zhihao Cheng
  0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Antipov @ 2026-04-16  5:09 UTC (permalink / raw)
  To: Zhihao Cheng, Richard Weinberger; +Cc: linux-mtd

On Thu, 2026-04-16 at 11:37 +0800, Zhihao Cheng wrote:

> > -	memcpy(dent->name, fname_name(nm), fname_len(nm));
> > -	dent->name[fname_len(nm)] = '\0';
> > +	strscpy(dent->name, fname_name(nm), fname_len(nm) + 1);
> 
> Hi Dmitry,
> I notice that some architectures can accelerate the implementation of 
> memcpy(), and the length of dentry name could be 255, I suggest to keep 
> the orignal implementation.

What about

memcpy_and_pad(dent->name, fname_len(nm) + 1, fname_name(nm), fname_len(nm), 0)?

I have a strong suspicion that all of the '\0' things should be left to
library functions. So this is not a style but rather an overall design point -
if you're inserting '\0' at your own, most likely you're doing something
wrong, at least because you're duplicating something which is already
implemented in a library function.

Dmitry

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 2/3] ubifs: use strscpy() and kmemdup_nul() where appropriate
  2026-04-16  5:09     ` Dmitry Antipov
@ 2026-04-16  6:41       ` Zhihao Cheng
  0 siblings, 0 replies; 7+ messages in thread
From: Zhihao Cheng @ 2026-04-16  6:41 UTC (permalink / raw)
  To: Dmitry Antipov, Richard Weinberger; +Cc: linux-mtd

在 2026/4/16 13:09, Dmitry Antipov 写道:
> On Thu, 2026-04-16 at 11:37 +0800, Zhihao Cheng wrote:
> 
>>> -	memcpy(dent->name, fname_name(nm), fname_len(nm));
>>> -	dent->name[fname_len(nm)] = '\0';
>>> +	strscpy(dent->name, fname_name(nm), fname_len(nm) + 1);
>>
>> Hi Dmitry,
>> I notice that some architectures can accelerate the implementation of
>> memcpy(), and the length of dentry name could be 255, I suggest to keep
>> the orignal implementation.
> 
> What about
> 
> memcpy_and_pad(dent->name, fname_len(nm) + 1, fname_name(nm), fname_len(nm), 0)?

I weakly accept memcpy_and_pad().
> 
> I have a strong suspicion that all of the '\0' things should be left to
> library functions. So this is not a style but rather an overall design point -
> if you're inserting '\0' at your own, most likely you're doing something
> wrong, at least because you're duplicating something which is already
> implemented in a library function.

Well, kernel developers, of course, need to understand when and how to 
set the trailing '\0'. There are many other places setting trailing 
'\0'(eg. xfs_init_local_fork, xfs_symlink_remote_read, 
ext4_xattr_move_to_block, etc.). I don't think there's a problem with 
this; the developers know what they're doing.
> 
> Dmitry
> .
> 


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-04-16  6:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-15  7:18 [PATCH v5 1/3] ubifs: prefer kstrtobool_from_user() over custom helper Dmitry Antipov
2026-04-15  7:18 ` [PATCH v5 2/3] ubifs: use strscpy() and kmemdup_nul() where appropriate Dmitry Antipov
2026-04-16  3:37   ` Zhihao Cheng
2026-04-16  5:09     ` Dmitry Antipov
2026-04-16  6:41       ` Zhihao Cheng
2026-04-15  7:18 ` [PATCH v5 3/3] ubifs: avoid redundant calls to memset() Dmitry Antipov
2026-04-16  3:52   ` Zhihao Cheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox