All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/11] Some bugfixs for ubifs/ubi
@ 2021-10-25  3:41 ` Zhihao Cheng
  0 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1


Zhihao Cheng (11):
  ubifs: rename_whiteout: Fix double free for whiteout_ui->data
  ubifs: Fix deadlock in concurrent rename whiteout and inode writeback
  ubifs: Fix wrong number of inodes locked by ui_mutex in ubifs_inode
    comment
  ubifs: Add missing iput if do_tmpfile() failed in rename whiteout
  ubifs: Rename whiteout atomically
  ubifs: Fix 'ui->dirty' race between do_tmpfile() and writeback work
  ubifs: Rectify space amount budget for mkdir/tmpfile operations
  ubifs: setflags: Don't make a budget for 'ui->data_len'
  ubifs: Fix read out-of-bounds in ubifs_wbuf_write_nolock()
  ubi: fastmap: Return error code if memory allocation fails in
    add_aeb()
  ubi: fastmap: Add all fastmap pebs into 'ai->fastmap' when
    fm->used_blocks>=2

 drivers/mtd/ubi/fastmap.c |  69 ++++++-----
 fs/ubifs/dir.c            | 235 +++++++++++++++++++++++---------------
 fs/ubifs/io.c             |  34 +++++-
 fs/ubifs/ioctl.c          |   4 +-
 fs/ubifs/journal.c        |  53 +++++++--
 fs/ubifs/ubifs.h          |   2 +-
 6 files changed, 254 insertions(+), 143 deletions(-)

-- 
2.31.1


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

* [PATCH 00/11] Some bugfixs for ubifs/ubi
@ 2021-10-25  3:41 ` Zhihao Cheng
  0 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1


Zhihao Cheng (11):
  ubifs: rename_whiteout: Fix double free for whiteout_ui->data
  ubifs: Fix deadlock in concurrent rename whiteout and inode writeback
  ubifs: Fix wrong number of inodes locked by ui_mutex in ubifs_inode
    comment
  ubifs: Add missing iput if do_tmpfile() failed in rename whiteout
  ubifs: Rename whiteout atomically
  ubifs: Fix 'ui->dirty' race between do_tmpfile() and writeback work
  ubifs: Rectify space amount budget for mkdir/tmpfile operations
  ubifs: setflags: Don't make a budget for 'ui->data_len'
  ubifs: Fix read out-of-bounds in ubifs_wbuf_write_nolock()
  ubi: fastmap: Return error code if memory allocation fails in
    add_aeb()
  ubi: fastmap: Add all fastmap pebs into 'ai->fastmap' when
    fm->used_blocks>=2

 drivers/mtd/ubi/fastmap.c |  69 ++++++-----
 fs/ubifs/dir.c            | 235 +++++++++++++++++++++++---------------
 fs/ubifs/io.c             |  34 +++++-
 fs/ubifs/ioctl.c          |   4 +-
 fs/ubifs/journal.c        |  53 +++++++--
 fs/ubifs/ubifs.h          |   2 +-
 6 files changed, 254 insertions(+), 143 deletions(-)

-- 
2.31.1


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

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

* [PATCH 01/11] ubifs: rename_whiteout: Fix double free for whiteout_ui->data
  2021-10-25  3:41 ` Zhihao Cheng
@ 2021-10-25  3:41   ` Zhihao Cheng
  -1 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

'whiteout_ui->data' will be freed twice if space budget fail for
rename whiteout operation as following process:

rename_whiteout
  dev = kmalloc
  whiteout_ui->data = dev
  kfree(whiteout_ui->data)  // Free first time
  iput(whiteout)
    ubifs_free_inode
      kfree(ui->data)	    // Double free!

KASAN reports:
==================================================================
BUG: KASAN: double-free or invalid-free in ubifs_free_inode+0x4f/0x70
Call Trace:
  kfree+0x117/0x490
  ubifs_free_inode+0x4f/0x70 [ubifs]
  i_callback+0x30/0x60
  rcu_do_batch+0x366/0xac0
  __do_softirq+0x133/0x57f

Allocated by task 1506:
  kmem_cache_alloc_trace+0x3c2/0x7a0
  do_rename+0x9b7/0x1150 [ubifs]
  ubifs_rename+0x106/0x1f0 [ubifs]
  do_syscall_64+0x35/0x80

Freed by task 1506:
  kfree+0x117/0x490
  do_rename.cold+0x53/0x8a [ubifs]
  ubifs_rename+0x106/0x1f0 [ubifs]
  do_syscall_64+0x35/0x80

The buggy address belongs to the object at ffff88810238bed8 which
belongs to the cache kmalloc-8 of size 8
==================================================================

Let ubifs_free_inode() free 'whiteout_ui->data'. BTW, delete unused
assignment 'whiteout_ui->data_len = 0', process 'ubifs_evict_inode()
-> ubifs_jnl_delete_inode() -> ubifs_jnl_write_inode()' doesn't need it
(because 'inc_nlink(whiteout)' won't be excuted by 'goto out_release',
 and the nlink of whiteout inode is 0).

Fixes: 9e0a1fff8db56ea ("ubifs: Implement RENAME_WHITEOUT")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/dir.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 7c61d0ec0159..cfa8881d8cca 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1425,8 +1425,6 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 
 		err = ubifs_budget_space(c, &wht_req);
 		if (err) {
-			kfree(whiteout_ui->data);
-			whiteout_ui->data_len = 0;
 			iput(whiteout);
 			goto out_release;
 		}
-- 
2.31.1


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

* [PATCH 01/11] ubifs: rename_whiteout: Fix double free for whiteout_ui->data
@ 2021-10-25  3:41   ` Zhihao Cheng
  0 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

'whiteout_ui->data' will be freed twice if space budget fail for
rename whiteout operation as following process:

rename_whiteout
  dev = kmalloc
  whiteout_ui->data = dev
  kfree(whiteout_ui->data)  // Free first time
  iput(whiteout)
    ubifs_free_inode
      kfree(ui->data)	    // Double free!

KASAN reports:
==================================================================
BUG: KASAN: double-free or invalid-free in ubifs_free_inode+0x4f/0x70
Call Trace:
  kfree+0x117/0x490
  ubifs_free_inode+0x4f/0x70 [ubifs]
  i_callback+0x30/0x60
  rcu_do_batch+0x366/0xac0
  __do_softirq+0x133/0x57f

Allocated by task 1506:
  kmem_cache_alloc_trace+0x3c2/0x7a0
  do_rename+0x9b7/0x1150 [ubifs]
  ubifs_rename+0x106/0x1f0 [ubifs]
  do_syscall_64+0x35/0x80

Freed by task 1506:
  kfree+0x117/0x490
  do_rename.cold+0x53/0x8a [ubifs]
  ubifs_rename+0x106/0x1f0 [ubifs]
  do_syscall_64+0x35/0x80

The buggy address belongs to the object at ffff88810238bed8 which
belongs to the cache kmalloc-8 of size 8
==================================================================

Let ubifs_free_inode() free 'whiteout_ui->data'. BTW, delete unused
assignment 'whiteout_ui->data_len = 0', process 'ubifs_evict_inode()
-> ubifs_jnl_delete_inode() -> ubifs_jnl_write_inode()' doesn't need it
(because 'inc_nlink(whiteout)' won't be excuted by 'goto out_release',
 and the nlink of whiteout inode is 0).

Fixes: 9e0a1fff8db56ea ("ubifs: Implement RENAME_WHITEOUT")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/dir.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 7c61d0ec0159..cfa8881d8cca 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1425,8 +1425,6 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 
 		err = ubifs_budget_space(c, &wht_req);
 		if (err) {
-			kfree(whiteout_ui->data);
-			whiteout_ui->data_len = 0;
 			iput(whiteout);
 			goto out_release;
 		}
-- 
2.31.1


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

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

* [PATCH 02/11] ubifs: Fix deadlock in concurrent rename whiteout and inode writeback
  2021-10-25  3:41 ` Zhihao Cheng
@ 2021-10-25  3:41   ` Zhihao Cheng
  -1 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

Following hung tasks:
[   77.028764] task:kworker/u8:4    state:D stack:    0 pid:  132
[   77.028820] Call Trace:
[   77.029027]  schedule+0x8c/0x1b0
[   77.029067]  mutex_lock+0x50/0x60
[   77.029074]  ubifs_write_inode+0x68/0x1f0 [ubifs]
[   77.029117]  __writeback_single_inode+0x43c/0x570
[   77.029128]  writeback_sb_inodes+0x259/0x740
[   77.029148]  wb_writeback+0x107/0x4d0
[   77.029163]  wb_workfn+0x162/0x7b0

[   92.390442] task:aa              state:D stack:    0 pid: 1506
[   92.390448] Call Trace:
[   92.390458]  schedule+0x8c/0x1b0
[   92.390461]  wb_wait_for_completion+0x82/0xd0
[   92.390469]  __writeback_inodes_sb_nr+0xb2/0x110
[   92.390472]  writeback_inodes_sb_nr+0x14/0x20
[   92.390476]  ubifs_budget_space+0x705/0xdd0 [ubifs]
[   92.390503]  do_rename.cold+0x7f/0x187 [ubifs]
[   92.390549]  ubifs_rename+0x8b/0x180 [ubifs]
[   92.390571]  vfs_rename+0xdb2/0x1170
[   92.390580]  do_renameat2+0x554/0x770

, are caused by concurrent rename whiteout and inode writeback processes:
	rename_whiteout(Thread 1)	        wb_workfn(Thread2)
ubifs_rename
  do_rename
    lock_4_inodes (Hold ui_mutex)
    ubifs_budget_space
      make_free_space
        shrink_liability
	  __writeback_inodes_sb_nr
	    bdi_split_work_to_wbs (Queue new wb work)
					      wb_do_writeback(wb work)
						__writeback_single_inode
					          ubifs_write_inode
					            LOCK(ui_mutex)
							   ↑
	      wb_wait_for_completion (Wait wb work) <-- deadlock!

Reproducer (Detail program in [Link]):
  1. SYS_renameat2("/mp/dir/file", "/mp/dir/whiteout", RENAME_WHITEOUT)
  2. Consume out of space before kernel(mdelay) doing budget for whiteout

Fix it by doing whiteout space budget before locking ubifs inodes.
BTW, it also fixes wrong goto tag 'out_release' in whiteout budget
error handling path(It should at least recover dir i_size and unlock
4 ubifs inodes).

Fixes: 9e0a1fff8db56ea ("ubifs: Implement RENAME_WHITEOUT")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=214733
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/dir.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index cfa8881d8cca..2735ad1affed 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1324,6 +1324,7 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 
 	if (flags & RENAME_WHITEOUT) {
 		union ubifs_dev_desc *dev = NULL;
+		struct ubifs_budget_req wht_req;
 
 		dev = kmalloc(sizeof(union ubifs_dev_desc), GFP_NOFS);
 		if (!dev) {
@@ -1345,6 +1346,20 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 		whiteout_ui->data = dev;
 		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.dirtied_ino = 1;
+		wht_req.dirtied_ino_d = ALIGN(whiteout_ui->data_len, 8);
+		/*
+		 * To avoid deadlock between space budget (holds ui_mutex and
+		 * waits wb work) and writeback work(waits ui_mutex), do space
+		 * budget before ubifs inodes locked.
+		 */
+		err = ubifs_budget_space(c, &wht_req);
+		if (err) {
+			iput(whiteout);
+			goto out_release;
+		}
 	}
 
 	lock_4_inodes(old_dir, new_dir, new_inode, whiteout);
@@ -1419,16 +1434,6 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 	}
 
 	if (whiteout) {
-		struct ubifs_budget_req wht_req = { .dirtied_ino = 1,
-				.dirtied_ino_d = \
-				ALIGN(ubifs_inode(whiteout)->data_len, 8) };
-
-		err = ubifs_budget_space(c, &wht_req);
-		if (err) {
-			iput(whiteout);
-			goto out_release;
-		}
-
 		inc_nlink(whiteout);
 		mark_inode_dirty(whiteout);
 
-- 
2.31.1


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

* [PATCH 02/11] ubifs: Fix deadlock in concurrent rename whiteout and inode writeback
@ 2021-10-25  3:41   ` Zhihao Cheng
  0 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

Following hung tasks:
[   77.028764] task:kworker/u8:4    state:D stack:    0 pid:  132
[   77.028820] Call Trace:
[   77.029027]  schedule+0x8c/0x1b0
[   77.029067]  mutex_lock+0x50/0x60
[   77.029074]  ubifs_write_inode+0x68/0x1f0 [ubifs]
[   77.029117]  __writeback_single_inode+0x43c/0x570
[   77.029128]  writeback_sb_inodes+0x259/0x740
[   77.029148]  wb_writeback+0x107/0x4d0
[   77.029163]  wb_workfn+0x162/0x7b0

[   92.390442] task:aa              state:D stack:    0 pid: 1506
[   92.390448] Call Trace:
[   92.390458]  schedule+0x8c/0x1b0
[   92.390461]  wb_wait_for_completion+0x82/0xd0
[   92.390469]  __writeback_inodes_sb_nr+0xb2/0x110
[   92.390472]  writeback_inodes_sb_nr+0x14/0x20
[   92.390476]  ubifs_budget_space+0x705/0xdd0 [ubifs]
[   92.390503]  do_rename.cold+0x7f/0x187 [ubifs]
[   92.390549]  ubifs_rename+0x8b/0x180 [ubifs]
[   92.390571]  vfs_rename+0xdb2/0x1170
[   92.390580]  do_renameat2+0x554/0x770

, are caused by concurrent rename whiteout and inode writeback processes:
	rename_whiteout(Thread 1)	        wb_workfn(Thread2)
ubifs_rename
  do_rename
    lock_4_inodes (Hold ui_mutex)
    ubifs_budget_space
      make_free_space
        shrink_liability
	  __writeback_inodes_sb_nr
	    bdi_split_work_to_wbs (Queue new wb work)
					      wb_do_writeback(wb work)
						__writeback_single_inode
					          ubifs_write_inode
					            LOCK(ui_mutex)
							   ↑
	      wb_wait_for_completion (Wait wb work) <-- deadlock!

Reproducer (Detail program in [Link]):
  1. SYS_renameat2("/mp/dir/file", "/mp/dir/whiteout", RENAME_WHITEOUT)
  2. Consume out of space before kernel(mdelay) doing budget for whiteout

Fix it by doing whiteout space budget before locking ubifs inodes.
BTW, it also fixes wrong goto tag 'out_release' in whiteout budget
error handling path(It should at least recover dir i_size and unlock
4 ubifs inodes).

