xfs
[Top] [All Lists]

Re: [PATCH] xfsdump: enable dump header checksums

To: Gim Leong Chin <chingimleong@xxxxxxxxxxxx>
Subject: Re: [PATCH] xfsdump: enable dump header checksums
From: Bill Kendall <wkendall@xxxxxxx>
Date: Fri, 02 Sep 2011 10:02:09 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1314975119.93913.YahooMailClassic@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
References: <1314975119.93913.YahooMailClassic@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.18) Gecko/20110617 Thunderbird/3.1.11
On 09/02/2011 09:51 AM, Gim Leong Chin wrote:
Hi Bill,


May I understand:

1) Are the check sums written in the dump file?
2) Which fields would have the check sums?

The headers that precede directory entries, files (inodes),
extents (user data) and extended attributes all have checksum
fields. They are stored in the dump stream and the stream has
flags indicating whether or not the checksums were written so
that restore knows if it should check them.

3) If the checksums at restore time do not agree with the fields, is there any 
kind of recovery action possible?

Possibly, though that's not part of this patch. For example if
a file header was corrupt, it may be possible to skip forward
until the next valid file header is found and continue the
restore from there.

Thanks,
Bill


Thanks!


GL

--- On Tue, 30/8/11, Bill Kendall<wkendall@xxxxxxx>  wrote:

From: Bill Kendall<wkendall@xxxxxxx>
Subject: [PATCH] xfsdump: enable dump header checksums
To: xfs@xxxxxxxxxxx
Date: Tuesday, 30 August, 2011, 5:41 AM
Various structures in a dump file
optionally contain a checksum, but
the code to compute and validate the checksum has not been
enabled.
The checksum code has a negligible performance impact and
so this
patch enables the checksum code unconditionally. Also:

- make sure all header sizes are multiples of 4 bytes
   (a requirement of the checksum routine)
- zero structures to ensure internal padding has a known
value
- fix a bug in dump_extattr_buildrecord() which
checksummed
   the wrong header structure
- add calc_checksum() and is_checksum_valid() routines to
   cut down on duplicate code

Signed-off-by: Bill Kendall<wkendall@xxxxxxx>
---
  common/content_inode.h |   20 +++++++++++
  dump/content.c
    |   85
+++++++++++-------------------------------------
  restore/Makefile       |
   2 +-
  restore/content.c
|   40 ++--------------------
  4 files changed, 44 insertions(+), 103 deletions(-)

diff --git a/common/content_inode.h
b/common/content_inode.h
index 479fdfc..e119354 100644
--- a/common/content_inode.h
+++ b/common/content_inode.h
@@ -347,4 +347,24 @@ typedef struct extattrhdr
extattrhdr_t;
      /* a linux "secure" mode attribute
       */

+/* Routines for calculating and validating checksums on
xfsdump headers
+ */
+static inline u_int32_t
+calc_checksum(void *startp, void *endp)
+{
+    u_int32_t sum;
+    u_int32_t *sump = (u_int32_t *)startp;
+    for (sum = 0; sump<  (u_int32_t
*)endp; sum += *sump++);
+    return ~sum + 1;
+}
+
+static inline bool_t
+is_checksum_valid(void *startp, void *endp)
+{
+    u_int32_t sum;
+    u_int32_t *sump = (u_int32_t *)startp;
+    for (sum = 0; sump<  (u_int32_t
*)endp; sum += *sump++);
+    return sum == 0 ? BOOL_TRUE :
BOOL_FALSE;
+}
+
  #endif /* CONTENT_INODE_H */
diff --git a/dump/content.c b/dump/content.c
index 9905c88..2cf15ba 100644
--- a/dump/content.c
+++ b/dump/content.c
@@ -585,6 +585,13 @@ content_init( intgen_t argc,
          sizeof(
content_inode_hdr_t ));
      ASSERT( sizeof( extattrhdr_t ) ==
EXTATTRHDR_SZ );

