On 7/28/21 2:15 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> If we encounter a directory that has been configured to pass on an
> extent size hint to a new realtime file and the hint isn't an integer
> multiple of the rt extent size, we should turn off the hint because that
> is a misconfiguration. Old kernels didn't check for this when copying
> attributes into new files and would crash in the rt allocator.
>
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
This looks reasonable to me, it's 90% refactoring and 10% new test.
My only concern is that a failed validation of the extent size /hint/ still
says ""Bad extent size %u on inode" but I'm not sure that's terribly important
to change ... so unless you want to -
Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
> repair/dinode.c | 71 ++++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 51 insertions(+), 20 deletions(-)
>
>
> diff --git a/repair/dinode.c b/repair/dinode.c
> index 291c5807..09e42ad2 100644
> --- a/repair/dinode.c
> +++ b/repair/dinode.c
> @@ -2179,6 +2179,56 @@ _("Bad %s nsec %u on inode %" PRIu64 ", "), name, be32_to_cpu(t->t_nsec), lino);
> }
> }
>
> +static void
> +validate_extsize(
> + struct xfs_mount *mp,
> + struct xfs_dinode *dino,
> + xfs_ino_t lino,
> + int *dirty)
> +{
> + uint16_t flags = be16_to_cpu(dino->di_flags);
> + unsigned int value = be32_to_cpu(dino->di_extsize);
> + bool misaligned = false;
> + bool bad;
> +
> + /*
> + * XFS allows a sysadmin to change the rt extent size when adding a rt
> + * section to a filesystem after formatting. If there are any
> + * directories with extszinherit and rtinherit set, the hint could
> + * become misaligned with the new rextsize. The verifier doesn't check
> + * this, because we allow rtinherit directories even without an rt
> + * device.
> + */
> + if ((flags & XFS_DIFLAG_EXTSZINHERIT) &&
> + (flags & XFS_DIFLAG_RTINHERIT) &&
> + value % mp->m_sb.sb_rextsize > 0)
> + misaligned = true;
> +
> + /*
> + * Complain if the verifier fails.
> + *
> + * Old kernels didn't check the alignment of extsize hints when copying
> + * them to new regular realtime files. The inode verifier now checks
> + * the alignment (because misaligned hints cause misbehavior in the rt
> + * allocator), so we have to complain and fix them.
> + */
> + bad = libxfs_inode_validate_extsize(mp, value,
> + be16_to_cpu(dino->di_mode), flags) != NULL;
> + if (bad || misaligned) {
> + do_warn(
> +_("Bad extent size %u on inode %" PRIu64 ", "),
> + value, lino);
> + if (!no_modify) {
> + do_warn(_("resetting to zero\n"));
> + dino->di_extsize = 0;
> + dino->di_flags &= ~cpu_to_be16(XFS_DIFLAG_EXTSIZE |
> + XFS_DIFLAG_EXTSZINHERIT);
> + *dirty = 1;
> + } else
> + do_warn(_("would reset to zero\n"));
> + }
> +}
> +
> /*
> * returns 0 if the inode is ok, 1 if the inode is corrupt
> * check_dups can be set to 1 *only* when called by the
> @@ -2690,26 +2740,7 @@ _("bad (negative) size %" PRId64 " on inode %" PRIu64 "\n"),
> if (process_check_sb_inodes(mp, dino, lino, &type, dirty) != 0)
> goto clear_bad_out;
>
> - /*
> - * only regular files with REALTIME or EXTSIZE flags set can have
> - * extsize set, or directories with EXTSZINHERIT.
> - */
> - if (libxfs_inode_validate_extsize(mp,
> - be32_to_cpu(dino->di_extsize),
> - be16_to_cpu(dino->di_mode),
> - be16_to_cpu(dino->di_flags)) != NULL) {
> - do_warn(
> -_("Bad extent size %u on inode %" PRIu64 ", "),
> - be32_to_cpu(dino->di_extsize), lino);
> - if (!no_modify) {
> - do_warn(_("resetting to zero\n"));
> - dino->di_extsize = 0;
> - dino->di_flags &= ~cpu_to_be16(XFS_DIFLAG_EXTSIZE |
> - XFS_DIFLAG_EXTSZINHERIT);
> - *dirty = 1;
> - } else
> - do_warn(_("would reset to zero\n"));
> - }
> + validate_extsize(mp, dino, lino, dirty);
>
> /*
> * Only (regular files and directories) with COWEXTSIZE flags
>