Fixes: 9e0a1fff8db56ea ("ubifs: Implement RENAME_WHITEOUT")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=214733
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/dir.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index cfa8881d8cca..2735ad1affed 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1324,6 +1324,7 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 
 	if (flags & RENAME_WHITEOUT) {
 		union ubifs_dev_desc *dev = NULL;
+		struct ubifs_budget_req wht_req;
 
 		dev = kmalloc(sizeof(union ubifs_dev_desc), GFP_NOFS);
 		if (!dev) {
@@ -1345,6 +1346,20 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 		whiteout_ui->data = dev;
 		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.dirtied_ino = 1;
+		wht_req.dirtied_ino_d = ALIGN(whiteout_ui->data_len, 8);
+		/*
+		 * To avoid deadlock between space budget (holds ui_mutex and
+		 * waits wb work) and writeback work(waits ui_mutex), do space
+		 * budget before ubifs inodes locked.
+		 */
+		err = ubifs_budget_space(c, &wht_req);
+		if (err) {
+			iput(whiteout);
+			goto out_release;
+		}
 	}
 
 	lock_4_inodes(old_dir, new_dir, new_inode, whiteout);
@@ -1419,16 +1434,6 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 	}
 
 	if (whiteout) {
-		struct ubifs_budget_req wht_req = { .dirtied_ino = 1,
-				.dirtied_ino_d = \
-				ALIGN(ubifs_inode(whiteout)->data_len, 8) };
-
-		err = ubifs_budget_space(c, &wht_req);
-		if (err) {
-			iput(whiteout);
-			goto out_release;
-		}
-
 		inc_nlink(whiteout);
 		mark_inode_dirty(whiteout);
 
-- 
2.31.1


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

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

* [PATCH 03/11] ubifs: Fix wrong number of inodes locked by ui_mutex in ubifs_inode comment
  2021-10-25  3:41 ` Zhihao Cheng
@ 2021-10-25  3:41   ` Zhihao Cheng
  -1 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

Since 9ec64962afb1702f75b("ubifs: Implement RENAME_EXCHANGE") and
9e0a1fff8db56eaaebb("ubifs: Implement RENAME_WHITEOUT") are applied,
ubifs_rename locks and changes 4 ubifs inodes, correct the comment
for ui_mutex in ubifs_inode.

Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/ubifs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index c38066ce9ab0..972e41daff01 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -372,7 +372,7 @@ struct ubifs_gced_idx_leb {
  * @ui_mutex exists for two main reasons. At first it prevents inodes from
  * being written back while UBIFS changing them, being in the middle of an VFS
  * operation. This way UBIFS makes sure the inode fields are consistent. For
- * example, in 'ubifs_rename()' we change 3 inodes simultaneously, and
+ * example, in 'ubifs_rename()' we change 4 inodes simultaneously, and
  * write-back must not write any of them before we have finished.
  *
  * The second reason is budgeting - UBIFS has to budget all operations. If an
-- 
2.31.1


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

* [PATCH 03/11] ubifs: Fix wrong number of inodes locked by ui_mutex in ubifs_inode comment
@ 2021-10-25  3:41   ` Zhihao Cheng
  0 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

Since 9ec64962afb1702f75b("ubifs: Implement RENAME_EXCHANGE") and
9e0a1fff8db56eaaebb("ubifs: Implement RENAME_WHITEOUT") are applied,
ubifs_rename locks and changes 4 ubifs inodes, correct the comment
for ui_mutex in ubifs_inode.

Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/ubifs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index c38066ce9ab0..972e41daff01 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -372,7 +372,7 @@ struct ubifs_gced_idx_leb {
  * @ui_mutex exists for two main reasons. At first it prevents inodes from
  * being written back while UBIFS changing them, being in the middle of an VFS
  * operation. This way UBIFS makes sure the inode fields are consistent. For
- * example, in 'ubifs_rename()' we change 3 inodes simultaneously, and
+ * example, in 'ubifs_rename()' we change 4 inodes simultaneously, and
  * write-back must not write any of them before we have finished.
  *
  * The second reason is budgeting - UBIFS has to budget all operations. If an
-- 
2.31.1


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

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

* [PATCH 04/11] ubifs: Add missing iput if do_tmpfile() failed in rename whiteout
  2021-10-25  3:41 ` Zhihao Cheng
@ 2021-10-25  3:41   ` Zhihao Cheng
  -1 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

whiteout inode should be put when do_tmpfile() failed if inode has been
initialized. Otherwise we will get following warning during umount:
  UBIFS error (ubi0:0 pid 1494): ubifs_assert_failed [ubifs]: UBIFS
  assert failed: c->bi.dd_growth == 0, in fs/ubifs/super.c:1930
  VFS: Busy inodes after unmount of ubifs. Self-destruct in 5 seconds.

Fixes: 9e0a1fff8db56ea ("ubifs: Implement RENAME_WHITEOUT")
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 2735ad1affed..6503e6857f6e 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1334,6 +1334,8 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 
 		err = do_tmpfile(old_dir, old_dentry, S_IFCHR | WHITEOUT_MODE, &whiteout);
 		if (err) {
+			if (whiteout)
+				iput(whiteout);
 			kfree(dev);
 			goto out_release;
 		}
-- 
2.31.1


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

* [PATCH 04/11] ubifs: Add missing iput if do_tmpfile() failed in rename whiteout
@ 2021-10-25  3:41   ` Zhihao Cheng
  0 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

whiteout inode should be put when do_tmpfile() failed if inode has been
initialized. Otherwise we will get following warning during umount:
  UBIFS error (ubi0:0 pid 1494): ubifs_assert_failed [ubifs]: UBIFS
  assert failed: c->bi.dd_growth == 0, in fs/ubifs/super.c:1930
  VFS: Busy inodes after unmount of ubifs. Self-destruct in 5 seconds.

Fixes: 9e0a1fff8db56ea ("ubifs: Implement RENAME_WHITEOUT")
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 2735ad1affed..6503e6857f6e 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1334,6 +1334,8 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 
 		err = do_tmpfile(old_dir, old_dentry, S_IFCHR | WHITEOUT_MODE, &whiteout);
 		if (err) {
+			if (whiteout)
+				iput(whiteout);
 			kfree(dev);
 			goto out_release;
 		}
-- 
2.31.1


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

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

* [PATCH 05/11] ubifs: Rename whiteout atomically
  2021-10-25  3:41 ` Zhihao Cheng
@ 2021-10-25  3:41   ` Zhihao Cheng
  -1 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

Currently, rename whiteout has 3 steps:
  1. create tmpfile(which associates old dentry to tmpfile inode) for
     whiteout, and store tmpfile to disk
  2. link whiteout, associate whiteout inode to old dentry agagin and
     store old dentry, old inode, new dentry on disk
  3. writeback dirty whiteout inode to disk

Suddenly power-cut or error occurring(eg. ENOSPC returned by budget,
memory allocation failure) during above steps may cause kinds of problems:
  Problem 1: ENOSPC returned by whiteout space budget (before step 2),
	     old dentry will disappear after rename syscall, whiteout file
	     cannot be found either.

	     ls dir  // we get file, whiteout
	     rename(dir/file, dir/whiteout, REANME_WHITEOUT)
	     ENOSPC = ubifs_budget_space(&wht_req) // return
	     ls dir  // empty (no file, no whiteout)
  Problem 2: Power-cut happens before step 3, whiteout inode with 'nlink=1'
	     is not stored on disk, whiteout dentry(old dentry) is written
	     on disk, whiteout file is lost on next mount (We get "dead
	     directory entry" after executing 'ls -l' on whiteout file).

Now, we use following 3 steps to finish rename whiteout:
  1. create an in-mem inode with 'nlink = 1' as whiteout
  2. ubifs_jnl_rename (Write on disk to finish associating old dentry to
     whiteout inode, associating new dentry with old inode)
  3. iput(whiteout)

Rely writing in-mem inode on disk by ubifs_jnl_rename() to finish rename
whiteout, which avoids middle disk state caused by suddenly power-cut
and error occurring.

Fixes: 9e0a1fff8db56ea ("ubifs: Implement RENAME_WHITEOUT")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/dir.c     | 144 +++++++++++++++++++++++++++++----------------
 fs/ubifs/journal.c |  53 ++++++++++++++---
 2 files changed, 137 insertions(+), 60 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 6503e6857f6e..6344e2bc9338 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -349,8 +349,58 @@ static int ubifs_create(struct user_namespace *mnt_userns, struct inode *dir,
 	return err;
 }
 
-static int do_tmpfile(struct inode *dir, struct dentry *dentry,
-		      umode_t mode, struct inode **whiteout)
+static struct inode *create_whiteout(struct inode *dir, struct dentry *dentry,
+				     umode_t mode)
+{
+	int err;
+	struct inode *inode;
+	struct ubifs_inode *ui;
+	struct ubifs_info *c = dir->i_sb->s_fs_info;
+	struct fscrypt_name nm;
+
+	/*
+	 * Create an inode('nlink = 1') for whiteout without updating journal,
+	 * let ubifs_jnl_rename() store it on flash to complete rename whiteout
+	 * atomically.
+	 */
+
+	dbg_gen("dent '%pd', mode %#hx in dir ino %lu",
+		dentry, mode, dir->i_ino);
+
+	err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm);
+	if (err)
+		return ERR_PTR(err);
+
+	inode = ubifs_new_inode(c, dir, mode);
+	if (IS_ERR(inode)) {
+		err = PTR_ERR(inode);
+		goto out_free;
+	}
+	ui = ubifs_inode(inode);
+
+	init_special_inode(inode, inode->i_mode, WHITEOUT_DEV);
+	ubifs_assert(c, inode->i_op == &ubifs_file_inode_operations);
+
+	err = ubifs_init_security(dir, inode, &dentry->d_name);
+	if (err)
+		goto out_inode;
+
+	/* The dir size is updated by do_rename. */
+	insert_inode_hash(inode);
+
+	return inode;
+
+out_inode:
+	make_bad_inode(inode);
+	iput(inode);
+out_free:
+	fscrypt_free_filename(&nm);
+	ubifs_err(c, "cannot create whiteout file, error %d", err);
+	return ERR_PTR(err);
+}
+
+static int ubifs_tmpfile(struct user_namespace *mnt_userns, struct inode *dir,
+			 struct dentry *dentry, umode_t mode)
 {
 	struct inode *inode;
 	struct ubifs_info *c = dir->i_sb->s_fs_info;
@@ -392,25 +442,13 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry,
 	}
 	ui = ubifs_inode(inode);
 
-	if (whiteout) {
-		init_special_inode(inode, inode->i_mode, WHITEOUT_DEV);
-		ubifs_assert(c, inode->i_op == &ubifs_file_inode_operations);
-	}
-
 	err = ubifs_init_security(dir, inode, &dentry->d_name);
 	if (err)
 		goto out_inode;
 
 	mutex_lock(&ui->ui_mutex);
 	insert_inode_hash(inode);
-
-	if (whiteout) {
-		mark_inode_dirty(inode);
-		drop_nlink(inode);
-		*whiteout = inode;
-	} else {
-		d_tmpfile(dentry, inode);
-	}
+	d_tmpfile(dentry, inode);
 	ubifs_assert(c, ui->dirty);
 
 	instantiated = 1;
@@ -441,12 +479,6 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry,
 	return err;
 }
 
-static int ubifs_tmpfile(struct user_namespace *mnt_userns, struct inode *dir,
-			 struct dentry *dentry, umode_t mode)
-{
-	return do_tmpfile(dir, dentry, mode, NULL);
-}
-
 /**
  * vfs_dent_type - get VFS directory entry type.
  * @type: UBIFS directory entry type
@@ -1264,17 +1296,19 @@ 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 timespec64 time;
 	unsigned int saved_nlink;
 	struct fscrypt_name old_nm, new_nm;
 
 	/*
-	 * Budget request settings: deletion direntry, new direntry, removing
-	 * the old inode, and changing old and new parent directory inodes.
+	 * Budget request settings:
+	 *   req: deletion direntry, new direntry, removing the old inode,
+	 *   and changing old and new parent directory inodes.
+	 *
+	 *   wht_req: new whiteout inode for RENAME_WHITEOUT.
 	 *
-	 * However, this operation also marks the target inode as dirty and
-	 * does not write it, so we allocate budget for the target inode
-	 * separately.
+	 *   ino_req: marks the target inode as dirty and does not write it.
 	 */
 
 	dbg_gen("dent '%pd' ino %lu in dir ino %lu to dent '%pd' in dir ino %lu flags 0x%x",
@@ -1324,7 +1358,6 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 
 	if (flags & RENAME_WHITEOUT) {
 		union ubifs_dev_desc *dev = NULL;
-		struct ubifs_budget_req wht_req;
 
 		dev = kmalloc(sizeof(union ubifs_dev_desc), GFP_NOFS);
 		if (!dev) {
@@ -1332,26 +1365,27 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 			goto out_release;
 		}
 
-		err = do_tmpfile(old_dir, old_dentry, S_IFCHR | WHITEOUT_MODE, &whiteout);
-		if (err) {
-			if (whiteout)
-				iput(whiteout);
+		/*
+		 * The whiteout inode without dentry is pinned in memory,
+		 * umount won't happen during rename process because we
+		 * got parent dentry.
+		 */
+		whiteout = create_whiteout(old_dir, old_dentry,
+					   S_IFCHR | WHITEOUT_MODE);
+		if (IS_ERR(whiteout)) {
+			err = PTR_ERR(whiteout);
 			kfree(dev);
 			goto out_release;
 		}
 
-		spin_lock(&whiteout->i_lock);
-		whiteout->i_state |= I_LINKABLE;
-		spin_unlock(&whiteout->i_lock);
-
 		whiteout_ui = ubifs_inode(whiteout);
 		whiteout_ui->data = dev;
 		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.dirtied_ino = 1;
-		wht_req.dirtied_ino_d = ALIGN(whiteout_ui->data_len, 8);
+		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
 		 * waits wb work) and writeback work(waits ui_mutex), do space
@@ -1359,6 +1393,11 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 		 */
 		err = ubifs_budget_space(c, &wht_req);
 		if (err) {
+			/*
+			 * Whiteout inode can not be written on flash by
+			 * ubifs_jnl_write_inode(), because it's neither
+			 * dirty nor zero-nlink.
+			 */
 			iput(whiteout);
 			goto out_release;
 		}
@@ -1433,17 +1472,8 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 		sync = IS_DIRSYNC(old_dir) || IS_DIRSYNC(new_dir);
 		if (unlink && IS_SYNC(new_inode))
 			sync = 1;
-	}
-
-	if (whiteout) {
-		inc_nlink(whiteout);
-		mark_inode_dirty(whiteout);
-
-		spin_lock(&whiteout->i_lock);
-		whiteout->i_state &= ~I_LINKABLE;
-		spin_unlock(&whiteout->i_lock);
-
-		iput(whiteout);
+		if (whiteout && IS_SYNC(whiteout))
+			sync = 1;
 	}
 
 	err = ubifs_jnl_rename(c, old_dir, old_inode, &old_nm, new_dir,
@@ -1454,6 +1484,11 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 	unlock_4_inodes(old_dir, new_dir, new_inode, whiteout);
 	ubifs_release_budget(c, &req);
 
+	if (whiteout) {
+		ubifs_release_budget(c, &wht_req);
+		iput(whiteout);
+	}
+
 	mutex_lock(&old_inode_ui->ui_mutex);
 	release = old_inode_ui->dirty;
 	mark_inode_dirty_sync(old_inode);
@@ -1462,11 +1497,16 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 	if (release)
 		ubifs_release_budget(c, &ino_req);
 	if (IS_SYNC(old_inode))
-		err = old_inode->i_sb->s_op->write_inode(old_inode, NULL);
+		/*
+		 * Rename finished here. Although old inode cannot be updated
+		 * on flash, old ctime is not a big problem, don't return err
+		 * code to userspace.
+		 */
+		old_inode->i_sb->s_op->write_inode(old_inode, NULL);
 
 	fscrypt_free_filename(&old_nm);
 	fscrypt_free_filename(&new_nm);
-	return err;
+	return 0;
 
 out_cancel:
 	if (unlink) {
@@ -1487,11 +1527,11 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 				inc_nlink(old_dir);
 		}
 	}
+	unlock_4_inodes(old_dir, new_dir, new_inode, whiteout);
 	if (whiteout) {
-		drop_nlink(whiteout);
+		ubifs_release_budget(c, &wht_req);
 		iput(whiteout);
 	}
-	unlock_4_inodes(old_dir, new_dir, new_inode, whiteout);
 out_release:
 	ubifs_release_budget(c, &ino_req);
 	ubifs_release_budget(c, &req);
diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index 8ea680dba61e..a77821f922e2 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -1207,9 +1207,9 @@ int ubifs_jnl_xrename(struct ubifs_info *c, const struct inode *fst_dir,
  * @sync: non-zero if the write-buffer has to be synchronized
  *
  * This function implements the re-name operation which may involve writing up
- * to 4 inodes and 2 directory entries. It marks the written inodes as clean
- * and returns zero on success. In case of failure, a negative error code is
- * returned.
+ * to 4 inodes(new inode, whiteout inode, old and new parent directory inodes)
+ * and 2 directory entries. It marks the written inodes as clean and returns
+ * zero on success. In case of failure, a negative error code is returned.
  */
 int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 		     const struct inode *old_inode,
@@ -1222,14 +1222,15 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 	void *p;
 	union ubifs_key key;
 	struct ubifs_dent_node *dent, *dent2;
-	int err, dlen1, dlen2, ilen, lnum, offs, len, orphan_added = 0;
+	int err, dlen1, dlen2, ilen, wlen, lnum, offs, len, orphan_added = 0;
 	int aligned_dlen1, aligned_dlen2, plen = UBIFS_INO_NODE_SZ;
 	int last_reference = !!(new_inode && new_inode->i_nlink == 0);
 	int move = (old_dir != new_dir);
-	struct ubifs_inode *new_ui;
+	struct ubifs_inode *new_ui, *whiteout_ui;
 	u8 hash_old_dir[UBIFS_HASH_ARR_SZ];
 	u8 hash_new_dir[UBIFS_HASH_ARR_SZ];
 	u8 hash_new_inode[UBIFS_HASH_ARR_SZ];
+	u8 hash_whiteout_inode[UBIFS_HASH_ARR_SZ];
 	u8 hash_dent1[UBIFS_HASH_ARR_SZ];
 	u8 hash_dent2[UBIFS_HASH_ARR_SZ];
 
@@ -1249,9 +1250,20 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 	} else
 		ilen = 0;
 
+	if (whiteout) {
+		whiteout_ui = ubifs_inode(whiteout);
+		ubifs_assert(c, mutex_is_locked(&whiteout_ui->ui_mutex));
+		ubifs_assert(c, whiteout->i_nlink == 1);
+		ubifs_assert(c, !whiteout_ui->dirty);
+		wlen = UBIFS_INO_NODE_SZ;
+		wlen += whiteout_ui->data_len;
+	} else
+		wlen = 0;
+
 	aligned_dlen1 = ALIGN(dlen1, 8);
 	aligned_dlen2 = ALIGN(dlen2, 8);
-	len = aligned_dlen1 + aligned_dlen2 + ALIGN(ilen, 8) + ALIGN(plen, 8);
+	len = aligned_dlen1 + aligned_dlen2 + ALIGN(ilen, 8) +
+	      ALIGN(wlen, 8) + ALIGN(plen, 8);
 	if (move)
 		len += plen;
 
@@ -1313,6 +1325,15 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 		p += ALIGN(ilen, 8);
 	}
 
+	if (whiteout) {
+		pack_inode(c, p, whiteout, 0);
+		err = ubifs_node_calc_hash(c, p, hash_whiteout_inode);
+		if (err)
+			goto out_release;
+
+		p += ALIGN(wlen, 8);
+	}
+
 	if (!move) {
 		pack_inode(c, p, old_dir, 1);
 		err = ubifs_node_calc_hash(c, p, hash_old_dir);
@@ -1352,6 +1373,9 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 		if (new_inode)
 			ubifs_wbuf_add_ino_nolock(&c->jheads[BASEHD].wbuf,
 						  new_inode->i_ino);
+		if (whiteout)
+			ubifs_wbuf_add_ino_nolock(&c->jheads[BASEHD].wbuf,
+						  whiteout->i_ino);
 	}
 	release_head(c, BASEHD);
 
@@ -1368,8 +1392,6 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 		err = ubifs_tnc_add_nm(c, &key, lnum, offs, dlen2, hash_dent2, old_nm);
 		if (err)
 			goto out_ro;
-
-		ubifs_delete_orphan(c, whiteout->i_ino);
 	} else {
 		err = ubifs_add_dirt(c, lnum, dlen2);
 		if (err)
@@ -1390,6 +1412,15 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 		offs += ALIGN(ilen, 8);
 	}
 