+    /* must be a multiple of 32-bits for
checksums
+     */
+    ASSERT( FILEHDR_SZ % sizeof( u_int32_t
) == 0 );
+    ASSERT( EXTENTHDR_SZ  % sizeof(
u_int32_t ) == 0 );
+    ASSERT( DIRENTHDR_SZ % sizeof(
u_int32_t ) == 0 );
+    ASSERT( EXTATTRHDR_SZ % sizeof(
u_int32_t ) == 0 );
+
      /* calculate offsets of portions of the
write hdr template
       */
      dwhdrtemplatep = ( drive_hdr_t *
)gwhdrtemplatep->gh_upper;
@@ -1491,8 +1498,7 @@ baseuuidbypass:
      var_skip(&fsid, inomap_skip );

      /* fill in write header template
content info. always produce
-     * an inomap and dir dump for
each media file. flag the checksums
-     * available if so compiled
(see -D...CHECKSUM in Makefile).
+     * an inomap and dir dump for
each media file.
       */
      ASSERT( sizeof(
cwhdrtemplatep->ch_specific )>= sizeof(
*scwhdrtemplatep ));
      scwhdrtemplatep->cih_mediafiletype =
CIH_MEDIAFILETYPE_DATA;
@@ -1506,15 +1512,9 @@ baseuuidbypass:
      if ( sc_inv_updatepr ) {

scwhdrtemplatep->cih_dumpattr |= CIH_DUMPATTR_INVENTORY;
      }
-#ifdef FILEHDR_CHECKSUM
      scwhdrtemplatep->cih_dumpattr |=
CIH_DUMPATTR_FILEHDR_CHECKSUM;
-#endif /* FILEHDR_CHECKSUM */
-#ifdef EXTENTHDR_CHECKSUM
      scwhdrtemplatep->cih_dumpattr |=
CIH_DUMPATTR_EXTENTHDR_CHECKSUM;
-#endif /* EXTENTHDR_CHECKSUM */
-#ifdef DIRENTHDR_CHECKSUM
      scwhdrtemplatep->cih_dumpattr |=
CIH_DUMPATTR_DIRENTHDR_CHECKSUM;
-#endif /* DIRENTHDR_CHECKSUM */
      scwhdrtemplatep->cih_dumpattr |=
CIH_DUMPATTR_DIRENTHDR_GEN;
      if ( sc_incrpr ) {

scwhdrtemplatep->cih_dumpattr |=
CIH_DUMPATTR_INCREMENTAL;
@@ -1528,10 +1528,8 @@ baseuuidbypass:
      }
      if ( sc_dumpextattrpr ) {

scwhdrtemplatep->cih_dumpattr |= CIH_DUMPATTR_EXTATTR;
-#ifdef EXTATTRHDR_CHECKSUM

scwhdrtemplatep->cih_dumpattr |=


CIH_DUMPATTR_EXTATTRHDR_CHECKSUM;
-#endif /* EXTATTRHDR_CHECKSUM */
      }

      scwhdrtemplatep->cih_rootino =
sc_rootxfsstatp->bs_ino;
@@ -3743,6 +3741,8 @@ dump_extattr_buildrecord( xfs_bstat_t
*statp,

    namesz, namesrcp,

    valuesz );
      ( void )strcpy( namep, namesrcp );
