On Tue, Mar 04, 2014 at 07:51:57PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Phase 1 of xfs_repair verifies and corrects the primary
> superblock of the filesystem. It does not verify that the CRC of the
> superblock that is found is correct, nor does it recalculate the CRC
> of the superblock it rewrites.
>
> This happens because phase1 does not use the libxfs buffer cache -
> it just uses pread/pwrite on a memory buffer, and works directly
> from that buffer. Hence we need to add CRC verification to
> verify_sb(), and CRC recalculation to write_primary_sb() so that it
> works correctly.
>
> This also enables us to use get_sb() as the method of fetching the
> superblock from disk after phase 1 without needing to use the libxfs
> buffer cache and guessing at the sector size. This prevents a
> verifier error because it attempts to CRC a superblock buffer that
> is much longer than the usual sector sizes.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> repair/agheader.c | 2 +-
> repair/globals.h | 3 ++-
> repair/phase1.c | 5 ++--
> repair/protos.h | 3 ++-
> repair/sb.c | 71 +++++++++++++++++++++++++++++------------------------
> repair/xfs_repair.c | 26 +++++++++++---------
> 6 files changed, 62 insertions(+), 48 deletions(-)
>
> diff --git a/repair/agheader.c b/repair/agheader.c
> index 53e47b6..fc5dac9 100644
> --- a/repair/agheader.c
> +++ b/repair/agheader.c
> @@ -472,7 +472,7 @@ verify_set_agheader(xfs_mount_t *mp, xfs_buf_t *sbuf, xfs_sb_t *sb,
> int status = XR_OK;
> int status_sb = XR_OK;
>
> - status = verify_sb(sb, (i == 0));
> + status = verify_sb(sbuf->b_addr, sb, (i == 0));
>
> if (status != XR_OK) {
> do_warn(_("bad on-disk superblock %d - %s\n"),
> diff --git a/repair/globals.h b/repair/globals.h
> index cbb2ce7..f6e0a22 100644
> --- a/repair/globals.h
> +++ b/repair/globals.h
> @@ -49,7 +49,8 @@
> #define XR_BAD_SB_UNIT 17 /* bad stripe unit */
> #define XR_BAD_SB_WIDTH 18 /* bad stripe width */
> #define XR_BAD_SVN 19 /* bad shared version number */
> -#define XR_BAD_ERR_CODE 20 /* Bad error code */
> +#define XR_BAD_CRC 20 /* Bad CRC */
> +#define XR_BAD_ERR_CODE 21 /* Bad error code */
>
> /* XFS filesystem (il)legal values */
>
> diff --git a/repair/phase1.c b/repair/phase1.c
> index 62de211..ec75ada 100644
> --- a/repair/phase1.c
> +++ b/repair/phase1.c
> @@ -70,13 +70,14 @@ phase1(xfs_mount_t *mp)
> ag_bp = alloc_ag_buf(MAX_SECTSIZE);
> sb = (xfs_sb_t *) ag_bp;
>
> - if (get_sb(sb, 0LL, MAX_SECTSIZE, 0) == XR_EOF)
> + rval = get_sb(sb, 0LL, MAX_SECTSIZE, 0);
> + if (rval == XR_EOF)
> do_error(_("error reading primary superblock\n"));
>
> /*
> * is this really an sb, verify internal consistency
> */
This comment can probably go away now. Otherwise, looks good...
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> - if ((rval = verify_sb(sb, 1)) != XR_OK) {
> + if (rval != XR_OK) {
> do_warn(_("bad primary superblock - %s !!!\n"),
> err_string(rval));
> if (!find_secondary_sb(sb))
> diff --git a/repair/protos.h b/repair/protos.h
> index 601f2a9..ff42fa7 100644
> --- a/repair/protos.h
> +++ b/repair/protos.h
> @@ -18,7 +18,8 @@
>
> void xfs_init(libxfs_init_t *args);
>
> -int verify_sb(xfs_sb_t *sb,
> +int verify_sb(char *sb_buf,
> + xfs_sb_t *sb,
> int is_primary_sb);
> int verify_set_primary_sb(xfs_sb_t *root_sb,
> int sb_index,
> diff --git a/repair/sb.c b/repair/sb.c
> index c54d89b..b111aca 100644
> --- a/repair/sb.c
> +++ b/repair/sb.c
> @@ -139,7 +139,7 @@ find_secondary_sb(xfs_sb_t *rsb)
> c_bufsb = (char *)sb + i;
> libxfs_sb_from_disk(&bufsb, (xfs_dsb_t *)c_bufsb);
>
> - if (verify_sb(&bufsb, 0) != XR_OK)
> + if (verify_sb(c_bufsb, &bufsb, 0) != XR_OK)
> continue;
>
> do_warn(_("found candidate secondary superblock...\n"));
> @@ -245,7 +245,7 @@ sb_validate_ino_align(struct xfs_sb *sb)
> */
>
> int
> -verify_sb(xfs_sb_t *sb, int is_primary_sb)
> +verify_sb(char *sb_buf, xfs_sb_t *sb, int is_primary_sb)
> {
> __uint32_t bsize;
> int i;
> @@ -263,8 +263,34 @@ verify_sb(xfs_sb_t *sb, int is_primary_sb)
> if (is_primary_sb && sb->sb_inprogress == 1)
> return(XR_BAD_INPROGRESS);
>
> - /* check to make sure blocksize is legal 2^N, 9 <= N <= 16 */
> + /*
> + * before going *any further*, validate the sector size and if the
> + * version says we should have CRCs enabled, validate that.
> + */
> +
> + /* check to make sure sectorsize is legal 2^N, 9 <= N <= 15 */
> + if (sb->sb_sectsize == 0)
> + return(XR_BAD_SECT_SIZE_DATA);
> +
> + bsize = 1;
> + for (i = 0; bsize < sb->sb_sectsize &&
> + i < sizeof(sb->sb_sectsize) * NBBY; i++) {
> + bsize <<= 1;
> + }
> +
> + if (i < XFS_MIN_SECTORSIZE_LOG || i > XFS_MAX_SECTORSIZE_LOG)
> + return(XR_BAD_SECT_SIZE_DATA);
> +
> + /* check sb sectorsize field against sb sectlog field */
> + if (i != sb->sb_sectlog)
> + return(XR_BAD_SECT_SIZE_DATA);
> +
> + /* sector size in range - CRC check time */
> + if (xfs_sb_version_hascrc(sb) &&
> + !xfs_verify_cksum(sb_buf, sb->sb_sectsize, XFS_SB_CRC_OFF))
> + return XR_BAD_CRC;
>
> + /* check to make sure blocksize is legal 2^N, 9 <= N <= 16 */
> if (sb->sb_blocksize == 0)
> return(XR_BAD_BLOCKSIZE);
>
> @@ -300,26 +326,6 @@ verify_sb(xfs_sb_t *sb, int is_primary_sb)
> sb->sb_inopblock != howmany(sb->sb_blocksize,sb->sb_inodesize))
> return(XR_BAD_INO_SIZE_DATA);
>
> - /* check to make sure sectorsize is legal 2^N, 9 <= N <= 15 */
> -
> - if (sb->sb_sectsize == 0)
> - return(XR_BAD_SECT_SIZE_DATA);
> -
> - bsize = 1;
> -
> - for (i = 0; bsize < sb->sb_sectsize &&
> - i < sizeof(sb->sb_sectsize) * NBBY; i++) {
> - bsize <<= 1;
> - }
> -
> - if (i < XFS_MIN_SECTORSIZE_LOG || i > XFS_MAX_SECTORSIZE_LOG)
> - return(XR_BAD_SECT_SIZE_DATA);
> -
> - /* check sb sectorsize field against sb sectlog field */
> -
> - if (i != sb->sb_sectlog)
> - return(XR_BAD_SECT_SIZE_DATA);
> -
> if (xfs_sb_version_hassector(sb)) {
>
> /* check to make sure log sector is legal 2^N, 9 <= N <= 15 */
> @@ -482,9 +488,11 @@ write_primary_sb(xfs_sb_t *sbp, int size)
> do_error(_("couldn't seek to offset 0 in filesystem\n"));
> }
>
> -
> libxfs_sb_to_disk(buf, sbp, XFS_SB_ALL_BITS);
>
> + if (xfs_sb_version_hascrc(sbp))
> + xfs_update_cksum((char *)buf, size, XFS_SB_CRC_OFF);
> +
> if (write(x.dfd, buf, size) != size) {
> free(buf);
> do_error(_("primary superblock write failed!\n"));
> @@ -494,7 +502,7 @@ write_primary_sb(xfs_sb_t *sbp, int size)
> }
>
> /*
> - * get a possible superblock -- don't check for internal consistency
> + * get a possible superblock -- checks for internal consistency
> */
> int
> get_sb(xfs_sb_t *sbp, xfs_off_t off, int size, xfs_agnumber_t agno)
> @@ -529,9 +537,10 @@ get_sb(xfs_sb_t *sbp, xfs_off_t off, int size, xfs_agnumber_t agno)
> do_error("%s\n", strerror(error));
> }
> libxfs_sb_from_disk(sbp, buf);
> - free(buf);
>
> - return (verify_sb(sbp, 0));
> + rval = verify_sb((char *)buf, sbp, agno == 0);
> + free(buf);
> + return rval;
> }
>
> /* returns element on list with highest reference count */
> @@ -745,13 +754,11 @@ verify_set_primary_sb(xfs_sb_t *rsb,
> off = (xfs_off_t)agno * rsb->sb_agblocks << rsb->sb_blocklog;
>
> checked[agno] = 1;
> -
> - if (get_sb(sb, off, size, agno) == XR_EOF) {
> - retval = XR_EOF;
> + retval = get_sb(sb, off, size, agno);
> + if (retval == XR_EOF)
> goto out_free_list;
> - }
>
> - if (verify_sb(sb, 0) == XR_OK) {
> + if (retval == XR_OK) {
> /*
> * save away geometry info.
> * don't bother checking the sb
> diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
> index 6327076..08b25f0 100644
> --- a/repair/xfs_repair.c
> +++ b/repair/xfs_repair.c
> @@ -137,6 +137,8 @@ err_string(int err_code)
> _("bad stripe width in superblock");
> err_message[XR_BAD_SVN] =
> _("bad shared version number in superblock");
> + err_message[XR_BAD_CRC] =
> + _("bad CRC in superblock");
> done = 1;
> }
>
> @@ -529,6 +531,8 @@ main(int argc, char **argv)
> xfs_buf_t *sbp;
> xfs_mount_t xfs_m;
> char *msgbuf;
> + struct xfs_sb psb;
> + int rval;
>
> progname = basename(argv[0]);
> setlocale(LC_ALL, "");
> @@ -558,13 +562,12 @@ main(int argc, char **argv)
> exit(1);
> }
>
> - /* prepare the mount structure */
> - memset(&xfs_m, 0, sizeof(xfs_mount_t));
> - libxfs_buftarg_init(&xfs_m, x.ddev, x.logdev, x.rtdev);
> - sbp = libxfs_readbuf(xfs_m.m_ddev_targp, XFS_SB_DADDR,
> - 1 << (XFS_MAX_SECTORSIZE_LOG - BBSHIFT), 0,
> - &xfs_sb_buf_ops);
> - libxfs_sb_from_disk(&xfs_m.m_sb, XFS_BUF_TO_SBP(sbp));
> + rval = get_sb(&psb, 0, XFS_MAX_SECTORSIZE, 0);
> + if (rval != XR_OK) {
> + do_warn(_("Primary superblock bad after phase 1!\n"
> + "Exiting now.\n"));
> + exit(1);
> + }
>
> /*
> * if the sector size of the filesystem we are trying to repair is
> @@ -583,7 +586,7 @@ main(int argc, char **argv)
> geom.sectsize = BBSIZE;
> }
>
> - if (xfs_m.m_sb.sb_sectsize < geom.sectsize) {
> + if (psb.sb_sectsize < geom.sectsize) {
> long old_flags;
>
> old_flags = fcntl(fd, F_GETFL, 0);
> @@ -595,7 +598,10 @@ main(int argc, char **argv)
> }
> }
> }
> - mp = libxfs_mount(&xfs_m, &xfs_m.m_sb, x.ddev, x.logdev, x.rtdev, 0);
> +
> + /* prepare the mount structure */
> + memset(&xfs_m, 0, sizeof(xfs_mount_t));
> + mp = libxfs_mount(&xfs_m, &psb, x.ddev, x.logdev, x.rtdev, 0);
>
> if (!mp) {
> fprintf(stderr,
> @@ -603,8 +609,6 @@ main(int argc, char **argv)
> progname);
> exit(1);
> }
> - libxfs_putbuf(sbp);
> - libxfs_purgebuf(sbp);
>
> /*
> * set XFS-independent status vars from the mount/sb structure
> --
> 1.9.0
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