+	if (whiteout) {
+		ino_key_init(c, &key, whiteout->i_ino);
+		err = ubifs_tnc_add(c, &key, lnum, offs, wlen,
+				    hash_whiteout_inode);
+		if (err)
+			goto out_ro;
+		offs += ALIGN(wlen, 8);
+	}
+
 	ino_key_init(c, &key, old_dir->i_ino);
 	err = ubifs_tnc_add(c, &key, lnum, offs, plen, hash_old_dir);
 	if (err)
@@ -1410,6 +1441,12 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 		new_ui->synced_i_size = new_ui->ui_size;
 		spin_unlock(&new_ui->ui_lock);
 	}
+	if (whiteout) {
+		/* No need to mark whiteout inode clean */
+		spin_lock(&whiteout_ui->ui_lock);
+		whiteout_ui->synced_i_size = whiteout_ui->ui_size;
+		spin_unlock(&whiteout_ui->ui_lock);
+	}
 	mark_inode_clean(c, ubifs_inode(old_dir));
 	if (move)
 		mark_inode_clean(c, ubifs_inode(new_dir));
-- 
2.31.1


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

* [PATCH 05/11] ubifs: Rename whiteout atomically
@ 2021-10-25  3:41   ` Zhihao Cheng
  0 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

Currently, rename whiteout has 3 steps:
  1. create tmpfile(which associates old dentry to tmpfile inode) for
     whiteout, and store tmpfile to disk
  2. link whiteout, associate whiteout inode to old dentry agagin and
     store old dentry, old inode, new dentry on disk
  3. writeback dirty whiteout inode to disk

Suddenly power-cut or error occurring(eg. ENOSPC returned by budget,
memory allocation failure) during above steps may cause kinds of problems:
  Problem 1: ENOSPC returned by whiteout space budget (before step 2),
	     old dentry will disappear after rename syscall, whiteout file
	     cannot be found either.

	     ls dir  // we get file, whiteout
	     rename(dir/file, dir/whiteout, REANME_WHITEOUT)
	     ENOSPC = ubifs_budget_space(&wht_req) // return
	     ls dir  // empty (no file, no whiteout)
  Problem 2: Power-cut happens before step 3, whiteout inode with 'nlink=1'
	     is not stored on disk, whiteout dentry(old dentry) is written
	     on disk, whiteout file is lost on next mount (We get "dead
	     directory entry" after executing 'ls -l' on whiteout file).

Now, we use following 3 steps to finish rename whiteout:
  1. create an in-mem inode with 'nlink = 1' as whiteout
  2. ubifs_jnl_rename (Write on disk to finish associating old dentry to
     whiteout inode, associating new dentry with old inode)
  3. iput(whiteout)

Rely writing in-mem inode on disk by ubifs_jnl_rename() to finish rename
whiteout, which avoids middle disk state caused by suddenly power-cut
and error occurring.

Fixes: 9e0a1fff8db56ea ("ubifs: Implement RENAME_WHITEOUT")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/dir.c     | 144 +++++++++++++++++++++++++++++----------------
 fs/ubifs/journal.c |  53 ++++++++++++++---
 2 files changed, 137 insertions(+), 60 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 6503e6857f6e..6344e2bc9338 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -349,8 +349,58 @@ static int ubifs_create(struct user_namespace *mnt_userns, struct inode *dir,
 	return err;
 }
 
-static int do_tmpfile(struct inode *dir, struct dentry *dentry,
-		      umode_t mode, struct inode **whiteout)
+static struct inode *create_whiteout(struct inode *dir, struct dentry *dentry,
+				     umode_t mode)
+{
+	int err;
+	struct inode *inode;
+	struct ubifs_inode *ui;
+	struct ubifs_info *c = dir->i_sb->s_fs_info;
+	struct fscrypt_name nm;
+
+	/*
+	 * Create an inode('nlink = 1') for whiteout without updating journal,
+	 * let ubifs_jnl_rename() store it on flash to complete rename whiteout
+	 * atomically.
+	 */
+
+	dbg_gen("dent '%pd', mode %#hx in dir ino %lu",
+		dentry, mode, dir->i_ino);
+
+	err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm);
+	if (err)
+		return ERR_PTR(err);
+
+	inode = ubifs_new_inode(c, dir, mode);
+	if (IS_ERR(inode)) {
+		err = PTR_ERR(inode);
+		goto out_free;
+	}
+	ui = ubifs_inode(inode);
+
+	init_special_inode(inode, inode->i_mode, WHITEOUT_DEV);
+	ubifs_assert(c, inode->i_op == &ubifs_file_inode_operations);
+
+	err = ubifs_init_security(dir, inode, &dentry->d_name);
+	if (err)
+		goto out_inode;
+
+	/* The dir size is updated by do_rename. */
+	insert_inode_hash(inode);
+
+	return inode;
+
+out_inode:
+	make_bad_inode(inode);
+	iput(inode);
+out_free:
+	fscrypt_free_filename(&nm);
+	ubifs_err(c, "cannot create whiteout file, error %d", err);
+	return ERR_PTR(err);
+}
+
+static int ubifs_tmpfile(struct user_namespace *mnt_userns, struct inode *dir,
+			 struct dentry *dentry, umode_t mode)
 {
 	struct inode *inode;
 	struct ubifs_info *c = dir->i_sb->s_fs_info;
@@ -392,25 +442,13 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry,
 	}
 	ui = ubifs_inode(inode);
 
-	if (whiteout) {
-		init_special_inode(inode, inode->i_mode, WHITEOUT_DEV);
-		ubifs_assert(c, inode->i_op == &ubifs_file_inode_operations);
-	}
-
 	err = ubifs_init_security(dir, inode, &dentry->d_name);
 	if (err)
 		goto out_inode;
 
 	mutex_lock(&ui->ui_mutex);
 	insert_inode_hash(inode);
-
-	if (whiteout) {
-		mark_inode_dirty(inode);
-		drop_nlink(inode);
-		*whiteout = inode;
-	} else {
-		d_tmpfile(dentry, inode);
-	}
+	d_tmpfile(dentry, inode);
 	ubifs_assert(c, ui->dirty);
 
 	instantiated = 1;
@@ -441,12 +479,6 @@ static int do_tmpfile(struct inode *dir, struct dentry *dentry,
 	return err;
 }
 
-static int ubifs_tmpfile(struct user_namespace *mnt_userns, struct inode *dir,
-			 struct dentry *dentry, umode_t mode)
-{
-	return do_tmpfile(dir, dentry, mode, NULL);
-}
-
 /**
  * vfs_dent_type - get VFS directory entry type.
  * @type: UBIFS directory entry type
@@ -1264,17 +1296,19 @@ 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 timespec64 time;
 	unsigned int saved_nlink;
 	struct fscrypt_name old_nm, new_nm;
 
 	/*
-	 * Budget request settings: deletion direntry, new direntry, removing
-	 * the old inode, and changing old and new parent directory inodes.
+	 * Budget request settings:
+	 *   req: deletion direntry, new direntry, removing the old inode,
+	 *   and changing old and new parent directory inodes.
+	 *
+	 *   wht_req: new whiteout inode for RENAME_WHITEOUT.
 	 *
-	 * However, this operation also marks the target inode as dirty and
-	 * does not write it, so we allocate budget for the target inode
-	 * separately.
+	 *   ino_req: marks the target inode as dirty and does not write it.
 	 */
 
 	dbg_gen("dent '%pd' ino %lu in dir ino %lu to dent '%pd' in dir ino %lu flags 0x%x",
@@ -1324,7 +1358,6 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 
 	if (flags & RENAME_WHITEOUT) {
 		union ubifs_dev_desc *dev = NULL;
-		struct ubifs_budget_req wht_req;
 
 		dev = kmalloc(sizeof(union ubifs_dev_desc), GFP_NOFS);
 		if (!dev) {
@@ -1332,26 +1365,27 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 			goto out_release;
 		}
 
-		err = do_tmpfile(old_dir, old_dentry, S_IFCHR | WHITEOUT_MODE, &whiteout);
-		if (err) {
-			if (whiteout)
-				iput(whiteout);
+		/*
+		 * The whiteout inode without dentry is pinned in memory,
+		 * umount won't happen during rename process because we
+		 * got parent dentry.
+		 */
+		whiteout = create_whiteout(old_dir, old_dentry,
+					   S_IFCHR | WHITEOUT_MODE);
+		if (IS_ERR(whiteout)) {
+			err = PTR_ERR(whiteout);
 			kfree(dev);
 			goto out_release;
 		}
 
-		spin_lock(&whiteout->i_lock);
-		whiteout->i_state |= I_LINKABLE;
-		spin_unlock(&whiteout->i_lock);
-
 		whiteout_ui = ubifs_inode(whiteout);
 		whiteout_ui->data = dev;
 		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.dirtied_ino = 1;
-		wht_req.dirtied_ino_d = ALIGN(whiteout_ui->data_len, 8);
+		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
 		 * waits wb work) and writeback work(waits ui_mutex), do space
@@ -1359,6 +1393,11 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 		 */
 		err = ubifs_budget_space(c, &wht_req);
 		if (err) {
+			/*
+			 * Whiteout inode can not be written on flash by
+			 * ubifs_jnl_write_inode(), because it's neither
+			 * dirty nor zero-nlink.
+			 */
 			iput(whiteout);
 			goto out_release;
 		}
@@ -1433,17 +1472,8 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 		sync = IS_DIRSYNC(old_dir) || IS_DIRSYNC(new_dir);
 		if (unlink && IS_SYNC(new_inode))
 			sync = 1;
-	}
-
-	if (whiteout) {
-		inc_nlink(whiteout);
-		mark_inode_dirty(whiteout);
-
-		spin_lock(&whiteout->i_lock);
-		whiteout->i_state &= ~I_LINKABLE;
-		spin_unlock(&whiteout->i_lock);
-
-		iput(whiteout);
+		if (whiteout && IS_SYNC(whiteout))
+			sync = 1;
 	}
 
 	err = ubifs_jnl_rename(c, old_dir, old_inode, &old_nm, new_dir,
@@ -1454,6 +1484,11 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 	unlock_4_inodes(old_dir, new_dir, new_inode, whiteout);
 	ubifs_release_budget(c, &req);
 
+	if (whiteout) {
+		ubifs_release_budget(c, &wht_req);
+		iput(whiteout);
+	}
+
 	mutex_lock(&old_inode_ui->ui_mutex);
 	release = old_inode_ui->dirty;
 	mark_inode_dirty_sync(old_inode);
@@ -1462,11 +1497,16 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 	if (release)
 		ubifs_release_budget(c, &ino_req);
 	if (IS_SYNC(old_inode))
-		err = old_inode->i_sb->s_op->write_inode(old_inode, NULL);
+		/*
+		 * Rename finished here. Although old inode cannot be updated
+		 * on flash, old ctime is not a big problem, don't return err
+		 * code to userspace.
+		 */
+		old_inode->i_sb->s_op->write_inode(old_inode, NULL);
 
 	fscrypt_free_filename(&old_nm);
 	fscrypt_free_filename(&new_nm);
-	return err;
+	return 0;
 
 out_cancel:
 	if (unlink) {
@@ -1487,11 +1527,11 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry,
 				inc_nlink(old_dir);
 		}
 	}
+	unlock_4_inodes(old_dir, new_dir, new_inode, whiteout);
 	if (whiteout) {
-		drop_nlink(whiteout);
+		ubifs_release_budget(c, &wht_req);
 		iput(whiteout);
 	}
-	unlock_4_inodes(old_dir, new_dir, new_inode, whiteout);
 out_release:
 	ubifs_release_budget(c, &ino_req);
 	ubifs_release_budget(c, &req);
diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index 8ea680dba61e..a77821f922e2 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -1207,9 +1207,9 @@ int ubifs_jnl_xrename(struct ubifs_info *c, const struct inode *fst_dir,
  * @sync: non-zero if the write-buffer has to be synchronized
  *
  * This function implements the re-name operation which may involve writing up
- * to 4 inodes and 2 directory entries. It marks the written inodes as clean
- * and returns zero on success. In case of failure, a negative error code is
- * returned.
+ * to 4 inodes(new inode, whiteout inode, old and new parent directory inodes)
+ * and 2 directory entries. It marks the written inodes as clean and returns
+ * zero on success. In case of failure, a negative error code is returned.
  */
 int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 		     const struct inode *old_inode,
@@ -1222,14 +1222,15 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 	void *p;
 	union ubifs_key key;
 	struct ubifs_dent_node *dent, *dent2;
-	int err, dlen1, dlen2, ilen, lnum, offs, len, orphan_added = 0;
+	int err, dlen1, dlen2, ilen, wlen, lnum, offs, len, orphan_added = 0;
 	int aligned_dlen1, aligned_dlen2, plen = UBIFS_INO_NODE_SZ;
 	int last_reference = !!(new_inode && new_inode->i_nlink == 0);
 	int move = (old_dir != new_dir);
-	struct ubifs_inode *new_ui;
+	struct ubifs_inode *new_ui, *whiteout_ui;
 	u8 hash_old_dir[UBIFS_HASH_ARR_SZ];
 	u8 hash_new_dir[UBIFS_HASH_ARR_SZ];
 	u8 hash_new_inode[UBIFS_HASH_ARR_SZ];
+	u8 hash_whiteout_inode[UBIFS_HASH_ARR_SZ];
 	u8 hash_dent1[UBIFS_HASH_ARR_SZ];
 	u8 hash_dent2[UBIFS_HASH_ARR_SZ];
 
@@ -1249,9 +1250,20 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 	} else
 		ilen = 0;
 
+	if (whiteout) {
+		whiteout_ui = ubifs_inode(whiteout);
+		ubifs_assert(c, mutex_is_locked(&whiteout_ui->ui_mutex));
+		ubifs_assert(c, whiteout->i_nlink == 1);
+		ubifs_assert(c, !whiteout_ui->dirty);
+		wlen = UBIFS_INO_NODE_SZ;
+		wlen += whiteout_ui->data_len;
+	} else
+		wlen = 0;
+
 	aligned_dlen1 = ALIGN(dlen1, 8);
 	aligned_dlen2 = ALIGN(dlen2, 8);
-	len = aligned_dlen1 + aligned_dlen2 + ALIGN(ilen, 8) + ALIGN(plen, 8);
+	len = aligned_dlen1 + aligned_dlen2 + ALIGN(ilen, 8) +
+	      ALIGN(wlen, 8) + ALIGN(plen, 8);
 	if (move)
 		len += plen;
 
@@ -1313,6 +1325,15 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 		p += ALIGN(ilen, 8);
 	}
 
+	if (whiteout) {
+		pack_inode(c, p, whiteout, 0);
+		err = ubifs_node_calc_hash(c, p, hash_whiteout_inode);
+		if (err)
+			goto out_release;
+
+		p += ALIGN(wlen, 8);
+	}
+
 	if (!move) {
 		pack_inode(c, p, old_dir, 1);
 		err = ubifs_node_calc_hash(c, p, hash_old_dir);
@@ -1352,6 +1373,9 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 		if (new_inode)
 			ubifs_wbuf_add_ino_nolock(&c->jheads[BASEHD].wbuf,
 						  new_inode->i_ino);
+		if (whiteout)
+			ubifs_wbuf_add_ino_nolock(&c->jheads[BASEHD].wbuf,
+						  whiteout->i_ino);
 	}
 	release_head(c, BASEHD);
 
@@ -1368,8 +1392,6 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 		err = ubifs_tnc_add_nm(c, &key, lnum, offs, dlen2, hash_dent2, old_nm);
 		if (err)
 			goto out_ro;
-
-		ubifs_delete_orphan(c, whiteout->i_ino);
 	} else {
 		err = ubifs_add_dirt(c, lnum, dlen2);
 		if (err)
@@ -1390,6 +1412,15 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 		offs += ALIGN(ilen, 8);
 	}
 
+	if (whiteout) {
+		ino_key_init(c, &key, whiteout->i_ino);
+		err = ubifs_tnc_add(c, &key, lnum, offs, wlen,
+				    hash_whiteout_inode);
+		if (err)
+			goto out_ro;
+		offs += ALIGN(wlen, 8);
+	}
+
 	ino_key_init(c, &key, old_dir->i_ino);
 	err = ubifs_tnc_add(c, &key, lnum, offs, plen, hash_old_dir);
 	if (err)
@@ -1410,6 +1441,12 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
 		new_ui->synced_i_size = new_ui->ui_size;
 		spin_unlock(&new_ui->ui_lock);
 	}
+	if (whiteout) {
+		/* No need to mark whiteout inode clean */
+		spin_lock(&whiteout_ui->ui_lock);
+		whiteout_ui->synced_i_size = whiteout_ui->ui_size;
+		spin_unlock(&whiteout_ui->ui_lock);
+	}
 	mark_inode_clean(c, ubifs_inode(old_dir));
 	if (move)
 		mark_inode_clean(c, ubifs_inode(new_dir));
-- 
2.31.1


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

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

* [PATCH 06/11] ubifs: Fix 'ui->dirty' race between do_tmpfile() and writeback work
  2021-10-25  3:41 ` Zhihao Cheng
