On Fri, May 04, 2018 at 02:29:55PM -0500, Eric Sandeen wrote:
> On 4/17/18 9:46 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Validate the log space information in a similar manner to the kernel so
> > that repair will stumble over (and fix) broken log info that prevents
> > mounting. Fixes logsunit fuzz-and-fix failures in xfs/350.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > repair/sb.c | 24 +++++++++++++++++++++++-
> > 1 file changed, 23 insertions(+), 1 deletion(-)
> >
> >
> > diff --git a/repair/sb.c b/repair/sb.c
> > index 3dc6538..f2968cd 100644
> > --- a/repair/sb.c
> > +++ b/repair/sb.c
> > @@ -299,6 +299,27 @@ sb_validate_ino_align(struct xfs_sb *sb)
> > }
> >
> > /*
> > + * Validate the given log space. Derived from xfs_log_mount, though we
> > + * can't validate the minimum log size until later.
> > + * Returns false if the log is garbage.
> > + */
> > +static bool
> > +verify_sb_loginfo(
> > + struct xfs_sb *sb)
> > +{
> > + if (xfs_sb_version_hascrc(sb) &&
>
> Hm, this could use a comment about why these things only matter on v5 supers.
/*
* We only do this validation on v5 filesystems because the kernel
* didn't reject too-small logs in the past.
*/
?
>
> > + (sb->sb_logblocks == 0 ||
> > + sb->sb_logblocks > XFS_MAX_LOG_BLOCKS ||
> > + (sb->sb_logblocks << sb->sb_blocklog) > XFS_MAX_LOG_BYTES))
> > + return false;
> > +
> > + if (sb->sb_logsunit > 1 && sb->sb_logsunit % sb->sb_blocksize)
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > +/*
> > * verify a superblock -- does not verify root inode #
> > * can only check that geometry info is internally
> > * consistent. because of growfs, that's no guarantee
> > @@ -409,7 +430,8 @@ verify_sb(char *sb_buf, xfs_sb_t *sb, int is_primary_sb)
> > sb->sb_inodesize != (1 << sb->sb_inodelog) ||
> > sb->sb_logsunit > XLOG_MAX_RECORD_BSIZE ||
> > sb->sb_inopblock != howmany(sb->sb_blocksize, sb->sb_inodesize) ||
> > - (sb->sb_blocklog - sb->sb_inodelog != sb->sb_inopblog))
> > + (sb->sb_blocklog - sb->sb_inodelog != sb->sb_inopblog) ||
> > + !verify_sb_loginfo(sb))
> > return XR_BAD_INO_SIZE_DATA;
>
> do you really want to return XR_BAD_INO_SIZE_DATA for a bad logblocks count,
> which will yield
>
> "bad inode size or inconsistent with number of inodes/block")
>
> This might even want a new XR_BAD_LOG_FOO_BAR error...?
Sure.
"bad log geometry information in superblock" ?
--D
>
> -Eric
>
> >
> > if (xfs_sb_version_hassector(sb)) {
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