+
+    memset( ( void * )&tmpah, 0,
sizeof( tmpah ));
      tmpah.ah_sz = recsz;
      ASSERT( EXTATTRHDR_SZ + namesz<
UINT16MAX );
      tmpah.ah_valoff = ( u_int16_t )(
EXTATTRHDR_SZ + namesz );
@@ -3750,17 +3750,8 @@ dump_extattr_buildrecord(
xfs_bstat_t *statp,
          (( flag&
ATTR_ROOT ) ? EXTATTRHDR_FLAGS_ROOT :
          (( flag&
ATTR_SECURE ) ? EXTATTRHDR_FLAGS_SECURE : 0));
      tmpah.ah_valsz = valuesz;
-    tmpah.ah_checksum = 0;
-#ifdef EXTATTRHDR_CHECKSUM
-    {
-    register u_int32_t *sump = ( u_int32_t
* )ahdrp;
-    register u_int32_t *endp = ( u_int32_t
* )( ahdrp + 1 );
-    register u_int32_t sum;
      tmpah.ah_flags |=
EXTATTRHDR_FLAGS_CHECKSUM;
-    for ( sum = 0 ; sump<  endp ; sum +=
*sump++ ) ;
-    tmpah.ah_checksum = ~sum + 1;
-    }
-#endif /* EXTATTRHDR_CHECKSUM */
+    tmpah.ah_checksum = calc_checksum(
&tmpah,&tmpah + 1 );

      xlate_extattrhdr(ahdrp,&tmpah,
-1);
      *valuepp = valuep;
@@ -3782,23 +3773,13 @@ dump_extattrhdr( drive_t *drivep,
      intgen_t rval;
      rv_t rv;

+    memset( ( void * )&ahdr, 0, sizeof(
ahdr ));
      ahdr.ah_sz = recsz;
      ASSERT( valoff<  UINT16MAX );
      ahdr.ah_valoff = ( u_int16_t )valoff;
-    ahdr.ah_flags = ( u_int16_t )flags;
+    ahdr.ah_flags = ( u_int16_t )flags |
EXTATTRHDR_FLAGS_CHECKSUM;
      ahdr.ah_valsz = valsz;
-    ahdr.ah_checksum = 0;
-
-#ifdef EXTATTRHDR_CHECKSUM
-    {
-    register u_int32_t *sump = ( u_int32_t
* )&ahdr;
-    register u_int32_t *endp = ( u_int32_t
* )(&ahdr + 1 );
-    register u_int32_t sum;
-    ahdr.ah_flags |=
EXTATTRHDR_FLAGS_CHECKSUM;
-    for ( sum = 0 ; sump<  endp ; sum +=
*sump++ ) ;
-    ahdr.ah_checksum = ~sum + 1;
-    }
-#endif /* EXTATTRHDR_CHECKSUM */
+    ahdr.ah_checksum = calc_checksum(
&ahdr,&ahdr + 1 );

      xlate_extattrhdr(&ahdr,
&tmpahdr, 1);
      rval = write_buf( ( char *
)&tmpahdr,
@@ -5102,11 +5083,6 @@ dump_filehdr( drive_t *drivep,
      drive_ops_t *dop = drivep->d_opsp;
      register filehdr_t *fhdrp =
contextp->cc_filehdrp;
      filehdr_t tmpfhdrp;
-#ifdef FILEHDR_CHECKSUM
-    register u_int32_t *sump = ( u_int32_t
* )fhdrp;
-    register u_int32_t *endp = ( u_int32_t
* )( fhdrp + 1 );
-    register u_int32_t sum;
-#endif /* FILEHDR_CHECKSUM */
      intgen_t rval;
      rv_t rv;

@@ -5118,13 +5094,8 @@ dump_filehdr( drive_t *drivep,

copy_xfs_bstat(&fhdrp->fh_stat, statp);
      }
      fhdrp->fh_offset = offset;
-    fhdrp->fh_flags = flags;
-
-#ifdef FILEHDR_CHECKSUM
-    fhdrp->fh_flags |=
FILEHDR_FLAGS_CHECKSUM;
-    for ( sum = 0 ; sump<  endp ; sum +=
*sump++ ) ;
-    fhdrp->fh_checksum = ~sum + 1;
-#endif /* FILEHDR_CHECKSUM */
+    fhdrp->fh_flags = flags |
FILEHDR_FLAGS_CHECKSUM;
+    fhdrp->fh_checksum = calc_checksum(
fhdrp, fhdrp + 1 );

      xlate_filehdr(fhdrp,&tmpfhdrp,
1);
      rval = write_buf( ( char *
)&tmpfhdrp,
@@ -5164,11 +5135,6 @@ dump_extenthdr( drive_t *drivep,
      drive_ops_t *dop = drivep->d_opsp;
      register extenthdr_t *ehdrp =
contextp->cc_extenthdrp;
      extenthdr_t tmpehdrp;
-#ifdef EXTENTHDR_CHECKSUM
-    register u_int32_t *sump = ( u_int32_t
* )ehdrp;
-    register u_int32_t *endp = ( u_int32_t
* )( ehdrp + 1 );
-    register u_int32_t sum;
-#endif /* EXTENTHDR_CHECKSUM */
      intgen_t rval;
      rv_t rv;
      char typestr[20];
@@ -5198,15 +5164,10 @@ dump_extenthdr( drive_t *drivep,

      ( void )memset( ( void * )ehdrp, 0,
sizeof( *ehdrp ));
      ehdrp->eh_type = type;
-    ehdrp->eh_flags = flags;
+    ehdrp->eh_flags = flags |
EXTENTHDR_FLAGS_CHECKSUM;
      ehdrp->eh_offset = offset;
      ehdrp->eh_sz = sz;
-
-#ifdef EXTENTHDR_CHECKSUM
-    ehdrp->eh_flags |=
EXTENTHDR_FLAGS_CHECKSUM;
-    for ( sum = 0 ; sump<  endp ; sum +=
*sump++ ) ;
-    ehdrp->eh_checksum = ~sum + 1;
-#endif /* EXTENTHDR_CHECKSUM */
+    ehdrp->eh_checksum = calc_checksum(
ehdrp, ehdrp + 1 );

      xlate_extenthdr(ehdrp,&tmpehdrp,
1);
      rval = write_buf( ( char *
)&tmpehdrp,
@@ -5249,11 +5210,6 @@ dump_dirent( drive_t *drivep,
      direnthdr_t *tmpdhdrp;
      size_t direntbufsz =
contextp->cc_mdirentbufsz;
      size_t sz;
-#ifdef DIRENTHDR_CHECKSUM
-    register u_int32_t *sump = ( u_int32_t
* )dhdrp;
-    register u_int32_t *endp = ( u_int32_t
* )( dhdrp + 1 );
-    register u_int32_t sum;
-#endif /* DIRENTHDR_CHECKSUM */
      intgen_t rval;
      rv_t rv;

@@ -5290,10 +5246,7 @@ dump_dirent( drive_t *drivep,
          strcpy(
dhdrp->dh_name, name );
      }

-#ifdef DIRENTHDR_CHECKSUM
-    for ( sum = 0 ; sump<  endp ; sum +=
*sump++ ) ;
-    dhdrp->dh_checksum = ~sum + 1;
-#endif /* DIRENTHDR_CHECKSUM */
+    dhdrp->dh_checksum = calc_checksum(
dhdrp, dhdrp + 1);

      tmpdhdrp = malloc(sz);
      xlate_direnthdr(dhdrp, tmpdhdrp, 1);
diff --git a/restore/Makefile b/restore/Makefile
index 78ecc2c..588a8f0 100644
--- a/restore/Makefile
+++ b/restore/Makefile
@@ -103,7 +103,7 @@ LLDLIBS = $(LIBUUID) $(LIBHANDLE)
$(LIBATTR) $(LIBRMT)
  LTDEPENDENCIES = $(LIBRMT)

  LCFLAGS = -DRESTORE -DRMT -DBASED -DDOSOCKS -DINVCONVFIX
-DPIPEINVFIX \
-    -DEOMFIX -DSESSCPLT -DWHITEPARSE
-DDIRENTHDR_CHECKSUM \
+    -DEOMFIX -DSESSCPLT -DWHITEPARSE \
      -DF_FSSETDM

  default: depend $(LTCOMMAND)
diff --git a/restore/content.c b/restore/content.c
index e3e4994..25849d7 100644
--- a/restore/content.c
+++ b/restore/content.c
@@ -8000,11 +8000,6 @@ read_filehdr( drive_t *drivep,
filehdr_t *fhdrp, bool_t fhcs )
      drive_ops_t *dop = drivep->d_opsp;
      /* REFERENCED */
      intgen_t nread;
-#ifdef FILEHDR_CHECKSUM
-    register u_int32_t *sump = ( u_int32_t
* )fhdrp;
-    register u_int32_t *endp = ( u_int32_t
* )( fhdrp + 1 );
-    register u_int32_t sum;
-#endif /* FILEHDR_CHECKSUM */
      intgen_t rval;
      filehdr_t tmpfh;

@@ -8041,21 +8036,18 @@ read_filehdr( drive_t *drivep,
filehdr_t *fhdrp, bool_t fhcs )

bstatp->bs_ino,
            bstatp->bs_mode
);

-#ifdef FILEHDR_CHECKSUM
      if ( fhcs ) {
          if ( ! (
fhdrp->fh_flags&  FILEHDR_FLAGS_CHECKSUM )) {

mlog( MLOG_NORMAL | MLOG_WARNING, _(

       "corrupt file header\n") );

return RV_CORRUPT;
          }
-        for ( sum = 0 ; sump
<  endp ; sum += *sump++ ) ;
-        if ( sum ) {
+        if (
!is_checksum_valid( fhdrp, fhdrp + 1)) {

mlog( MLOG_NORMAL | MLOG_WARNING, _(

       "bad file header checksum\n") );

return RV_CORRUPT;
          }
      }
-#endif /* FILEHDR_CHECKSUM */

      return RV_OK;
  }
@@ -8067,11 +8059,6 @@ read_extenthdr( drive_t *drivep,
extenthdr_t *ehdrp, bool_t ehcs )
      drive_ops_t *dop = drivep->d_opsp;
      /* REFERENCED */
      intgen_t nread;
-#ifdef EXTENTHDR_CHECKSUM
-    register u_int32_t *sump = ( u_int32_t
* )ehdrp;
-    register u_int32_t *endp = ( u_int32_t
* )( ehdrp + 1 );
-    register u_int32_t sum;
-#endif /* EXTENTHDR_CHECKSUM */
      intgen_t rval;
      extenthdr_t tmpeh;

@@ -8108,21 +8095,18 @@ read_extenthdr( drive_t *drivep,
extenthdr_t *ehdrp, bool_t ehcs )

ehdrp->eh_type,
            ehdrp->eh_flags
);

-#ifdef EXTENTHDR_CHECKSUM
      if ( ehcs ) {
          if ( ! (
ehdrp->eh_flags&  EXTENTHDR_FLAGS_CHECKSUM )) {

mlog( MLOG_NORMAL | MLOG_WARNING, _(

       "corrupt extent header\n") );

return RV_CORRUPT;
          }
-        for ( sum = 0 ; sump
<  endp ; sum += *sump++ ) ;
-        if ( sum ) {
+        if (
!is_checksum_valid( ehdrp, ehdrp + 1)) {

mlog( MLOG_NORMAL | MLOG_WARNING, _(

       "bad extent header checksum\n") );

return RV_CORRUPT;
          }
      }
-#endif /* EXTENTHDR_CHECKSUM */

      return RV_OK;
  }
@@ -8137,11 +8121,6 @@ read_dirent( drive_t *drivep,
      drive_ops_t *dop = drivep->d_opsp;
      /* REFERENCED */
      intgen_t nread;
-#ifdef DIRENTHDR_CHECKSUM
-    register u_int32_t *sump = ( u_int32_t
* )dhdrp;
-    register u_int32_t *endp = ( u_int32_t
* )( dhdrp + 1 );
-    register u_int32_t sum;
-#endif /* DIRENTHDR_CHECKSUM */
      intgen_t rval;
      direnthdr_t tmpdh;

@@ -8180,21 +8159,18 @@ read_dirent( drive_t *drivep,
            ( size_t
)dhdrp->dh_gen,
            ( size_t
)dhdrp->dh_sz );

-#ifdef DIRENTHDR_CHECKSUM
      if ( dhcs ) {
          if ( dhdrp->dh_sz
== 0 ) {

mlog( MLOG_NORMAL | MLOG_WARNING, _(

       "corrupt directory entry header\n") );

return RV_CORRUPT;
          }
-        for ( sum = 0 ; sump
<  endp ; sum += *sump++ ) ;
-        if ( sum ) {
+        if (
!is_checksum_valid( dhdrp, dhdrp + 1)) {

mlog( MLOG_NORMAL | MLOG_WARNING, _(

       "bad directory entry header
checksum\n") );

return RV_CORRUPT;
          }
      }
-#endif /* DIRENTHDR_CHECKSUM */

      /* if null, return
       */
@@ -8246,11 +8222,6 @@ read_extattrhdr( drive_t *drivep,
extattrhdr_t *ahdrp, bool_t ahcs )
      drive_ops_t *dop = drivep->d_opsp;
      /* REFERENCED */
      intgen_t nread;
-#ifdef EXTATTRHDR_CHECKSUM
-    register u_int32_t *sump = ( u_int32_t
* )ahdrp;
-    register u_int32_t *endp = ( u_int32_t
* )( ahdrp + 1 );
-    register u_int32_t sum;
-#endif /* EXTATTRHDR_CHECKSUM */
      intgen_t rval;
      extattrhdr_t tmpah;

@@ -8288,21 +8259,18 @@ read_extattrhdr( drive_t *drivep,
extattrhdr_t *ahdrp, bool_t ahcs )

ahdrp->ah_valsz,

ahdrp->ah_checksum );

-#ifdef EXTATTRHDR_CHECKSUM
      if ( ahcs ) {
          if ( ! (
ahdrp->ah_flags&  EXTATTRHDR_FLAGS_CHECKSUM )) {

mlog( MLOG_NORMAL | MLOG_WARNING, _(

       "corrupt extattr header\n") );

return RV_CORRUPT;
          }
-        for ( sum = 0 ; sump
<  endp ; sum += *sump++ ) ;
-        if ( sum ) {
+        if (
!is_checksum_valid( ahdrp, ahdrp + 1)) {

mlog( MLOG_NORMAL | MLOG_WARNING, _(

       "bad extattr header checksum\n") );

return RV_CORRUPT;
          }
      }
-#endif /* EXTATTRHDR_CHECKSUM */

      return RV_OK;
  }
--
1.7.0.4

_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs


<Prev in Thread] Current Thread [Next in Thread>