@ 2021-10-25  3:41   ` Zhihao Cheng
  -1 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

'ui->dirty' is not protected by 'ui_mutex' in function do_tmpfile() which
may race with ubifs_write_inode[wb_workfn] to access/update 'ui->dirty',
finally dirty space is released twice.

	open(O_TMPFILE)                wb_workfn
do_tmpfile
  ubifs_budget_space(ino_req = { .dirtied_ino = 1})
  d_tmpfile // mark inode(tmpfile) dirty
  ubifs_jnl_update // without holding tmpfile's ui_mutex
    mark_inode_clean(ui)
      if (ui->dirty)
        ubifs_release_dirty_inode_budget(ui)  // release first time
                                   ubifs_write_inode
				     mutex_lock(&ui->ui_mutex)
                                     ubifs_release_dirty_inode_budget(ui)
				     // release second time
				     mutex_unlock(&ui->ui_mutex)
      ui->dirty = 0

Run generic/476 can reproduce following message easily
(See reproducer in [Link]):

  UBIFS error (ubi0:0 pid 2578): ubifs_assert_failed [ubifs]: UBIFS assert
  failed: c->bi.dd_growth >= 0, in fs/ubifs/budget.c:554
  UBIFS warning (ubi0:0 pid 2578): ubifs_ro_mode [ubifs]: switched to
  read-only mode, error -22
  Workqueue: writeback wb_workfn (flush-ubifs_0_0)
  Call Trace:
    ubifs_ro_mode+0x54/0x60 [ubifs]
    ubifs_assert_failed+0x4b/0x80 [ubifs]
    ubifs_release_budget+0x468/0x5a0 [ubifs]
    ubifs_release_dirty_inode_budget+0x53/0x80 [ubifs]
    ubifs_write_inode+0x121/0x1f0 [ubifs]
    ...
    wb_workfn+0x283/0x7b0

Fix it by holding tmpfile ubifs inode lock during ubifs_jnl_update().
Similar problem exists in whiteout renaming, but previous fix("ubifs:
Rename whiteout atomically") has solved the problem.

Fixes: 474b93704f32163 ("ubifs: Implement O_TMPFILE")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=214765
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/dir.c | 60 +++++++++++++++++++++++++-------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 6344e2bc9338..8a2c42e6b22b 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -399,6 +399,32 @@ static struct inode *create_whiteout(struct inode *dir, struct dentry *dentry,
 	return ERR_PTR(err);
 }
 
+/**
+ * lock_2_inodes - a wrapper for locking two UBIFS inodes.
+ * @inode1: first inode
+ * @inode2: second inode
+ *
+ * We do not implement any tricks to guarantee strict lock ordering, because
+ * VFS has already done it for us on the @i_mutex. So this is just a simple
+ * wrapper function.
+ */
+static void lock_2_inodes(struct inode *inode1, struct inode *inode2)
+{
+	mutex_lock_nested(&ubifs_inode(inode1)->ui_mutex, WB_MUTEX_1);
+	mutex_lock_nested(&ubifs_inode(inode2)->ui_mutex, WB_MUTEX_2);
+}
+
+/**
+ * unlock_2_inodes - a wrapper for unlocking two UBIFS inodes.
+ * @inode1: first inode
+ * @inode2: second inode
+ */
+static void unlock_2_inodes(struct inode *inode1, struct inode *inode2)
+{
+	mutex_unlock(&ubifs_inode(inode2)->ui_mutex);
+	mutex_unlock(&ubifs_inode(inode1)->ui_mutex);
+}
+
 static int ubifs_tmpfile(struct user_namespace *mnt_userns, struct inode *dir,
 			 struct dentry *dentry, umode_t mode)
 {
@@ -406,7 +432,7 @@ static int ubifs_tmpfile(struct user_namespace *mnt_userns, struct inode *dir,
 	struct ubifs_info *c = dir->i_sb->s_fs_info;
 	struct ubifs_budget_req req = { .new_ino = 1, .new_dent = 1};
 	struct ubifs_budget_req ino_req = { .dirtied_ino = 1 };
-	struct ubifs_inode *ui, *dir_ui = ubifs_inode(dir);
+	struct ubifs_inode *ui;
 	int err, instantiated = 0;
 	struct fscrypt_name nm;
 
@@ -454,18 +480,18 @@ static int ubifs_tmpfile(struct user_namespace *mnt_userns, struct inode *dir,
 	instantiated = 1;
 	mutex_unlock(&ui->ui_mutex);
 
-	mutex_lock(&dir_ui->ui_mutex);
+	lock_2_inodes(dir, inode);
 	err = ubifs_jnl_update(c, dir, &nm, inode, 1, 0);
 	if (err)
 		goto out_cancel;
-	mutex_unlock(&dir_ui->ui_mutex);
+	unlock_2_inodes(dir, inode);
 
 	ubifs_release_budget(c, &req);
 
 	return 0;
 
 out_cancel:
-	mutex_unlock(&dir_ui->ui_mutex);
+	unlock_2_inodes(dir, inode);
 out_inode:
 	make_bad_inode(inode);
 	if (!instantiated)
@@ -692,32 +718,6 @@ static int ubifs_dir_release(struct inode *dir, struct file *file)
 	return 0;
 }
 
-/**
- * lock_2_inodes - a wrapper for locking two UBIFS inodes.
- * @inode1: first inode
- * @inode2: second inode
- *
- * We do not implement any tricks to guarantee strict lock ordering, because
- * VFS has already done it for us on the @i_mutex. So this is just a simple
- * wrapper function.
- */
-static void lock_2_inodes(struct inode *inode1, struct inode *inode2)
-{
-	mutex_lock_nested(&ubifs_inode(inode1)->ui_mutex, WB_MUTEX_1);
-	mutex_lock_nested(&ubifs_inode(inode2)->ui_mutex, WB_MUTEX_2);
-}
-
-/**
- * unlock_2_inodes - a wrapper for unlocking two UBIFS inodes.
- * @inode1: first inode
- * @inode2: second inode
- */
-static void unlock_2_inodes(struct inode *inode1, struct inode *inode2)
-{
-	mutex_unlock(&ubifs_inode(inode2)->ui_mutex);
-	mutex_unlock(&ubifs_inode(inode1)->ui_mutex);
-}
-
 static int ubifs_link(struct dentry *old_dentry, struct inode *dir,
 		      struct dentry *dentry)
 {
-- 
2.31.1


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

* [PATCH 06/11] ubifs: Fix 'ui->dirty' race between do_tmpfile() and writeback work
@ 2021-10-25  3:41   ` Zhihao Cheng
  0 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

'ui->dirty' is not protected by 'ui_mutex' in function do_tmpfile() which
may race with ubifs_write_inode[wb_workfn] to access/update 'ui->dirty',
finally dirty space is released twice.

	open(O_TMPFILE)                wb_workfn
do_tmpfile
  ubifs_budget_space(ino_req = { .dirtied_ino = 1})
  d_tmpfile // mark inode(tmpfile) dirty
  ubifs_jnl_update // without holding tmpfile's ui_mutex
    mark_inode_clean(ui)
      if (ui->dirty)
        ubifs_release_dirty_inode_budget(ui)  // release first time
                                   ubifs_write_inode
				     mutex_lock(&ui->ui_mutex)
                                     ubifs_release_dirty_inode_budget(ui)
				     // release second time
				     mutex_unlock(&ui->ui_mutex)
      ui->dirty = 0

Run generic/476 can reproduce following message easily
(See reproducer in [Link]):

  UBIFS error (ubi0:0 pid 2578): ubifs_assert_failed [ubifs]: UBIFS assert
  failed: c->bi.dd_growth >= 0, in fs/ubifs/budget.c:554
  UBIFS warning (ubi0:0 pid 2578): ubifs_ro_mode [ubifs]: switched to
  read-only mode, error -22
  Workqueue: writeback wb_workfn (flush-ubifs_0_0)
  Call Trace:
    ubifs_ro_mode+0x54/0x60 [ubifs]
    ubifs_assert_failed+0x4b/0x80 [ubifs]
    ubifs_release_budget+0x468/0x5a0 [ubifs]
    ubifs_release_dirty_inode_budget+0x53/0x80 [ubifs]
    ubifs_write_inode+0x121/0x1f0 [ubifs]
    ...
    wb_workfn+0x283/0x7b0

