From: Darrick J. Wong <djwong@xxxxxxxxxx> Per some very late review comments, capture the generation numbers of both inodes involved in a file content exchange operation so that we don't accidentally target files with have been reallocated. Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx> --- v30.2.1: make a generic helper for later use, and add more comments about why we're doing this in the first place --- fs/xfs/libxfs/xfs_log_format.h | 2 ++ fs/xfs/libxfs/xfs_log_recover.h | 2 ++ fs/xfs/xfs_exchmaps_item.c | 25 ++++++++++++++++++++----- fs/xfs/xfs_log_recover.c | 31 +++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 5 deletions(-) diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h index 8dbe1f997dfd5..accba2acd623d 100644 --- a/fs/xfs/libxfs/xfs_log_format.h +++ b/fs/xfs/libxfs/xfs_log_format.h @@ -896,6 +896,8 @@ struct xfs_xmi_log_format { uint64_t xmi_inode1; /* inumber of first file */ uint64_t xmi_inode2; /* inumber of second file */ + uint32_t xmi_igen1; /* generation of first file */ + uint32_t xmi_igen2; /* generation of second file */ uint64_t xmi_startoff1; /* block offset into file1 */ uint64_t xmi_startoff2; /* block offset into file2 */ uint64_t xmi_blockcount; /* number of blocks */ diff --git a/fs/xfs/libxfs/xfs_log_recover.h b/fs/xfs/libxfs/xfs_log_recover.h index 47b758b49cb35..521d327e4c89e 100644 --- a/fs/xfs/libxfs/xfs_log_recover.h +++ b/fs/xfs/libxfs/xfs_log_recover.h @@ -123,6 +123,8 @@ bool xlog_is_buffer_cancelled(struct xlog *log, xfs_daddr_t blkno, uint len); int xlog_recover_iget(struct xfs_mount *mp, xfs_ino_t ino, struct xfs_inode **ipp); +int xlog_recover_iget_handle(struct xfs_mount *mp, xfs_ino_t ino, uint32_t gen, + struct xfs_inode **ipp); void xlog_recover_release_intent(struct xlog *log, unsigned short intent_type, uint64_t intent_id); int xlog_alloc_buf_cancel_table(struct xlog *log); diff --git a/fs/xfs/xfs_exchmaps_item.c b/fs/xfs/xfs_exchmaps_item.c index a40216f33214c..264a121c5e16d 100644 --- a/fs/xfs/xfs_exchmaps_item.c +++ b/fs/xfs/xfs_exchmaps_item.c @@ -231,7 +231,9 @@ xfs_exchmaps_create_intent( xlf = &xmi_lip->xmi_format; xlf->xmi_inode1 = xmi->xmi_ip1->i_ino; + xlf->xmi_igen1 = VFS_I(xmi->xmi_ip1)->i_generation; xlf->xmi_inode2 = xmi->xmi_ip2->i_ino; + xlf->xmi_igen2 = VFS_I(xmi->xmi_ip2)->i_generation; xlf->xmi_startoff1 = xmi->xmi_startoff1; xlf->xmi_startoff2 = xmi->xmi_startoff2; xlf->xmi_blockcount = xmi->xmi_blockcount; @@ -368,14 +370,25 @@ xfs_xmi_item_recover_intent( /* * Grab both inodes and set IRECOVERY to prevent trimming of post-eof * mappings and freeing of unlinked inodes until we're totally done - * processing files. + * processing files. The ondisk format of this new log item contains + * file handle information, which is why recovery for other items do + * not check the inode generation number. */ - error = xlog_recover_iget(mp, xlf->xmi_inode1, &ip1); - if (error) + error = xlog_recover_iget_handle(mp, xlf->xmi_inode1, xlf->xmi_igen1, + &ip1); + if (error) { + XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, xlf, + sizeof(*xlf)); return ERR_PTR(error); - error = xlog_recover_iget(mp, xlf->xmi_inode2, &ip2); - if (error) + } + + error = xlog_recover_iget_handle(mp, xlf->xmi_inode2, xlf->xmi_igen2, + &ip2); + if (error) { + XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, xlf, + sizeof(*xlf)); goto err_rele1; + } req->ip1 = ip1; req->ip2 = ip2; @@ -485,6 +498,8 @@ xfs_exchmaps_relog_intent( new_xlf->xmi_inode1 = old_xlf->xmi_inode1; new_xlf->xmi_inode2 = old_xlf->xmi_inode2; + new_xlf->xmi_igen1 = old_xlf->xmi_igen1; + new_xlf->xmi_igen2 = old_xlf->xmi_igen2; new_xlf->xmi_startoff1 = old_xlf->xmi_startoff1; new_xlf->xmi_startoff2 = old_xlf->xmi_startoff2; new_xlf->xmi_blockcount = old_xlf->xmi_blockcount; diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index 1e5ba95adf2c7..b445e8ce4a7d2 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -1767,6 +1767,37 @@ xlog_recover_iget( return 0; } +/* + * Get an inode so that we can recover a log operation. + * + * Log intent items that target inodes effectively contain a file handle. + * Check that the generation number matches the intent item like we do for + * other file handles. Log intent items defined after this validation weakness + * was identified must use this function. + */ +int +xlog_recover_iget_handle( + struct xfs_mount *mp, + xfs_ino_t ino, + uint32_t gen, + struct xfs_inode **ipp) +{ + struct xfs_inode *ip; + int error; + + error = xlog_recover_iget(mp, ino, &ip); + if (error) + return error; + + if (VFS_I(ip)->i_generation != gen) { + xfs_irele(ip); + return -EFSCORRUPTED; + } + + *ipp = ip; + return 0; +} + /****************************************************************************** * * Log recover routines