On 20.02.2018 23:11, Dave Chinner wrote:

On Tue, Feb 20, 2018 at 03:53:02PM +0200, Nikolay Borisov wrote:

quoted

The check performed before the memcpy responsible for copying the rest
of the inode is already performed before we call xfs_log_dinode_to_disk.
So let's remove the 2nd instance of the check. No functional changes.

Signed-off-by: Nikolay Borisov <redacted>
Reported-by: Jeff Mahoney <redacted>
---
 fs/xfs/xfs_log_recover.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 00240c9ee72e..88dccfb1de96 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c

@@ -3174,11 +3174,8 @@ xlog_recover_inode_pass2(
 	xfs_log_dinode_to_disk(ldip, dip);
 
 	/* the rest is in on-disk format */
-	if (item->ri_buf[1].i_len > isize) {
-		memcpy((char *)dip + isize,
-			item->ri_buf[1].i_addr + isize,
+	memcpy((char *)dip + isize, item->ri_buf[1].i_addr + isize,
 			item->ri_buf[1].i_len - isize);
-	}
 

This looks wrong.

The previous check is:

	if (unlikely(item->ri_buf[1].i_len > isize)) {
		CORRUPTION_ERROR
		....
		error = -EFSCORRUPTED;
		goto out_release;
	}

So after this item->ri_buf[1].i_len is always <= isize. IOWs, the
memcpy() is currently dead code that is never executed, not code we
want to execute in every inode recovery.

Doh, you are right, will resend.

Cheers,

Dave.