Fix it by holding tmpfile ubifs inode lock during ubifs_jnl_update().
Similar problem exists in whiteout renaming, but previous fix("ubifs:
Rename whiteout atomically") has solved the problem.

Fixes: 474b93704f32163 ("ubifs: Implement O_TMPFILE")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=214765
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/dir.c | 60 +++++++++++++++++++++++++-------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 6344e2bc9338..8a2c42e6b22b 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -399,6 +399,32 @@ static struct inode *create_whiteout(struct inode *dir, struct dentry *dentry,
 	return ERR_PTR(err);
 }
 
+/**
+ * lock_2_inodes - a wrapper for locking two UBIFS inodes.
+ * @inode1: first inode
+ * @inode2: second inode
+ *
+ * We do not implement any tricks to guarantee strict lock ordering, because
+ * VFS has already done it for us on the @i_mutex. So this is just a simple
+ * wrapper function.
+ */
+static void lock_2_inodes(struct inode *inode1, struct inode *inode2)
+{
+	mutex_lock_nested(&ubifs_inode(inode1)->ui_mutex, WB_MUTEX_1);
+	mutex_lock_nested(&ubifs_inode(inode2)->ui_mutex, WB_MUTEX_2);
+}
+
+/**
+ * unlock_2_inodes - a wrapper for unlocking two UBIFS inodes.
+ * @inode1: first inode
+ * @inode2: second inode
+ */
+static void unlock_2_inodes(struct inode *inode1, struct inode *inode2)
+{
+	mutex_unlock(&ubifs_inode(inode2)->ui_mutex);
+	mutex_unlock(&ubifs_inode(inode1)->ui_mutex);
+}
+
 static int ubifs_tmpfile(struct user_namespace *mnt_userns, struct inode *dir,
 			 struct dentry *dentry, umode_t mode)
 {
@@ -406,7 +432,7 @@ static int ubifs_tmpfile(struct user_namespace *mnt_userns, struct inode *dir,
 	struct ubifs_info *c = dir->i_sb->s_fs_info;
 	struct ubifs_budget_req req = { .new_ino = 1, .new_dent = 1};
 	struct ubifs_budget_req ino_req = { .dirtied_ino = 1 };
-	struct ubifs_inode *ui, *dir_ui = ubifs_inode(dir);
+	struct ubifs_inode *ui;
 	int err, instantiated = 0;
 	struct fscrypt_name nm;
 
@@ -454,18 +480,18 @@ static int ubifs_tmpfile(struct user_namespace *mnt_userns, struct inode *dir,
 	instantiated = 1;
 	mutex_unlock(&ui->ui_mutex);
 
-	mutex_lock(&dir_ui->ui_mutex);
+	lock_2_inodes(dir, inode);
 	err = ubifs_jnl_update(c, dir, &nm, inode, 1, 0);
 	if (err)
 		goto out_cancel;
-	mutex_unlock(&dir_ui->ui_mutex);
+	unlock_2_inodes(dir, inode);
 
 	ubifs_release_budget(c, &req);
 
 	return 0;
 
 out_cancel:
-	mutex_unlock(&dir_ui->ui_mutex);
+	unlock_2_inodes(dir, inode);
 out_inode:
 	make_bad_inode(inode);
 	if (!instantiated)
@@ -692,32 +718,6 @@ static int ubifs_dir_release(struct inode *dir, struct file *file)
 	return 0;
 }
 
-/**
- * lock_2_inodes - a wrapper for locking two UBIFS inodes.
- * @inode1: first inode
- * @inode2: second inode
- *
- * We do not implement any tricks to guarantee strict lock ordering, because
- * VFS has already done it for us on the @i_mutex. So this is just a simple
- * wrapper function.
- */
-static void lock_2_inodes(struct inode *inode1, struct inode *inode2)
-{
-	mutex_lock_nested(&ubifs_inode(inode1)->ui_mutex, WB_MUTEX_1);
-	mutex_lock_nested(&ubifs_inode(inode2)->ui_mutex, WB_MUTEX_2);
-}
-
-/**
- * unlock_2_inodes - a wrapper for unlocking two UBIFS inodes.
- * @inode1: first inode
- * @inode2: second inode
- */
-static void unlock_2_inodes(struct inode *inode1, struct inode *inode2)
-{
-	mutex_unlock(&ubifs_inode(inode2)->ui_mutex);
-	mutex_unlock(&ubifs_inode(inode1)->ui_mutex);
-}
-
 static int ubifs_link(struct dentry *old_dentry, struct inode *dir,
 		      struct dentry *dentry)
 {
-- 
2.31.1


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

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

* [PATCH 07/11] ubifs: Rectify space amount budget for mkdir/tmpfile operations
  2021-10-25  3:41 ` Zhihao Cheng
@ 2021-10-25  3:41   ` Zhihao Cheng
  -1 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

UBIFS should make sure the flash has enough space to store dirty (Data
that is newer than disk) data (in memory), space budget is exactly
designed to do that. If space budget calculates less data than we need,
'make_reservation()' will do more work(return -ENOSPC if no free space
lelf, sometimes we can see "cannot reserve xxx bytes in jhead xxx, error
-28" in ubifs error messages) with ubifs inodes locked, which may effect
other syscalls.

A simple way to decide how much space do we need when make a budget:
See how much space is needed by 'make_reservation()' in ubifs_jnl_xxx()
function according to corresponding operation.

It's better to report ENOSPC in ubifs_budget_space(), as early as we can.

Fixes: 474b93704f32163 ("ubifs: Implement O_TMPFILE")
Fixes: 1e51764a3c2ac05 ("UBIFS: add new flash file system")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/dir.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 8a2c42e6b22b..3a0c70362ad6 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -430,15 +430,18 @@ static int ubifs_tmpfile(struct user_namespace *mnt_userns, struct inode *dir,
 {
 	struct inode *inode;
 	struct ubifs_info *c = dir->i_sb->s_fs_info;
-	struct ubifs_budget_req req = { .new_ino = 1, .new_dent = 1};
+	struct ubifs_budget_req req = { .new_ino = 1, .new_dent = 1,
+					.dirtied_ino = 1};
 	struct ubifs_budget_req ino_req = { .dirtied_ino = 1 };
 	struct ubifs_inode *ui;
 	int err, instantiated = 0;
 	struct fscrypt_name nm;
 
 	/*
-	 * Budget request settings: new dirty inode, new direntry,
-	 * budget for dirtied inode will be released via writeback.
+	 * Budget request settings: new inode, new direntry, changing the
+	 * parent directory inode.
+	 * Allocate budget separately for new dirtied inode, the budget will
+	 * be released via writeback.
 	 */
 
 	dbg_gen("dent '%pd', mode %#hx in dir ino %lu",
@@ -981,7 +984,8 @@ static int ubifs_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
 	struct ubifs_inode *dir_ui = ubifs_inode(dir);
 	struct ubifs_info *c = dir->i_sb->s_fs_info;
 	int err, sz_change;
-	struct ubifs_budget_req req = { .new_ino = 1, .new_dent = 1 };
+	struct ubifs_budget_req req = { .new_ino = 1, .new_dent = 1,
+					.dirtied_ino = 1};
 	struct fscrypt_name nm;
 
 	/*
-- 
2.31.1


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

* [PATCH 07/11] ubifs: Rectify space amount budget for mkdir/tmpfile operations
@ 2021-10-25  3:41   ` Zhihao Cheng
  0 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

UBIFS should make sure the flash has enough space to store dirty (Data
that is newer than disk) data (in memory), space budget is exactly
designed to do that. If space budget calculates less data than we need,
'make_reservation()' will do more work(return -ENOSPC if no free space
lelf, sometimes we can see "cannot reserve xxx bytes in jhead xxx, error
-28" in ubifs error messages) with ubifs inodes locked, which may effect
other syscalls.

A simple way to decide how much space do we need when make a budget:
See how much space is needed by 'make_reservation()' in ubifs_jnl_xxx()
function according to corresponding operation.

It's better to report ENOSPC in ubifs_budget_space(), as early as we can.

Fixes: 474b93704f32163 ("ubifs: Implement O_TMPFILE")
Fixes: 1e51764a3c2ac05 ("UBIFS: add new flash file system")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/dir.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 8a2c42e6b22b..3a0c70362ad6 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -430,15 +430,18 @@ static int ubifs_tmpfile(struct user_namespace *mnt_userns, struct inode *dir,
 {
 	struct inode *inode;
 	struct ubifs_info *c = dir->i_sb->s_fs_info;
-	struct ubifs_budget_req req = { .new_ino = 1, .new_dent = 1};
+	struct ubifs_budget_req req = { .new_ino = 1, .new_dent = 1,
+					.dirtied_ino = 1};
 	struct ubifs_budget_req ino_req = { .dirtied_ino = 1 };
 	struct ubifs_inode *ui;
 	int err, instantiated = 0;
 	struct fscrypt_name nm;
 
 	/*
-	 * Budget request settings: new dirty inode, new direntry,
-	 * budget for dirtied inode will be released via writeback.
+	 * Budget request settings: new inode, new direntry, changing the
+	 * parent directory inode.
+	 * Allocate budget separately for new dirtied inode, the budget will
+	 * be released via writeback.
 	 */
 
 	dbg_gen("dent '%pd', mode %#hx in dir ino %lu",
@@ -981,7 +984,8 @@ static int ubifs_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
 	struct ubifs_inode *dir_ui = ubifs_inode(dir);
 	struct ubifs_info *c = dir->i_sb->s_fs_info;
 	int err, sz_change;
-	struct ubifs_budget_req req = { .new_ino = 1, .new_dent = 1 };
+	struct ubifs_budget_req req = { .new_ino = 1, .new_dent = 1,
+					.dirtied_ino = 1};
 	struct fscrypt_name nm;
 
 	/*
-- 
2.31.1


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

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

* [PATCH 08/11] ubifs: setflags: Don't make a budget for 'ui->data_len'
  2021-10-25  3:41 ` Zhihao Cheng
@ 2021-10-25  3:41   ` Zhihao Cheng
  -1 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

'setflags()' affects regular files and directories, only xattr inode,
symlink inode and special inode(pipe/char_dev/block_dev) have none-
zero 'ui->data_len' field.

Remove superfluous space budget for 'dirtied_ino_d', besides add an
assert to verify that 'setflags()' only operates ubifs inode with
zero data_len.

Fixes: 1e51764a3c2ac05a ("UBIFS: add new flash file system")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ubifs/ioctl.c b/fs/ubifs/ioctl.c
index c6a863487780..ed95e97a3740 100644
--- a/fs/ubifs/ioctl.c
+++ b/fs/ubifs/ioctl.c
@@ -107,9 +107,9 @@ static int setflags(struct inode *inode, int flags)
 	int err, release;
 	struct ubifs_inode *ui = ubifs_inode(inode);
 	struct ubifs_info *c = inode->i_sb->s_fs_info;
-	struct ubifs_budget_req req = { .dirtied_ino = 1,
-					.dirtied_ino_d = ui->data_len };
+	struct ubifs_budget_req req = { .dirtied_ino = 1 };
 
+	ubifs_assert(c, !ui->data_len);
 	err = ubifs_budget_space(c, &req);
 	if (err)
 		return err;
-- 
2.31.1


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

* [PATCH 08/11] ubifs: setflags: Don't make a budget for 'ui->data_len'
@ 2021-10-25  3:41   ` Zhihao Cheng
  0 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

'setflags()' affects regular files and directories, only xattr inode,
symlink inode and special inode(pipe/char_dev/block_dev) have none-
zero 'ui->data_len' field.

Remove superfluous space budget for 'dirtied_ino_d', besides add an
assert to verify that 'setflags()' only operates ubifs inode with
zero data_len.

Fixes: 1e51764a3c2ac05a ("UBIFS: add new flash file system")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ubifs/ioctl.c b/fs/ubifs/ioctl.c
index c6a863487780..ed95e97a3740 100644
--- a/fs/ubifs/ioctl.c
+++ b/fs/ubifs/ioctl.c
@@ -107,9 +107,9 @@ static int setflags(struct inode *inode, int flags)
 	int err, release;
 	struct ubifs_inode *ui = ubifs_inode(inode);
 	struct ubifs_info *c = inode->i_sb->s_fs_info;
-	struct ubifs_budget_req req = { .dirtied_ino = 1,
-					.dirtied_ino_d = ui->data_len };
+	struct ubifs_budget_req req = { .dirtied_ino = 1 };
 
+	ubifs_assert(c, !ui->data_len);
 	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/

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

* [PATCH 09/11] ubifs: Fix read out-of-bounds in ubifs_wbuf_write_nolock()
  2021-10-25  3:41 ` Zhihao Cheng
@ 2021-10-25  3:41   ` Zhihao Cheng
  -1 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

Function ubifs_wbuf_write_nolock() may access buf out of bounds in
following process:

ubifs_wbuf_write_nolock():
  aligned_len = ALIGN(len, 8);   // Assume len = 4089, aligned_len = 4096
  if (aligned_len <= wbuf->avail) ... // Not satisfy
  if (wbuf->used) {
    ubifs_leb_write()  // Fill some data in avail wbuf
    len -= wbuf->avail;   // len is still not 8-bytes aligned
    aligned_len -= wbuf->avail;
  }
  n = aligned_len >> c->max_write_shift;
  if (n) {
    n <<= c->max_write_shift;
    err = ubifs_leb_write(c, wbuf->lnum, buf + written,
                          wbuf->offs, n);
    // n > len, read out of bounds less than 8(n-len) bytes
  }

, which can be catched by KASAN:
  =========================================================
  BUG: KASAN: slab-out-of-bounds in ecc_sw_hamming_calculate+0x1dc/0x7d0
  Read of size 4 at addr ffff888105594ff8 by task kworker/u8:4/128
  Workqueue: writeback wb_workfn (flush-ubifs_0_0)
  Call Trace:
    kasan_report.cold+0x81/0x165
    nand_write_page_swecc+0xa9/0x160
    ubifs_leb_write+0xf2/0x1b0 [ubifs]
    ubifs_wbuf_write_nolock+0x421/0x12c0 [ubifs]
    write_head+0xdc/0x1c0 [ubifs]
    ubifs_jnl_write_inode+0x627/0x960 [ubifs]
    wb_workfn+0x8af/0xb80

Function ubifs_wbuf_write_nolock() accepts that parameter 'len' is not 8
bytes aligned, the 'len' represents the true length of buf (which is
allocated in 'ubifs_jnl_xxx', eg. ubifs_jnl_write_inode), so
ubifs_wbuf_write_nolock() must handle the length read from 'buf' carefully
to write leb safely.

Fetch a reproducer in [Link].

Fixes: 1e51764a3c2ac0 ("UBIFS: add new flash file system")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=214785
Reported-by: Chengsong Ke <kechengsong@huawei.com>
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/io.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index 00b61dba62b7..b019dd6f7fa0 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -833,16 +833,42 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len)
 	 */
 	n = aligned_len >> c->max_write_shift;
 	if (n) {
-		n <<= c->max_write_shift;
+		int m = n - 1;
+
 		dbg_io("write %d bytes to LEB %d:%d", n, wbuf->lnum,
 		       wbuf->offs);
-		err = ubifs_leb_write(c, wbuf->lnum, buf + written,
-				      wbuf->offs, n);
+
+		if (m) {
+			/* '(n-1)<<c->max_write_shift < len' is always true. */
+			m <<= c->max_write_shift;
+			err = ubifs_leb_write(c, wbuf->lnum, buf + written,
+					      wbuf->offs, m);
+			if (err)
+				goto out;
+			wbuf->offs += m;
+			aligned_len -= m;
+			len -= m;
+			written += m;
+		}
+
+		/*
+		 * The non-written len of buf may be less than 'n' because
+		 * parameter 'len' is not 8 bytes aligned, so here we read
+		 * min(len, n) bytes from buf.
+		 */
+		n = 1 << c->max_write_shift;
+		memcpy(wbuf->buf, buf + written, min(len, n));
+		if (n > len) {
+			ubifs_assert(c, n - len < 8);
+			ubifs_pad(c, wbuf->buf + len, n - len);
+		}
+
+		err = ubifs_leb_write(c, wbuf->lnum, wbuf->buf, wbuf->offs, n);
 		if (err)
 			goto out;
 		wbuf->offs += n;
 		aligned_len -= n;
-		len -= n;
+		len -= min(len, n);
 		written += n;
 	}
 
-- 
2.31.1


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

* [PATCH 09/11] ubifs: Fix read out-of-bounds in ubifs_wbuf_write_nolock()
@ 2021-10-25  3:41   ` Zhihao Cheng
  0 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

Function ubifs_wbuf_write_nolock() may access buf out of bounds in
following process:

ubifs_wbuf_write_nolock():
  aligned_len = ALIGN(len, 8);   // Assume len = 4089, aligned_len = 4096
  if (aligned_len <= wbuf->avail) ... // Not satisfy
  if (wbuf->used) {
    ubifs_leb_write()  // Fill some data in avail wbuf
    len -= wbuf->avail;   // len is still not 8-bytes aligned
    aligned_len -= wbuf->avail;
  }
  n = aligned_len >> c->max_write_shift;
  if (n) {
    n <<= c->max_write_shift;
    err = ubifs_leb_write(c, wbuf->lnum, buf + written,
                          wbuf->offs, n);
    // n > len, read out of bounds less than 8(n-len) bytes
  }

, which can be catched by KASAN:
  =========================================================
  BUG: KASAN: slab-out-of-bounds in ecc_sw_hamming_calculate+0x1dc/0x7d0
  Read of size 4 at addr ffff888105594ff8 by task kworker/u8:4/128
  Workqueue: writeback wb_workfn (flush-ubifs_0_0)
  Call Trace:
    kasan_report.cold+0x81/0x165
    nand_write_page_swecc+0xa9/0x160
    ubifs_leb_write+0xf2/0x1b0 [ubifs]
    ubifs_wbuf_write_nolock+0x421/0x12c0 [ubifs]
    write_head+0xdc/0x1c0 [ubifs]
    ubifs_jnl_write_inode+0x627/0x960 [ubifs]
    wb_workfn+0x8af/0xb80

Function ubifs_wbuf_write_nolock() accepts that parameter 'len' is not 8
bytes aligned, the 'len' represents the true length of buf (which is
allocated in 'ubifs_jnl_xxx', eg. ubifs_jnl_write_inode), so
ubifs_wbuf_write_nolock() must handle the length read from 'buf' carefully
to write leb safely.

Fetch a reproducer in [Link].

Fixes: 1e51764a3c2ac0 ("UBIFS: add new flash file system")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=214785
Reported-by: Chengsong Ke <kechengsong@huawei.com>
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 fs/ubifs/io.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index 00b61dba62b7..b019dd6f7fa0 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -833,16 +833,42 @@ int ubifs_wbuf_write_nolock(struct ubifs_wbuf *wbuf, void *buf, int len)
 	 */
 	n = aligned_len >> c->max_write_shift;
 	if (n) {
-		n <<= c->max_write_shift;
+		int m = n - 1;
+
 		dbg_io("write %d bytes to LEB %d:%d", n, wbuf->lnum,
 		       wbuf->offs);
-		err = ubifs_leb_write(c, wbuf->lnum, buf + written,
-				      wbuf->offs, n);
+
+		if (m) {
+			/* '(n-1)<<c->max_write_shift < len' is always true. */
+			m <<= c->max_write_shift;
+			err = ubifs_leb_write(c, wbuf->lnum, buf + written,
+					      wbuf->offs, m);
+			if (err)
+				goto out;
+			wbuf->offs += m;
+			aligned_len -= m;
+			len -= m;
+			written += m;
+		}
+
+		/*
+		 * The non-written len of buf may be less than 'n' because
+		 * parameter 'len' is not 8 bytes aligned, so here we read
+		 * min(len, n) bytes from buf.
+		 */
+		n = 1 << c->max_write_shift;
+		memcpy(wbuf->buf, buf + written, min(len, n));
+		if (n > len) {
+			ubifs_assert(c, n - len < 8);
+			ubifs_pad(c, wbuf->buf + len, n - len);
+		}
+
+		err = ubifs_leb_write(c, wbuf->lnum, wbuf->buf, wbuf->offs, n);
 		if (err)
 			goto out;
 		wbuf->offs += n;
 		aligned_len -= n;
-		len -= n;
+		len -= min(len, n);
 		written += n;
 	}
 
-- 
2.31.1


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

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

* [PATCH 10/11] ubi: fastmap: Return error code if memory allocation fails in add_aeb()
  2021-10-25  3:41 ` Zhihao Cheng
@ 2021-10-25  3:41   ` Zhihao Cheng
  -1 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

Abort fastmap scanning and return error code if memory allocation fails
in add_aeb(). Otherwise ubi will get wrong peb statistics information
after scanning.

Fixes: dbb7d2a88d2a7b ("UBI: Add fastmap core")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 drivers/mtd/ubi/fastmap.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
index 022af59906aa..6b5f1ffd961b 100644
--- a/drivers/mtd/ubi/fastmap.c
+++ b/drivers/mtd/ubi/fastmap.c
@@ -468,7 +468,9 @@ static int scan_pool(struct ubi_device *ubi, struct ubi_attach_info *ai,
 			if (err == UBI_IO_FF_BITFLIPS)
 				scrub = 1;
 
-			add_aeb(ai, free, pnum, ec, scrub);
+			ret = add_aeb(ai, free, pnum, ec, scrub);
+			if (ret)
+				goto out;
 			continue;
 		} else if (err == 0 || err == UBI_IO_BITFLIPS) {
 			dbg_bld("Found non empty PEB:%i in pool", pnum);
@@ -638,8 +640,10 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
 		if (fm_pos >= fm_size)
 			goto fail_bad;
 
-		add_aeb(ai, &ai->free, be32_to_cpu(fmec->pnum),
-			be32_to_cpu(fmec->ec), 0);
+		ret = add_aeb(ai, &ai->free, be32_to_cpu(fmec->pnum),
+			      be32_to_cpu(fmec->ec), 0);
+		if (ret)
+			goto fail;
 	}
 
 	/* read EC values from used list */
@@ -649,8 +653,10 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
 		if (fm_pos >= fm_size)
 			goto fail_bad;
 
-		add_aeb(ai, &used, be32_to_cpu(fmec->pnum),
-			be32_to_cpu(fmec->ec), 0);
+		ret = add_aeb(ai, &used, be32_to_cpu(fmec->pnum),
+			      be32_to_cpu(fmec->ec), 0);
+		if (ret)
+			goto fail;
 	}
 
 	/* read EC values from scrub list */
@@ -660,8 +666,10 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
 		if (fm_pos >= fm_size)
 			goto fail_bad;
 
-		add_aeb(ai, &used, be32_to_cpu(fmec->pnum),
-			be32_to_cpu(fmec->ec), 1);
+		ret = add_aeb(ai, &used, be32_to_cpu(fmec->pnum),
+			      be32_to_cpu(fmec->ec), 1);
+		if (ret)
+			goto fail;
 	}
 
 	/* read EC values from erase list */
@@ -671,8 +679,10 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
 		if (fm_pos >= fm_size)
 			goto fail_bad;
 
-		add_aeb(ai, &ai->erase, be32_to_cpu(fmec->pnum),
-			be32_to_cpu(fmec->ec), 1);
+		ret = add_aeb(ai, &ai->erase, be32_to_cpu(fmec->pnum),
+			      be32_to_cpu(fmec->ec), 1);
+		if (ret)
+			goto fail;
 	}
 
 	ai->mean_ec = div_u64(ai->ec_sum, ai->ec_count);
-- 
2.31.1


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

* [PATCH 10/11] ubi: fastmap: Return error code if memory allocation fails in add_aeb()
@ 2021-10-25  3:41   ` Zhihao Cheng
  0 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

Abort fastmap scanning and return error code if memory allocation fails
in add_aeb(). Otherwise ubi will get wrong peb statistics information
after scanning.

Fixes: dbb7d2a88d2a7b ("UBI: Add fastmap core")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 drivers/mtd/ubi/fastmap.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
index 022af59906aa..6b5f1ffd961b 100644
--- a/drivers/mtd/ubi/fastmap.c
+++ b/drivers/mtd/ubi/fastmap.c
@@ -468,7 +468,9 @@ static int scan_pool(struct ubi_device *ubi, struct ubi_attach_info *ai,
 			if (err == UBI_IO_FF_BITFLIPS)
 				scrub = 1;
 
-			add_aeb(ai, free, pnum, ec, scrub);
+			ret = add_aeb(ai, free, pnum, ec, scrub);
+			if (ret)
+				goto out;
 			continue;
 		} else if (err == 0 || err == UBI_IO_BITFLIPS) {
 			dbg_bld("Found non empty PEB:%i in pool", pnum);
@@ -638,8 +640,10 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
 		if (fm_pos >= fm_size)
 			goto fail_bad;
 
-		add_aeb(ai, &ai->free, be32_to_cpu(fmec->pnum),
-			be32_to_cpu(fmec->ec), 0);
+		ret = add_aeb(ai, &ai->free, be32_to_cpu(fmec->pnum),
+			      be32_to_cpu(fmec->ec), 0);
+		if (ret)
+			goto fail;
 	}
 
 	/* read EC values from used list */
@@ -649,8 +653,10 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
 		if (fm_pos >= fm_size)
 			goto fail_bad;
 
-		add_aeb(ai, &used, be32_to_cpu(fmec->pnum),
-			be32_to_cpu(fmec->ec), 0);
+		ret = add_aeb(ai, &used, be32_to_cpu(fmec->pnum),
+			      be32_to_cpu(fmec->ec), 0);
+		if (ret)
+			goto fail;
 	}
 
 	/* read EC values from scrub list */
@@ -660,8 +666,10 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
 		if (fm_pos >= fm_size)
 			goto fail_bad;
 
-		add_aeb(ai, &used, be32_to_cpu(fmec->pnum),
-			be32_to_cpu(fmec->ec), 1);
+		ret = add_aeb(ai, &used, be32_to_cpu(fmec->pnum),
+			      be32_to_cpu(fmec->ec), 1);
+		if (ret)
+			goto fail;
 	}
 
 	/* read EC values from erase list */
@@ -671,8 +679,10 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
 		if (fm_pos >= fm_size)
 			goto fail_bad;
 
-		add_aeb(ai, &ai->erase, be32_to_cpu(fmec->pnum),
-			be32_to_cpu(fmec->ec), 1);
+		ret = add_aeb(ai, &ai->erase, be32_to_cpu(fmec->pnum),
+			      be32_to_cpu(fmec->ec), 1);
+		if (ret)
+			goto fail;
 	}
 
 	ai->mean_ec = div_u64(ai->ec_sum, ai->ec_count);
-- 
2.31.1


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

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

* [PATCH 11/11] ubi: fastmap: Add all fastmap pebs into 'ai->fastmap' when fm->used_blocks>=2
  2021-10-25  3:41 ` Zhihao Cheng
@ 2021-10-25  3:41   ` Zhihao Cheng
  -1 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

Fastmap pebs(pnum >= UBI_FM_MAX_START) won't be added into 'ai->fastmap'
while attaching ubi device if 'fm->used_blocks' is greater than 2, which
may cause warning from 'ubi_assert(ubi->good_peb_count == found_pebs)':

  UBI assert failed in ubi_wl_init at 1878 (pid 2409)
  Call Trace:
    ubi_wl_init.cold+0xae/0x2af [ubi]
    ubi_attach+0x1b0/0x780 [ubi]
    ubi_init+0x23a/0x3ad [ubi]
    load_module+0x22d2/0x2430

Reproduce:
  ID="0x20,0x33,0x00,0x00" # 16M 16KB PEB, 512 page
  modprobe nandsim id_bytes=$ID
  modprobe ubi mtd="0,0" fm_autoconvert  # Fastmap takes 2 pebs
  rmmod ubi
  modprobe ubi mtd="0,0" fm_autoconvert  # Attach by fastmap

Add all fastmap pebs into list 'ai->fastmap' to make sure they can be
counted into 'found_pebs'.

Fixes: fdf10ed710c0aa ("ubi: Rework Fastmap attach base code")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 drivers/mtd/ubi/fastmap.c | 41 ++++++++++++++-------------------------
 1 file changed, 15 insertions(+), 26 deletions(-)

diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
index 6b5f1ffd961b..88fdf8f5709f 100644
--- a/drivers/mtd/ubi/fastmap.c
+++ b/drivers/mtd/ubi/fastmap.c
@@ -828,24 +828,6 @@ static int find_fm_anchor(struct ubi_attach_info *ai)
 	return ret;
 }
 
-static struct ubi_ainf_peb *clone_aeb(struct ubi_attach_info *ai,
-				      struct ubi_ainf_peb *old)
-{
-	struct ubi_ainf_peb *new;
-
-	new = ubi_alloc_aeb(ai, old->pnum, old->ec);
-	if (!new)
-		return NULL;
-
-	new->vol_id = old->vol_id;
-	new->sqnum = old->sqnum;
-	new->lnum = old->lnum;
-	new->scrub = old->scrub;
-	new->copy_flag = old->copy_flag;
-
-	return new;
-}
-
 /**
  * ubi_scan_fastmap - scan the fastmap.
  * @ubi: UBI device object
@@ -875,15 +857,11 @@ int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai,
 	if (fm_anchor < 0)
 		return UBI_NO_FASTMAP;
 
-	/* Copy all (possible) fastmap blocks into our new attach structure. */
+	/* Add fastmap blocks(pnum < UBI_FM_MAX_START) into attach structure. */
 	list_for_each_entry(aeb, &scan_ai->fastmap, u.list) {
-		struct ubi_ainf_peb *new;
-
-		new = clone_aeb(ai, aeb);
-		if (!new)
-			return -ENOMEM;
-
-		list_add(&new->u.list, &ai->fastmap);
+		ret = add_aeb(ai, &ai->fastmap, aeb->pnum, aeb->ec, 0);
+		if (ret)
+			return ret;
 	}
 
 	down_write(&ubi->fm_protect);
@@ -1029,6 +1007,17 @@ int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai,
 				"err: %i)", i, pnum, ret);
 			goto free_hdr;
 		}
+
+		/*
+		 * Add left fastmap blocks (pnum >= UBI_FM_MAX_START) into
+		 * attach structure.
+		 */
+		if (pnum >= UBI_FM_MAX_START) {
+			ret = add_aeb(ai, &ai->fastmap, pnum,
+				      be64_to_cpu(ech->ec), 0);
+			if (ret)
+				goto free_hdr;
+		}
 	}
 
 	kfree(fmsb);
-- 
2.31.1


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

* [PATCH 11/11] ubi: fastmap: Add all fastmap pebs into 'ai->fastmap' when fm->used_blocks>=2
@ 2021-10-25  3:41   ` Zhihao Cheng
  0 siblings, 0 replies; 34+ messages in thread
From: Zhihao Cheng @ 2021-10-25  3:41 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: linux-mtd, linux-kernel, chengzhihao1

Fastmap pebs(pnum >= UBI_FM_MAX_START) won't be added into 'ai->fastmap'
while attaching ubi device if 'fm->used_blocks' is greater than 2, which
may cause warning from 'ubi_assert(ubi->good_peb_count == found_pebs)':

  UBI assert failed in ubi_wl_init at 1878 (pid 2409)
  Call Trace:
    ubi_wl_init.cold+0xae/0x2af [ubi]
    ubi_attach+0x1b0/0x780 [ubi]
    ubi_init+0x23a/0x3ad [ubi]
    load_module+0x22d2/0x2430

Reproduce:
  ID="0x20,0x33,0x00,0x00" # 16M 16KB PEB, 512 page
  modprobe nandsim id_bytes=$ID
  modprobe ubi mtd="0,0" fm_autoconvert  # Fastmap takes 2 pebs
  rmmod ubi
  modprobe ubi mtd="0,0" fm_autoconvert  # Attach by fastmap

Add all fastmap pebs into list 'ai->fastmap' to make sure they can be
counted into 'found_pebs'.

Fixes: fdf10ed710c0aa ("ubi: Rework Fastmap attach base code")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 drivers/mtd/ubi/fastmap.c | 41 ++++++++++++++-------------------------
 1 file changed, 15 insertions(+), 26 deletions(-)

diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
index 6b5f1ffd961b..88fdf8f5709f 100644
--- a/drivers/mtd/ubi/fastmap.c
+++ b/drivers/mtd/ubi/fastmap.c
@@ -828,24 +828,6 @@ static int find_fm_anchor(struct ubi_attach_info *ai)
 	return ret;
 }
 
-static struct ubi_ainf_peb *clone_aeb(struct ubi_attach_info *ai,
-				      struct ubi_ainf_peb *old)
-{
-	struct ubi_ainf_peb *new;
-
-	new = ubi_alloc_aeb(ai, old->pnum, old->ec);
-	if (!new)
-		return NULL;
-
-	new->vol_id = old->vol_id;
-	new->sqnum = old->sqnum;
-	new->lnum = old->lnum;
-	new->scrub = old->scrub;
-	new->copy_flag = old->copy_flag;
-
-	return new;
-}
-
 /**
  * ubi_scan_fastmap - scan the fastmap.
  * @ubi: UBI device object
@@ -875,15 +857,11 @@ int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai,
 	if (fm_anchor < 0)
 		return UBI_NO_FASTMAP;
 
-	/* Copy all (possible) fastmap blocks into our new attach structure. */
+	/* Add fastmap blocks(pnum < UBI_FM_MAX_START) into attach structure. */
 	list_for_each_entry(aeb, &scan_ai->fastmap, u.list) {
-		struct ubi_ainf_peb *new;
-
-		new = clone_aeb(ai, aeb);
-		if (!new)
-			return -ENOMEM;
-
-		list_add(&new->u.list, &ai->fastmap);
+		ret = add_aeb(ai, &ai->fastmap, aeb->pnum, aeb->ec, 0);
+		if (ret)
+			return ret;
 	}
 
 	down_write(&ubi->fm_protect);
@@ -1029,6 +1007,17 @@ int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai,
 				"err: %i)", i, pnum, ret);
 			goto free_hdr;
 		}
+
+		/*
+		 * Add left fastmap blocks (pnum >= UBI_FM_MAX_START) into
+		 * attach structure.
+		 */
+		if (pnum >= UBI_FM_MAX_START) {
+			ret = add_aeb(ai, &ai->fastmap, pnum,
+				      be64_to_cpu(ech->ec), 0);
+			if (ret)
+				goto free_hdr;
+		}
 	}
 
 	kfree(fmsb);
-- 
2.31.1


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

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

* Re: [PATCH 05/11] ubifs: Rename whiteout atomically
  2021-10-25  3:41   ` Zhihao Cheng
  (?)
@ 2021-10-25 14:57     ` kernel test robot
  -1 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2021-10-25 14:57 UTC (permalink / raw)
  To: Zhihao Cheng, richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: llvm, kbuild-all, linux-mtd, linux-kernel, chengzhihao1

[-- Attachment #1: Type: text/plain, Size: 3455 bytes --]

Hi Zhihao,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on linus/master rw-ubifs/next v5.15-rc6 next-20211025]
[cannot apply to rw-ubifs/fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2f111a6fd5b5297b4e92f53798ca086f7c7d33a4
config: i386-randconfig-a006-20211025 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project a461fa64bb37cffd73f683c74f6b0780379fc2ca)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/8a6ca5bc1be93cca9c7b0167a71bdd338f854600
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
        git checkout 8a6ca5bc1be93cca9c7b0167a71bdd338f854600
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/ubifs/dir.c:357:22: warning: variable 'ui' set but not used [-Wunused-but-set-variable]
           struct ubifs_inode *ui;
                               ^
   1 warning generated.


vim +/ui +357 fs/ubifs/dir.c

   351	
   352	static struct inode *create_whiteout(struct inode *dir, struct dentry *dentry,
   353					     umode_t mode)
   354	{
   355		int err;
   356		struct inode *inode;
 > 357		struct ubifs_inode *ui;
   358		struct ubifs_info *c = dir->i_sb->s_fs_info;
   359		struct fscrypt_name nm;
   360	
   361		/*
   362		 * Create an inode('nlink = 1') for whiteout without updating journal,
   363		 * let ubifs_jnl_rename() store it on flash to complete rename whiteout
   364		 * atomically.
   365		 */
   366	
   367		dbg_gen("dent '%pd', mode %#hx in dir ino %lu",
   368			dentry, mode, dir->i_ino);
   369	
   370		err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm);
   371		if (err)
   372			return ERR_PTR(err);
   373	
   374		inode = ubifs_new_inode(c, dir, mode);
   375		if (IS_ERR(inode)) {
   376			err = PTR_ERR(inode);
   377			goto out_free;
   378		}
   379		ui = ubifs_inode(inode);
   380	
   381		init_special_inode(inode, inode->i_mode, WHITEOUT_DEV);
   382		ubifs_assert(c, inode->i_op == &ubifs_file_inode_operations);
   383	
   384		err = ubifs_init_security(dir, inode, &dentry->d_name);
   385		if (err)
   386			goto out_inode;
   387	
   388		/* The dir size is updated by do_rename. */
   389		insert_inode_hash(inode);
   390	
   391		return inode;
   392	
   393	out_inode:
   394		make_bad_inode(inode);
   395		iput(inode);
   396	out_free:
   397		fscrypt_free_filename(&nm);
   398		ubifs_err(c, "cannot create whiteout file, error %d", err);
   399		return ERR_PTR(err);
   400	}
   401	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34879 bytes --]

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

* Re: [PATCH 05/11] ubifs: Rename whiteout atomically
@ 2021-10-25 14:57     ` kernel test robot
  0 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2021-10-25 14:57 UTC (permalink / raw)
  To: Zhihao Cheng, richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: llvm, kbuild-all, linux-mtd, linux-kernel, chengzhihao1

[-- Attachment #1: Type: text/plain, Size: 3455 bytes --]

Hi Zhihao,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on linus/master rw-ubifs/next v5.15-rc6 next-20211025]
[cannot apply to rw-ubifs/fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2f111a6fd5b5297b4e92f53798ca086f7c7d33a4
config: i386-randconfig-a006-20211025 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project a461fa64bb37cffd73f683c74f6b0780379fc2ca)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/8a6ca5bc1be93cca9c7b0167a71bdd338f854600
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
        git checkout 8a6ca5bc1be93cca9c7b0167a71bdd338f854600
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/ubifs/dir.c:357:22: warning: variable 'ui' set but not used [-Wunused-but-set-variable]
           struct ubifs_inode *ui;
                               ^
   1 warning generated.


vim +/ui +357 fs/ubifs/dir.c

   351	
   352	static struct inode *create_whiteout(struct inode *dir, struct dentry *dentry,
   353					     umode_t mode)
   354	{
   355		int err;
   356		struct inode *inode;
 > 357		struct ubifs_inode *ui;
   358		struct ubifs_info *c = dir->i_sb->s_fs_info;
   359		struct fscrypt_name nm;
   360	
   361		/*
   362		 * Create an inode('nlink = 1') for whiteout without updating journal,
   363		 * let ubifs_jnl_rename() store it on flash to complete rename whiteout
   364		 * atomically.
   365		 */
   366	
   367		dbg_gen("dent '%pd', mode %#hx in dir ino %lu",
   368			dentry, mode, dir->i_ino);
   369	
   370		err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm);
   371		if (err)
   372			return ERR_PTR(err);
   373	
   374		inode = ubifs_new_inode(c, dir, mode);
   375		if (IS_ERR(inode)) {
   376			err = PTR_ERR(inode);
   377			goto out_free;
   378		}
   379		ui = ubifs_inode(inode);
   380	
   381		init_special_inode(inode, inode->i_mode, WHITEOUT_DEV);
   382		ubifs_assert(c, inode->i_op == &ubifs_file_inode_operations);
   383	
   384		err = ubifs_init_security(dir, inode, &dentry->d_name);
   385		if (err)
   386			goto out_inode;
   387	
   388		/* The dir size is updated by do_rename. */
   389		insert_inode_hash(inode);
   390	
   391		return inode;
   392	
   393	out_inode:
   394		make_bad_inode(inode);
   395		iput(inode);
   396	out_free:
   397		fscrypt_free_filename(&nm);
   398		ubifs_err(c, "cannot create whiteout file, error %d", err);
   399		return ERR_PTR(err);
   400	}
   401	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34879 bytes --]

