On Fri, Nov 10, 2023 at 03:33:14PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Because on v3 inodes, di_flushiter doesn't exist. It overlaps with
> zero padding in the inode, except when NREXT64=1 configurations are
> in use and the zero padding is no longer padding but holds the 64
> bit extent counter.
>
> This manifests obviously on big endian platforms (e.g. s390) because
> the log dinode is in host order and the overlap is the LSBs of the
> extent count field. It is not noticed on little endian machines
> because the overlap is at the MSB end of the extent count field and
> we need to get more than 2^^48 extents in the inode before it
> manifests. i.e. the heat death of the universe will occur before we
> see the problem in little endian machines.
>
> This is a zero-day issue for NREXT64=1 configuraitons on big endian
> machines. Fix it by only clearing di_flushiter on v2 inodes during
> recovery.
>
> Fixes: 9b7d16e34bbe ("xfs: Introduce XFS_DIFLAG2_NREXT64 and associated helpers")
> cc: stable@xxxxxxxxxx # 5.19+
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> fs/xfs/xfs_inode_item_recover.c | 32 +++++++++++++++++---------------
> 1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode_item_recover.c b/fs/xfs/xfs_inode_item_recover.c
> index f4c31c2b60d5..dbdab4ce7c44 100644
> --- a/fs/xfs/xfs_inode_item_recover.c
> +++ b/fs/xfs/xfs_inode_item_recover.c
> @@ -371,24 +371,26 @@ xlog_recover_inode_commit_pass2(
> * superblock flag to determine whether we need to look at di_flushiter
> * to skip replay when the on disk inode is newer than the log one
> */
> - if (!xfs_has_v3inodes(mp) &&
> - ldip->di_flushiter < be16_to_cpu(dip->di_flushiter)) {
> - /*
> - * Deal with the wrap case, DI_MAX_FLUSH is less
> - * than smaller numbers
> - */
> - if (be16_to_cpu(dip->di_flushiter) == DI_MAX_FLUSH &&
> - ldip->di_flushiter < (DI_MAX_FLUSH >> 1)) {
> - /* do nothing */
> - } else {
> - trace_xfs_log_recover_inode_skip(log, in_f);
> - error = 0;
> - goto out_release;
> + if (!xfs_has_v3inodes(mp)) {
> + if (ldip->di_flushiter < be16_to_cpu(dip->di_flushiter)) {
> + /*
> + * Deal with the wrap case, DI_MAX_FLUSH is less
> + * than smaller numbers
> + */
> + if (be16_to_cpu(dip->di_flushiter) == DI_MAX_FLUSH &&
> + ldip->di_flushiter < (DI_MAX_FLUSH >> 1)) {
> + /* do nothing */
> + } else {
> + trace_xfs_log_recover_inode_skip(log, in_f);
> + error = 0;
> + goto out_release;
> + }
> }
> +
> + /* Take the opportunity to reset the flush iteration count */
> + ldip->di_flushiter = 0;
Hmm. Well this fixes the zeroday problem, so thank you for getting the
root of this!
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
Though hch did suggest reducing the amount of indenting here by
compressing the if tests together. I can't decide if it's worth
rearranging that old V4 code since none of it's scheduled for removal
until 2030, but it /is/ legacy code that maybe we just don't care to
touch?
<shrug>
--D
> }
>
> - /* Take the opportunity to reset the flush iteration count */
> - ldip->di_flushiter = 0;
>
> if (unlikely(S_ISREG(ldip->di_mode))) {
> if ((ldip->di_format != XFS_DINODE_FMT_EXTENTS) &&
> --
> 2.42.0
>
