]> git.ipfire.org Git - thirdparty/xfsprogs-dev.git/commitdiff
metadump: Only verify obfuscated metadata being dumped
authorDave Chinner <dchinner@redhat.com>
Mon, 3 Mar 2014 01:29:32 +0000 (12:29 +1100)
committerDave Chinner <david@fromorbit.com>
Mon, 3 Mar 2014 01:29:32 +0000 (12:29 +1100)
The discontiguous buffer support series added a verifier check on
the metadata buffers before they go written to the metadump image.
If this failed, it returned an error, and the result would be that
we stopped processing the metadata and exited, truncating the dump.

xfs_metadump is supposed to dump the metadata in the filesystem for
forensic analysis purposes, which means we actually want it to
retain any corruptions it finds in the filesystem. Hence running the
verifier - even to recalculate CRCs - when the metadata is
unmodified is the wrong thing to be doing. And stopping the dump
when we come across an error is even worse.

We still need to do CRC recalculation when obfuscating names and
attributes. Hence we need to make running the verifier conditional
on the buffer or inode:
a) being uncorrupted when read, and
b) modified by the obfuscation code.

If either of these conditions is not true, then we don't run the
verifier or recalculate the CRCs.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Dave Chinner <david@fromorbit.com>
db/io.h
db/metadump.c

diff --git a/db/io.h b/db/io.h
index 4f24c83b8673dab0414181bcfeb7d3ff3452efd8..d8cf3831e1cd57e5ad7921f794a4ae8b8e9aa2a6 100644 (file)
--- a/db/io.h
+++ b/db/io.h
@@ -41,6 +41,7 @@ typedef struct iocur {
        int                     ino_crc_ok:1;
        int                     ino_buf:1;
        int                     dquot_buf:1;
+       int                     need_crc:1;
 } iocur_t;
 
 #define DB_RING_ADD 1                   /* add to ring on set_cur */
index 14902a727305d0bc7f2530cf7320d1672c6ca813..324800942a209b7de782a546c28b4e8cdfd71154 100644 (file)
@@ -190,26 +190,36 @@ write_buf_segment(
        return 0;
 }
 
+/*
+ * we want to preserve the state of the metadata in the dump - whether it is
+ * intact or corrupt, so even if the buffer has a verifier attached to it we
+ * don't want to run it prior to writing the buffer to the metadump image.
+ *
+ * The only reason for running the verifier is to recalculate the CRCs on a
+ * buffer that has been obfuscated. i.e. a buffer than metadump modified itself.
+ * In this case, we only run the verifier if the buffer was not corrupt to begin
+ * with so that we don't accidentally correct buffers with CRC or errors in them
+ * when we are obfuscating them.
+ */
 static int
 write_buf(
        iocur_t         *buf)
 {
+       struct xfs_buf  *bp = buf->bp;
        int             i;
        int             ret;
 
        /*
         * Run the write verifier to recalculate the buffer CRCs and check
-        * we are writing something valid to disk
+        * metadump didn't introduce a new corruption. Warn if the verifier
+        * failed, but still continue to dump it into the output file.
         */
-       if (buf->bp && buf->bp->b_ops) {
-               buf->bp->b_error = 0;
-               buf->bp->b_ops->verify_write(buf->bp);
-               if (buf->bp->b_error) {
-                       fprintf(stderr,
-       _("%s: write verifer failed on bno 0x%llx/0x%x\n"),
-                               __func__, (long long)buf->bp->b_bn,
-                               buf->bp->b_bcount);
-                       return -buf->bp->b_error;
+       if (buf->need_crc && bp && bp->b_ops && !bp->b_error) {
+               bp->b_ops->verify_write(bp);
+               if (bp->b_error) {
+                       print_warning(
+                               "obfuscation corrupted block at bno 0x%llx/0x%x",
+                               (long long)bp->b_bn, bp->b_bcount);
                }
        }
 
@@ -1359,12 +1369,15 @@ process_single_fsb_objects(
 
                        obfuscate_dir_data_block(dp, o,
                                                 last == mp->m_dirblkfsbs);
+                       iocur_top->need_crc = 1;
                        break;
                case TYP_SYMLINK:
                        obfuscate_symlink_block(dp);
+                       iocur_top->need_crc = 1;
                        break;
                case TYP_ATTR:
                        obfuscate_attr_block(dp, o);
+                       iocur_top->need_crc = 1;
                        break;
                default:
                        break;
@@ -1444,6 +1457,7 @@ process_multi_fsb_objects(
 
                        obfuscate_dir_data_block(iocur_top->data, o,
                                                  last == mp->m_dirblkfsbs);
+                       iocur_top->need_crc = 1;
                        ret = write_buf(iocur_top);
 out_pop:
                        pop_cur();
@@ -1724,6 +1738,13 @@ process_inode_data(
        return 1;
 }
 
+/*
+ * when we process the inode, we may change the data in the data and/or
+ * attribute fork if they are in short form and we are obfuscating names.
+ * In this case we need to recalculate the CRC of the inode, but we should
+ * only do that if the CRC in the inode is good to begin with. If the crc
+ * is not ok, we just leave it alone.
+ */
 static int
 process_inode(
        xfs_agnumber_t          agno,
@@ -1731,17 +1752,30 @@ process_inode(
        xfs_dinode_t            *dip)
 {
        int                     success;
+       bool                    crc_was_ok = false; /* no recalc by default */
+       bool                    need_new_crc = false;
 
        success = 1;
        cur_ino = XFS_AGINO_TO_INO(mp, agno, agino);
 
+       /* we only care about crc recalculation if we are obfuscating names. */
+       if (!dont_obfuscate) {
+               crc_was_ok = xfs_verify_cksum((char *)dip,
+                                       mp->m_sb.sb_inodesize,
+                                       offsetof(struct xfs_dinode, di_crc));
+       }
+
        /* copy appropriate data fork metadata */
        switch (be16_to_cpu(dip->di_mode) & S_IFMT) {
                case S_IFDIR:
                        success = process_inode_data(dip, TYP_DIR2);
+                       if (dip->di_format == XFS_DINODE_FMT_LOCAL)
+                               need_new_crc = 1;
                        break;
                case S_IFLNK:
                        success = process_inode_data(dip, TYP_SYMLINK);
+                       if (dip->di_format == XFS_DINODE_FMT_LOCAL)
+                               need_new_crc = 1;
                        break;
                case S_IFREG:
                        success = process_inode_data(dip, TYP_DATA);
@@ -1756,6 +1790,7 @@ process_inode(
                attr_data.remote_val_count = 0;
                switch (dip->di_aformat) {
                        case XFS_DINODE_FMT_LOCAL:
+                               need_new_crc = 1;
                                if (!dont_obfuscate)
                                        obfuscate_sf_attr(dip);
                                break;
@@ -1770,6 +1805,9 @@ process_inode(
                }
                nametable_clear();
        }
+
+       if (crc_was_ok && need_new_crc)
+               xfs_dinode_calc_crc(mp, dip);
        return success;
 }
 
@@ -1840,9 +1878,6 @@ copy_inode_chunk(
 
                if (!process_inode(agno, agino + i, dip))
                        goto pop_out;
-
-               /* calculate the new CRC for the inode */
-               xfs_dinode_calc_crc(mp, dip);
        }
 skip_processing:
        if (write_buf(iocur_top))