[-- Attachment #3: Type: text/plain, Size: 144 bytes --]

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

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

* Re: [PATCH 05/11] ubifs: Rename whiteout atomically
@ 2021-10-25 14:57     ` kernel test robot
  0 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2021-10-25 14:57 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3550 bytes --]

Hi Zhihao,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on linus/master rw-ubifs/next v5.15-rc6 next-20211025]
[cannot apply to rw-ubifs/fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2f111a6fd5b5297b4e92f53798ca086f7c7d33a4
config: i386-randconfig-a006-20211025 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project a461fa64bb37cffd73f683c74f6b0780379fc2ca)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/8a6ca5bc1be93cca9c7b0167a71bdd338f854600
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
        git checkout 8a6ca5bc1be93cca9c7b0167a71bdd338f854600
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/ubifs/dir.c:357:22: warning: variable 'ui' set but not used [-Wunused-but-set-variable]
           struct ubifs_inode *ui;
                               ^
   1 warning generated.


vim +/ui +357 fs/ubifs/dir.c

   351	
   352	static struct inode *create_whiteout(struct inode *dir, struct dentry *dentry,
   353					     umode_t mode)
   354	{
   355		int err;
   356		struct inode *inode;
 > 357		struct ubifs_inode *ui;
   358		struct ubifs_info *c = dir->i_sb->s_fs_info;
   359		struct fscrypt_name nm;
   360	
   361		/*
   362		 * Create an inode('nlink = 1') for whiteout without updating journal,
   363		 * let ubifs_jnl_rename() store it on flash to complete rename whiteout
   364		 * atomically.
   365		 */
   366	
   367		dbg_gen("dent '%pd', mode %#hx in dir ino %lu",
   368			dentry, mode, dir->i_ino);
   369	
   370		err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm);
   371		if (err)
   372			return ERR_PTR(err);
   373	
   374		inode = ubifs_new_inode(c, dir, mode);
   375		if (IS_ERR(inode)) {
   376			err = PTR_ERR(inode);
   377			goto out_free;
   378		}
   379		ui = ubifs_inode(inode);
   380	
   381		init_special_inode(inode, inode->i_mode, WHITEOUT_DEV);
   382		ubifs_assert(c, inode->i_op == &ubifs_file_inode_operations);
   383	
   384		err = ubifs_init_security(dir, inode, &dentry->d_name);
   385		if (err)
   386			goto out_inode;
   387	
   388		/* The dir size is updated by do_rename. */
   389		insert_inode_hash(inode);
   390	
   391		return inode;
   392	
   393	out_inode:
   394		make_bad_inode(inode);
   395		iput(inode);
   396	out_free:
   397		fscrypt_free_filename(&nm);
   398		ubifs_err(c, "cannot create whiteout file, error %d", err);
   399		return ERR_PTR(err);
   400	}
   401	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 34879 bytes --]

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

* Re: [PATCH 05/11] ubifs: Rename whiteout atomically
  2021-10-25  3:41   ` Zhihao Cheng
  (?)
@ 2021-10-29  5:33     ` kernel test robot
  -1 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2021-10-29  5:33 UTC (permalink / raw)
  To: Zhihao Cheng, richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: kbuild-all, linux-mtd, linux-kernel, chengzhihao1

[-- Attachment #1: Type: text/plain, Size: 3272 bytes --]

Hi Zhihao,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linux/master]
[also build test ERROR on linus/master rw-ubifs/next v5.15-rc7 next-20211028]
[cannot apply to rw-ubifs/fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2f111a6fd5b5297b4e92f53798ca086f7c7d33a4
config: x86_64-buildonly-randconfig-r004-20211028 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/8a6ca5bc1be93cca9c7b0167a71bdd338f854600
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
        git checkout 8a6ca5bc1be93cca9c7b0167a71bdd338f854600
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   fs/ubifs/dir.c: In function 'create_whiteout':
>> fs/ubifs/dir.c:357:22: error: variable 'ui' set but not used [-Werror=unused-but-set-variable]
     357 |  struct ubifs_inode *ui;
         |                      ^~
   cc1: all warnings being treated as errors


vim +/ui +357 fs/ubifs/dir.c

   351	
   352	static struct inode *create_whiteout(struct inode *dir, struct dentry *dentry,
   353					     umode_t mode)
   354	{
   355		int err;
   356		struct inode *inode;
 > 357		struct ubifs_inode *ui;
   358		struct ubifs_info *c = dir->i_sb->s_fs_info;
   359		struct fscrypt_name nm;
   360	
   361		/*
   362		 * Create an inode('nlink = 1') for whiteout without updating journal,
   363		 * let ubifs_jnl_rename() store it on flash to complete rename whiteout
   364		 * atomically.
   365		 */
   366	
   367		dbg_gen("dent '%pd', mode %#hx in dir ino %lu",
   368			dentry, mode, dir->i_ino);
   369	
   370		err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm);
   371		if (err)
   372			return ERR_PTR(err);
   373	
   374		inode = ubifs_new_inode(c, dir, mode);
   375		if (IS_ERR(inode)) {
   376			err = PTR_ERR(inode);
   377			goto out_free;
   378		}
   379		ui = ubifs_inode(inode);
   380	
   381		init_special_inode(inode, inode->i_mode, WHITEOUT_DEV);
   382		ubifs_assert(c, inode->i_op == &ubifs_file_inode_operations);
   383	
   384		err = ubifs_init_security(dir, inode, &dentry->d_name);
   385		if (err)
   386			goto out_inode;
   387	
   388		/* The dir size is updated by do_rename. */
   389		insert_inode_hash(inode);
   390	
   391		return inode;
   392	
   393	out_inode:
   394		make_bad_inode(inode);
   395		iput(inode);
   396	out_free:
   397		fscrypt_free_filename(&nm);
   398		ubifs_err(c, "cannot create whiteout file, error %d", err);
   399		return ERR_PTR(err);
   400	}
   401	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35706 bytes --]

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

* Re: [PATCH 05/11] ubifs: Rename whiteout atomically
@ 2021-10-29  5:33     ` kernel test robot
  0 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2021-10-29  5:33 UTC (permalink / raw)
  To: Zhihao Cheng, richard, miquel.raynal, vigneshr, mcoquelin.stm32,
	alexandre.torgue, Artem.Bityutskiy, ext-adrian.hunter
  Cc: kbuild-all, linux-mtd, linux-kernel, chengzhihao1

[-- Attachment #1: Type: text/plain, Size: 3272 bytes --]

Hi Zhihao,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linux/master]
[also build test ERROR on linus/master rw-ubifs/next v5.15-rc7 next-20211028]
[cannot apply to rw-ubifs/fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2f111a6fd5b5297b4e92f53798ca086f7c7d33a4
config: x86_64-buildonly-randconfig-r004-20211028 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/8a6ca5bc1be93cca9c7b0167a71bdd338f854600
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
        git checkout 8a6ca5bc1be93cca9c7b0167a71bdd338f854600
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   fs/ubifs/dir.c: In function 'create_whiteout':
>> fs/ubifs/dir.c:357:22: error: variable 'ui' set but not used [-Werror=unused-but-set-variable]
     357 |  struct ubifs_inode *ui;
         |                      ^~
   cc1: all warnings being treated as errors


vim +/ui +357 fs/ubifs/dir.c

   351	
   352	static struct inode *create_whiteout(struct inode *dir, struct dentry *dentry,
   353					     umode_t mode)
   354	{
   355		int err;
   356		struct inode *inode;
 > 357		struct ubifs_inode *ui;
   358		struct ubifs_info *c = dir->i_sb->s_fs_info;
   359		struct fscrypt_name nm;
   360	
   361		/*
   362		 * Create an inode('nlink = 1') for whiteout without updating journal,
   363		 * let ubifs_jnl_rename() store it on flash to complete rename whiteout
   364		 * atomically.
   365		 */
   366	
   367		dbg_gen("dent '%pd', mode %#hx in dir ino %lu",
   368			dentry, mode, dir->i_ino);
   369	
   370		err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm);
   371		if (err)
   372			return ERR_PTR(err);
   373	
   374		inode = ubifs_new_inode(c, dir, mode);
   375		if (IS_ERR(inode)) {
   376			err = PTR_ERR(inode);
   377			goto out_free;
   378		}
   379		ui = ubifs_inode(inode);
   380	
   381		init_special_inode(inode, inode->i_mode, WHITEOUT_DEV);
   382		ubifs_assert(c, inode->i_op == &ubifs_file_inode_operations);
   383	
   384		err = ubifs_init_security(dir, inode, &dentry->d_name);
   385		if (err)
   386			goto out_inode;
   387	
   388		/* The dir size is updated by do_rename. */
   389		insert_inode_hash(inode);
   390	
   391		return inode;
   392	
   393	out_inode:
   394		make_bad_inode(inode);
   395		iput(inode);
   396	out_free:
   397		fscrypt_free_filename(&nm);
   398		ubifs_err(c, "cannot create whiteout file, error %d", err);
   399		return ERR_PTR(err);
   400	}
   401	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35706 bytes --]

[-- Attachment #3: Type: text/plain, Size: 144 bytes --]

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

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

* Re: [PATCH 05/11] ubifs: Rename whiteout atomically
@ 2021-10-29  5:33     ` kernel test robot
  0 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2021-10-29  5:33 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3366 bytes --]

Hi Zhihao,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linux/master]
[also build test ERROR on linus/master rw-ubifs/next v5.15-rc7 next-20211028]
[cannot apply to rw-ubifs/fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2f111a6fd5b5297b4e92f53798ca086f7c7d33a4
config: x86_64-buildonly-randconfig-r004-20211028 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/8a6ca5bc1be93cca9c7b0167a71bdd338f854600
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
        git checkout 8a6ca5bc1be93cca9c7b0167a71bdd338f854600
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   fs/ubifs/dir.c: In function 'create_whiteout':
>> fs/ubifs/dir.c:357:22: error: variable 'ui' set but not used [-Werror=unused-but-set-variable]
     357 |  struct ubifs_inode *ui;
         |                      ^~
   cc1: all warnings being treated as errors


vim +/ui +357 fs/ubifs/dir.c

   351	
   352	static struct inode *create_whiteout(struct inode *dir, struct dentry *dentry,
   353					     umode_t mode)
   354	{
   355		int err;
   356		struct inode *inode;
 > 357		struct ubifs_inode *ui;
   358		struct ubifs_info *c = dir->i_sb->s_fs_info;
   359		struct fscrypt_name nm;
   360	
   361		/*
   362		 * Create an inode('nlink = 1') for whiteout without updating journal,
   363		 * let ubifs_jnl_rename() store it on flash to complete rename whiteout
   364		 * atomically.
   365		 */
   366	
   367		dbg_gen("dent '%pd', mode %#hx in dir ino %lu",
   368			dentry, mode, dir->i_ino);
   369	
   370		err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm);
   371		if (err)
   372			return ERR_PTR(err);
   373	
   374		inode = ubifs_new_inode(c, dir, mode);
   375		if (IS_ERR(inode)) {
   376			err = PTR_ERR(inode);
   377			goto out_free;
   378		}
   379		ui = ubifs_inode(inode);
   380	
   381		init_special_inode(inode, inode->i_mode, WHITEOUT_DEV);
   382		ubifs_assert(c, inode->i_op == &ubifs_file_inode_operations);
   383	
   384		err = ubifs_init_security(dir, inode, &dentry->d_name);
   385		if (err)
   386			goto out_inode;
   387	
   388		/* The dir size is updated by do_rename. */
   389		insert_inode_hash(inode);
   390	
   391		return inode;
   392	
   393	out_inode:
   394		make_bad_inode(inode);
   395		iput(inode);
   396	out_free:
   397		fscrypt_free_filename(&nm);
   398		ubifs_err(c, "cannot create whiteout file, error %d", err);
   399		return ERR_PTR(err);
   400	}
   401	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 35706 bytes --]

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

* Re: [PATCH 05/11] ubifs: Rename whiteout atomically
  2021-10-30 10:27 [PATCH 05/11] ubifs: Rename whiteout atomically kernel test robot
  2021-11-05  5:35   ` kernel test robot
@ 2021-11-05  5:35   ` kernel test robot
  0 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2021-11-05  5:35 UTC (permalink / raw)
  To: Zhihao Cheng; +Cc: llvm, kbuild-all, linux-mtd, Linux Kernel Mailing List

[-- Attachment #1: Type: text/plain, Size: 5598 bytes --]

Hi Zhihao,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on linus/master rw-ubifs/next v5.15-rc7 next-20211029]
[cannot apply to rw-ubifs/fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2f111a6fd5b5297b4e92f53798ca086f7c7d33a4
config: i386-randconfig-c001-20211028 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 5db7568a6a1fcb408eb8988abdaff2a225a8eb72)
reproduce (this is a W=1 build):
         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
         chmod +x ~/bin/make.cross
         # https://github.com/0day-ci/linux/commit/8a6ca5bc1be93cca9c7b0167a71bdd338f854600
         git remote add linux-review https://github.com/0day-ci/linux
         git fetch --no-tags linux-review Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
         git checkout 8a6ca5bc1be93cca9c7b0167a71bdd338f854600
         # save the attached .config to linux build tree
         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386 clang-analyzer

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


clang-analyzer warnings: (new ones prefixed by >>)

 >> fs/ubifs/dir.c:379:2: warning: Value stored to 'ui' is never read [clang-analyzer-deadcode.DeadStores]
            ui = ubifs_inode(inode);
            ^    ~~~~~~~~~~~~~~~~~~


vim +/ui +379 fs/ubifs/dir.c

1e51764a3c2ac0 Artem Bityutskiy 2008-07-14  351
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  352  static struct inode *create_whiteout(struct inode *dir, struct dentry *dentry,
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  353  				     umode_t mode)
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  354  {
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  355  	int err;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  356  	struct inode *inode;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  357  	struct ubifs_inode *ui;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  358  	struct ubifs_info *c = dir->i_sb->s_fs_info;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  359  	struct fscrypt_name nm;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  360
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  361  	/*
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  362  	 * Create an inode('nlink = 1') for whiteout without updating journal,
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  363  	 * let ubifs_jnl_rename() store it on flash to complete rename whiteout
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  364  	 * atomically.
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  365  	 */
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  366
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  367  	dbg_gen("dent '%pd', mode %#hx in dir ino %lu",
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  368  		dentry, mode, dir->i_ino);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  369
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  370  	err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  371  	if (err)
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  372  		return ERR_PTR(err);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  373
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  374  	inode = ubifs_new_inode(c, dir, mode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  375  	if (IS_ERR(inode)) {
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  376  		err = PTR_ERR(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  377  		goto out_free;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  378  	}
8a6ca5bc1be93c Zhihao Cheng     2021-10-25 @379  	ui = ubifs_inode(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  380
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  381  	init_special_inode(inode, inode->i_mode, WHITEOUT_DEV);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  382  	ubifs_assert(c, inode->i_op == &ubifs_file_inode_operations);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  383
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  384  	err = ubifs_init_security(dir, inode, &dentry->d_name);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  385  	if (err)
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  386  		goto out_inode;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  387
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  388  	/* The dir size is updated by do_rename. */
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  389  	insert_inode_hash(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  390
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  391  	return inode;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  392
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  393  out_inode:
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  394  	make_bad_inode(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  395  	iput(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  396  out_free:
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  397  	fscrypt_free_filename(&nm);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  398  	ubifs_err(c, "cannot create whiteout file, error %d", err);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  399  	return ERR_PTR(err);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  400  }
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  401

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32366 bytes --]

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

* Re: [PATCH 05/11] ubifs: Rename whiteout atomically
@ 2021-11-05  5:35   ` kernel test robot
  0 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2021-11-05  5:35 UTC (permalink / raw)
  To: Zhihao Cheng; +Cc: llvm, kbuild-all, linux-mtd, Linux Kernel Mailing List

[-- Attachment #1: Type: text/plain, Size: 5598 bytes --]

Hi Zhihao,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on linus/master rw-ubifs/next v5.15-rc7 next-20211029]
[cannot apply to rw-ubifs/fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2f111a6fd5b5297b4e92f53798ca086f7c7d33a4
config: i386-randconfig-c001-20211028 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 5db7568a6a1fcb408eb8988abdaff2a225a8eb72)
reproduce (this is a W=1 build):
         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
         chmod +x ~/bin/make.cross
         # https://github.com/0day-ci/linux/commit/8a6ca5bc1be93cca9c7b0167a71bdd338f854600
         git remote add linux-review https://github.com/0day-ci/linux
         git fetch --no-tags linux-review Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
         git checkout 8a6ca5bc1be93cca9c7b0167a71bdd338f854600
         # save the attached .config to linux build tree
         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386 clang-analyzer

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


clang-analyzer warnings: (new ones prefixed by >>)

 >> fs/ubifs/dir.c:379:2: warning: Value stored to 'ui' is never read [clang-analyzer-deadcode.DeadStores]
            ui = ubifs_inode(inode);
            ^    ~~~~~~~~~~~~~~~~~~


vim +/ui +379 fs/ubifs/dir.c

1e51764a3c2ac0 Artem Bityutskiy 2008-07-14  351
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  352  static struct inode *create_whiteout(struct inode *dir, struct dentry *dentry,
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  353  				     umode_t mode)
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  354  {
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  355  	int err;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  356  	struct inode *inode;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  357  	struct ubifs_inode *ui;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  358  	struct ubifs_info *c = dir->i_sb->s_fs_info;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  359  	struct fscrypt_name nm;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  360
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  361  	/*
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  362  	 * Create an inode('nlink = 1') for whiteout without updating journal,
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  363  	 * let ubifs_jnl_rename() store it on flash to complete rename whiteout
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  364  	 * atomically.
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  365  	 */
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  366
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  367  	dbg_gen("dent '%pd', mode %#hx in dir ino %lu",
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  368  		dentry, mode, dir->i_ino);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  369
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  370  	err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  371  	if (err)
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  372  		return ERR_PTR(err);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  373
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  374  	inode = ubifs_new_inode(c, dir, mode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  375  	if (IS_ERR(inode)) {
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  376  		err = PTR_ERR(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  377  		goto out_free;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  378  	}
8a6ca5bc1be93c Zhihao Cheng     2021-10-25 @379  	ui = ubifs_inode(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  380
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  381  	init_special_inode(inode, inode->i_mode, WHITEOUT_DEV);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  382  	ubifs_assert(c, inode->i_op == &ubifs_file_inode_operations);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  383
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  384  	err = ubifs_init_security(dir, inode, &dentry->d_name);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  385  	if (err)
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  386  		goto out_inode;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  387
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  388  	/* The dir size is updated by do_rename. */
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  389  	insert_inode_hash(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  390
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  391  	return inode;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  392
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  393  out_inode:
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  394  	make_bad_inode(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  395  	iput(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  396  out_free:
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  397  	fscrypt_free_filename(&nm);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  398  	ubifs_err(c, "cannot create whiteout file, error %d", err);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  399  	return ERR_PTR(err);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  400  }
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  401

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32366 bytes --]

[-- Attachment #3: Type: text/plain, Size: 144 bytes --]

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

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

* Re: [PATCH 05/11] ubifs: Rename whiteout atomically
@ 2021-11-05  5:35   ` kernel test robot
  0 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2021-11-05  5:35 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 5694 bytes --]

Hi Zhihao,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on linus/master rw-ubifs/next v5.15-rc7 next-20211029]
[cannot apply to rw-ubifs/fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2f111a6fd5b5297b4e92f53798ca086f7c7d33a4
config: i386-randconfig-c001-20211028 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 5db7568a6a1fcb408eb8988abdaff2a225a8eb72)
reproduce (this is a W=1 build):
         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
         chmod +x ~/bin/make.cross
         # https://github.com/0day-ci/linux/commit/8a6ca5bc1be93cca9c7b0167a71bdd338f854600
         git remote add linux-review https://github.com/0day-ci/linux
         git fetch --no-tags linux-review Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
         git checkout 8a6ca5bc1be93cca9c7b0167a71bdd338f854600
         # save the attached .config to linux build tree
         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386 clang-analyzer

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


clang-analyzer warnings: (new ones prefixed by >>)

 >> fs/ubifs/dir.c:379:2: warning: Value stored to 'ui' is never read [clang-analyzer-deadcode.DeadStores]
            ui = ubifs_inode(inode);
            ^    ~~~~~~~~~~~~~~~~~~


vim +/ui +379 fs/ubifs/dir.c

1e51764a3c2ac0 Artem Bityutskiy 2008-07-14  351
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  352  static struct inode *create_whiteout(struct inode *dir, struct dentry *dentry,
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  353  				     umode_t mode)
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  354  {
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  355  	int err;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  356  	struct inode *inode;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  357  	struct ubifs_inode *ui;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  358  	struct ubifs_info *c = dir->i_sb->s_fs_info;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  359  	struct fscrypt_name nm;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  360
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  361  	/*
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  362  	 * Create an inode('nlink = 1') for whiteout without updating journal,
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  363  	 * let ubifs_jnl_rename() store it on flash to complete rename whiteout
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  364  	 * atomically.
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  365  	 */
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  366
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  367  	dbg_gen("dent '%pd', mode %#hx in dir ino %lu",
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  368  		dentry, mode, dir->i_ino);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  369
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  370  	err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  371  	if (err)
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  372  		return ERR_PTR(err);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  373
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  374  	inode = ubifs_new_inode(c, dir, mode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  375  	if (IS_ERR(inode)) {
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  376  		err = PTR_ERR(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  377  		goto out_free;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  378  	}
8a6ca5bc1be93c Zhihao Cheng     2021-10-25 @379  	ui = ubifs_inode(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  380
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  381  	init_special_inode(inode, inode->i_mode, WHITEOUT_DEV);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  382  	ubifs_assert(c, inode->i_op == &ubifs_file_inode_operations);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  383
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  384  	err = ubifs_init_security(dir, inode, &dentry->d_name);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  385  	if (err)
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  386  		goto out_inode;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  387
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  388  	/* The dir size is updated by do_rename. */
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  389  	insert_inode_hash(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  390
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  391  	return inode;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  392
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  393  out_inode:
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  394  	make_bad_inode(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  395  	iput(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  396  out_free:
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  397  	fscrypt_free_filename(&nm);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  398  	ubifs_err(c, "cannot create whiteout file, error %d", err);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  399  	return ERR_PTR(err);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  400  }
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  401

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 32366 bytes --]

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

* Re: [PATCH 05/11] ubifs: Rename whiteout atomically
@ 2021-10-30 10:27 kernel test robot
  2021-11-05  5:35   ` kernel test robot
  0 siblings, 1 reply; 34+ messages in thread
From: kernel test robot @ 2021-10-30 10:27 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 17125 bytes --]

CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20211025034116.3544321-6-chengzhihao1@huawei.com>
References: <20211025034116.3544321-6-chengzhihao1@huawei.com>
TO: Zhihao Cheng <chengzhihao1@huawei.com>
TO: richard(a)nod.at
TO: miquel.raynal(a)bootlin.com
TO: vigneshr(a)ti.com
TO: mcoquelin.stm32(a)gmail.com
TO: alexandre.torgue(a)foss.st.com
TO: Artem.Bityutskiy(a)nokia.com
TO: ext-adrian.hunter(a)nokia.com
CC: linux-mtd(a)lists.infradead.org
CC: linux-kernel(a)vger.kernel.org
CC: chengzhihao1(a)huawei.com

Hi Zhihao,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on linus/master rw-ubifs/next v5.15-rc7 next-20211029]
[cannot apply to rw-ubifs/fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2f111a6fd5b5297b4e92f53798ca086f7c7d33a4
:::::: branch date: 5 days ago
:::::: commit date: 5 days ago
config: i386-randconfig-c001-20211028 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 5db7568a6a1fcb408eb8988abdaff2a225a8eb72)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/8a6ca5bc1be93cca9c7b0167a71bdd338f854600
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Zhihao-Cheng/Some-bugfixs-for-ubifs-ubi/20211025-113114
        git checkout 8a6ca5bc1be93cca9c7b0167a71bdd338f854600
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386 clang-analyzer 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


clang-analyzer warnings: (new ones prefixed by >>)
           ^
   include/linux/printk.h:131:2: note: expanded from macro 'no_printk'
           if (0)                                          \
           ^
   fs/ubifs/dir.c:205:8: note: Calling 'fscrypt_prepare_lookup'
           err = fscrypt_prepare_lookup(dir, dentry, &nm);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/fscrypt.h:882:6: note: Assuming the condition is false
           if (IS_ENCRYPTED(dir))
               ^
   include/linux/fs.h:2292:30: note: expanded from macro 'IS_ENCRYPTED'
   #define IS_ENCRYPTED(inode)     ((inode)->i_flags & S_ENCRYPTED)
                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/fscrypt.h:882:2: note: Taking false branch
           if (IS_ENCRYPTED(dir))
           ^
   include/linux/fscrypt.h:887:2: note: Value assigned to 'nm.disk_name.name'
           fname->disk_name.name = (unsigned char *)dentry->d_name.name;
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/ubifs/dir.c:205:8: note: Returning from 'fscrypt_prepare_lookup'
           err = fscrypt_prepare_lookup(dir, dentry, &nm);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/ubifs/dir.c:207:2: note: Taking false branch
           if (err == -ENOENT)
           ^
   fs/ubifs/dir.c:209:6: note: 'err' is 0
           if (err)
               ^~~
   fs/ubifs/dir.c:209:2: note: Taking false branch
           if (err)
           ^
   fs/ubifs/dir.c:212:6: note: Assuming field 'len' is <= UBIFS_MAX_NLEN
           if (fname_len(&nm) > UBIFS_MAX_NLEN) {
               ^
   include/linux/fscrypt.h:44:23: note: expanded from macro 'fname_len'
   #define fname_len(p)            ((p)->disk_name.len)
                                   ^
   fs/ubifs/dir.c:212:2: note: Taking false branch
           if (fname_len(&nm) > UBIFS_MAX_NLEN) {
           ^
   fs/ubifs/dir.c:218:6: note: Assuming 'dent' is non-null
           if (!dent) {
               ^~~~~
   fs/ubifs/dir.c:218:2: note: Taking false branch
           if (!dent) {
           ^
   fs/ubifs/dir.c:223:6: note: Assuming field 'name' is equal to NULL
           if (fname_name(&nm) == NULL) {
               ^
   include/linux/fscrypt.h:43:24: note: expanded from macro 'fname_name'
   #define fname_name(p)           ((p)->disk_name.name)
                                   ^
   fs/ubifs/dir.c:223:2: note: Taking true branch
           if (fname_name(&nm) == NULL) {
           ^
   fs/ubifs/dir.c:224:3: note: Taking false branch
                   if (nm.hash & ~UBIFS_S_KEY_HASH_MASK)
                   ^
   fs/ubifs/dir.c:233:6: note: Assuming 'err' is 0
           if (err) {
               ^~~
   fs/ubifs/dir.c:233:2: note: Taking false branch
           if (err) {
           ^
   fs/ubifs/dir.c:241:6: note: Calling 'dbg_check_name'
           if (dbg_check_name(c, dent, &nm)) {
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/ubifs/dir.c:184:7: note: Calling 'dbg_is_chk_gen'
           if (!dbg_is_chk_gen(c))
                ^~~~~~~~~~~~~~~~~
   fs/ubifs/debug.h:206:12: note: Assuming field 'chk_gen' is not equal to 0
           return !!(ubifs_dbg.chk_gen || c->dbg->chk_gen);
                     ^~~~~~~~~~~~~~~~~
   fs/ubifs/debug.h:206:30: note: Left side of '||' is true
           return !!(ubifs_dbg.chk_gen || c->dbg->chk_gen);
                                       ^
   fs/ubifs/debug.h:206:2: note: Returning the value 1, which participates in a condition later
           return !!(ubifs_dbg.chk_gen || c->dbg->chk_gen);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/ubifs/dir.c:184:7: note: Returning from 'dbg_is_chk_gen'
           if (!dbg_is_chk_gen(c))
                ^~~~~~~~~~~~~~~~~
   fs/ubifs/dir.c:184:2: note: Taking false branch
           if (!dbg_is_chk_gen(c))
           ^
   fs/ubifs/dir.c:186:6: note: Assuming field 'nlen' is equal to field 'len'
           if (le16_to_cpu(dent->nlen) != fname_len(nm))
               ^
   include/linux/byteorder/generic.h:91:21: note: expanded from macro 'le16_to_cpu'
   #define le16_to_cpu __le16_to_cpu
                       ^
   include/uapi/linux/byteorder/little_endian.h:36:26: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                            ^
   fs/ubifs/dir.c:186:2: note: Taking false branch
           if (le16_to_cpu(dent->nlen) != fname_len(nm))
           ^
   fs/ubifs/dir.c:188:6: note: Null pointer passed as 2nd argument to memory comparison function
           if (memcmp(dent->name, fname_name(nm), fname_len(nm)))
               ^
>> fs/ubifs/dir.c:379:2: warning: Value stored to 'ui' is never read [clang-analyzer-deadcode.DeadStores]
           ui = ubifs_inode(inode);
           ^    ~~~~~~~~~~~~~~~~~~
   fs/ubifs/dir.c:379:2: note: Value stored to 'ui' is never read
           ui = ubifs_inode(inode);
           ^    ~~~~~~~~~~~~~~~~~~
   Suppressed 9 warnings (8 in non-user code, 1 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   8 warnings generated.
   Suppressed 8 warnings (8 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   9 warnings generated.
   fs/fs-writeback.c:148:3: warning: Argument to kfree() is the address of the local variable 'work', which is not memory allocated by malloc() [clang-analyzer-unix.Malloc]
                   kfree(work);
                   ^
   fs/fs-writeback.c:2691:6: note: Assuming the condition is false
           if (bdi == &noop_backing_dev_info)
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/fs-writeback.c:2691:2: note: Taking false branch
           if (bdi == &noop_backing_dev_info)
           ^
   fs/fs-writeback.c:2693:2: note: Taking false branch
           WARN_ON(!rwsem_is_locked(&sb->s_umount));
           ^
   include/asm-generic/bug.h:122:2: note: expanded from macro 'WARN_ON'
           if (unlikely(__ret_warn_on))                                    \
           ^
   fs/fs-writeback.c:2697:2: note: Calling 'bdi_split_work_to_wbs'
           bdi_split_work_to_wbs(bdi, &work, false);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/fs-writeback.c:1202:2: note: Loop condition is false.  Exiting loop
           might_sleep();
           ^
   include/linux/kernel.h:132:2: note: expanded from macro 'might_sleep'
           do { __might_sleep(__FILE__, __LINE__, 0); might_resched(); } while (0)
           ^
   fs/fs-writeback.c:1204:7: note: 'skip_if_busy' is false
           if (!skip_if_busy || !writeback_in_progress(&bdi->wb)) {
                ^~~~~~~~~~~~
   fs/fs-writeback.c:1204:20: note: Left side of '||' is true
           if (!skip_if_busy || !writeback_in_progress(&bdi->wb)) {
                             ^
   fs/fs-writeback.c:1206:3: note: Calling 'wb_queue_work'
                   wb_queue_work(&bdi->wb, base_work);
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/fs-writeback.c:163:6: note: Assuming field 'done' is null
           if (work->done)
               ^~~~~~~~~~
   fs/fs-writeback.c:163:2: note: Taking false branch
           if (work->done)
           ^
   fs/fs-writeback.c:168:6: note: Assuming the condition is false
           if (test_bit(WB_registered, &wb->state)) {
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/fs-writeback.c:168:2: note: Taking false branch
           if (test_bit(WB_registered, &wb->state)) {
           ^
   fs/fs-writeback.c:172:3: note: Calling 'finish_writeback_work'
                   finish_writeback_work(wb, work);
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/fs-writeback.c:147:6: note: Assuming field 'auto_free' is not equal to 0
           if (work->auto_free)
               ^~~~~~~~~~~~~~~
   fs/fs-writeback.c:147:2: note: Taking true branch
           if (work->auto_free)
           ^
   fs/fs-writeback.c:148:3: note: Argument to kfree() is the address of the local variable 'work', which is not memory allocated by malloc()
                   kfree(work);
                   ^     ~~~~
   Suppressed 8 warnings (8 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   6 warnings generated.
   Suppressed 6 warnings (6 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   7 warnings generated.
   fs/proc/generic.c:467:4: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
                           strcpy((char*)ent->data,dest);
                           ^~~~~~
   fs/proc/generic.c:467:4: note: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119
                           strcpy((char*)ent->data,dest);
                           ^~~~~~
   Suppressed 6 warnings (6 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   8 warnings generated.
   Suppressed 8 warnings (8 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   6 warnings generated.
   Suppressed 6 warnings (6 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   6 warnings generated.
   Suppressed 6 warnings (6 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   6 warnings generated.
   Suppressed 6 warnings (6 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   6 warnings generated.
   Suppressed 6 warnings (6 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   6 warnings generated.
   Suppressed 6 warnings (6 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.

vim +/ui +379 fs/ubifs/dir.c

1e51764a3c2ac0 Artem Bityutskiy 2008-07-14  351  
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  352  static struct inode *create_whiteout(struct inode *dir, struct dentry *dentry,
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  353  				     umode_t mode)
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  354  {
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  355  	int err;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  356  	struct inode *inode;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  357  	struct ubifs_inode *ui;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  358  	struct ubifs_info *c = dir->i_sb->s_fs_info;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  359  	struct fscrypt_name nm;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  360  
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  361  	/*
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  362  	 * Create an inode('nlink = 1') for whiteout without updating journal,
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  363  	 * let ubifs_jnl_rename() store it on flash to complete rename whiteout
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  364  	 * atomically.
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  365  	 */
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  366  
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  367  	dbg_gen("dent '%pd', mode %#hx in dir ino %lu",
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  368  		dentry, mode, dir->i_ino);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  369  
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  370  	err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  371  	if (err)
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  372  		return ERR_PTR(err);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  373  
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  374  	inode = ubifs_new_inode(c, dir, mode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  375  	if (IS_ERR(inode)) {
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  376  		err = PTR_ERR(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  377  		goto out_free;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  378  	}
8a6ca5bc1be93c Zhihao Cheng     2021-10-25 @379  	ui = ubifs_inode(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  380  
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  381  	init_special_inode(inode, inode->i_mode, WHITEOUT_DEV);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  382  	ubifs_assert(c, inode->i_op == &ubifs_file_inode_operations);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  383  
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  384  	err = ubifs_init_security(dir, inode, &dentry->d_name);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  385  	if (err)
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  386  		goto out_inode;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  387  
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  388  	/* The dir size is updated by do_rename. */
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  389  	insert_inode_hash(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  390  
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  391  	return inode;
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  392  
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  393  out_inode:
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  394  	make_bad_inode(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  395  	iput(inode);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  396  out_free:
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  397  	fscrypt_free_filename(&nm);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  398  	ubifs_err(c, "cannot create whiteout file, error %d", err);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  399  	return ERR_PTR(err);
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  400  }
8a6ca5bc1be93c Zhihao Cheng     2021-10-25  401  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 32366 bytes --]

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

end of thread, other threads:[~2021-11-05  5:37 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-25  3:41 [PATCH 00/11] Some bugfixs for ubifs/ubi Zhihao Cheng
2021-10-25  3:41 ` Zhihao Cheng
2021-10-25  3:41 ` [PATCH 01/11] ubifs: rename_whiteout: Fix double free for whiteout_ui->data Zhihao Cheng
2021-10-25  3:41   ` Zhihao Cheng
2021-10-25  3:41 ` [PATCH 02/11] ubifs: Fix deadlock in concurrent rename whiteout and inode writeback Zhihao Cheng
2021-10-25  3:41   ` Zhihao Cheng
2021-10-25  3:41 ` [PATCH 03/11] ubifs: Fix wrong number of inodes locked by ui_mutex in ubifs_inode comment Zhihao Cheng
2021-10-25  3:41   ` Zhihao Cheng
2021-10-25  3:41 ` [PATCH 04/11] ubifs: Add missing iput if do_tmpfile() failed in rename whiteout Zhihao Cheng
2021-10-25  3:41   ` Zhihao Cheng
2021-10-25  3:41 ` [PATCH 05/11] ubifs: Rename whiteout atomically Zhihao Cheng
2021-10-25  3:41   ` Zhihao Cheng
2021-10-25 14:57   ` kernel test robot
2021-10-25 14:57     ` kernel test robot
2021-10-25 14:57     ` kernel test robot
2021-10-29  5:33   ` kernel test robot
2021-10-29  5:33     ` kernel test robot
2021-10-29  5:33     ` kernel test robot
2021-10-25  3:41 ` [PATCH 06/11] ubifs: Fix 'ui->dirty' race between do_tmpfile() and writeback work Zhihao Cheng
2021-10-25  3:41   ` Zhihao Cheng
2021-10-25  3:41 ` [PATCH 07/11] ubifs: Rectify space amount budget for mkdir/tmpfile operations Zhihao Cheng
2021-10-25  3:41   ` Zhihao Cheng
2021-10-25  3:41 ` [PATCH 08/11] ubifs: setflags: Don't make a budget for 'ui->data_len' Zhihao Cheng
2021-10-25  3:41   ` Zhihao Cheng
2021-10-25  3:41 ` [PATCH 09/11] ubifs: Fix read out-of-bounds in ubifs_wbuf_write_nolock() Zhihao Cheng
2021-10-25  3:41   ` Zhihao Cheng
2021-10-25  3:41 ` [PATCH 10/11] ubi: fastmap: Return error code if memory allocation fails in add_aeb() Zhihao Cheng
2021-10-25  3:41   ` Zhihao Cheng
2021-10-25  3:41 ` [PATCH 11/11] ubi: fastmap: Add all fastmap pebs into 'ai->fastmap' when fm->used_blocks>=2 Zhihao Cheng
2021-10-25  3:41   ` Zhihao Cheng
2021-10-30 10:27 [PATCH 05/11] ubifs: Rename whiteout atomically kernel test robot
2021-11-05  5:35 ` kernel test robot
2021-11-05  5:35   ` kernel test robot
2021-11-05  5:35   ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.