xfs: Don't log uninitialised fields in inode structures

Source kernel commit: 20413e37d71befd02b5846acdaf5e2564dd1c38e

Prevent kmemcheck from throwing warnings about reading uninitialised
memory when formatting inodes into the incore log buffer. There are
several issues here - we don't always log all the fields in the
inode log format item, and we never log the inode the
di_next_unlinked field.

In the case of the inode log format item, this is exacerbated
by the old xfs_inode_log_format structure padding issue. Hence make
the padded, 64 bit aligned version of the structure the one we always
use for formatting the log and get rid of the 64 bit variant. This
means we'll always log the 64-bit version and so recovery only needs
to convert from the unpadded 32 bit version from older 32 bit
kernels.

Signed-Off-By: Dave Chinner <dchinner@redhat.com>
Tested-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

diff --git a/libxfs/xfs_log_format.h b/libxfs/xfs_log_format.h
index 8372e9bcd7b6ba4b8fcc7b9133d5acecef4842e2..71de185735e06c1d44e13e55e30ff1d38b5a62d8 100644 (file)
--- a/libxfs/xfs_log_format.h
+++ b/libxfs/xfs_log_format.h
@@ -270,6 +270,7 @@ typedef struct xfs_inode_log_format {
        uint32_t                ilf_fields;     /* flags for fields logged */
        uint16_t                ilf_asize;      /* size of attr d/ext/root */
        uint16_t                ilf_dsize;      /* size of data/ext/root */
+       uint32_t                ilf_pad;        /* pad for 64 bit boundary */
        uint64_t                ilf_ino;        /* inode number */
        union {
                uint32_t        ilfu_rdev;      /* rdev value for dev inode*/
@@ -280,29 +281,17 @@ typedef struct xfs_inode_log_format {
        int32_t                 ilf_boffset;    /* off of inode in buffer */
 } xfs_inode_log_format_t;
 
-typedef struct xfs_inode_log_format_32 {
-       uint16_t                ilf_type;       /* inode log item type */
-       uint16_t                ilf_size;       /* size of this item */
-       uint32_t                ilf_fields;     /* flags for fields logged */
-       uint16_t                ilf_asize;      /* size of attr d/ext/root */
-       uint16_t                ilf_dsize;      /* size of data/ext/root */
-       uint64_t                ilf_ino;        /* inode number */
-       union {
-               uint32_t        ilfu_rdev;      /* rdev value for dev inode*/
-               uuid_t          ilfu_uuid;      /* mount point value */
-       } ilf_u;
-       int64_t                 ilf_blkno;      /* blkno of inode buffer */
-       int32_t                 ilf_len;        /* len of inode buffer */
-       int32_t                 ilf_boffset;    /* off of inode in buffer */
-} __attribute__((packed)) xfs_inode_log_format_32_t;
-
-typedef struct xfs_inode_log_format_64 {
+/*
+ * Old 32 bit systems will log in this format without the 64 bit
+ * alignment padding. Recovery will detect this and convert it to the
+ * correct format.
+ */
+struct xfs_inode_log_format_32 {
        uint16_t                ilf_type;       /* inode log item type */
        uint16_t                ilf_size;       /* size of this item */
        uint32_t                ilf_fields;     /* flags for fields logged */
        uint16_t                ilf_asize;      /* size of attr d/ext/root */
        uint16_t                ilf_dsize;      /* size of data/ext/root */
-       uint32_t                ilf_pad;        /* pad for 64 bit boundary */
        uint64_t                ilf_ino;        /* inode number */
        union {
                uint32_t        ilfu_rdev;      /* rdev value for dev inode*/
@@ -311,7 +300,7 @@ typedef struct xfs_inode_log_format_64 {
        int64_t                 ilf_blkno;      /* blkno of inode buffer */
        int32_t                 ilf_len;        /* len of inode buffer */
        int32_t                 ilf_boffset;    /* off of inode in buffer */
-} xfs_inode_log_format_64_t;
+} __attribute__((packed));
 
 
 /*

diff --git a/logprint/log_misc.c b/logprint/log_misc.c
index 2fd01ceb668baee3d2ab2890ddccbe8173a0b109..9d8cd95a1acccc87c56c07e5410ce0e04d271dbb 100644 (file)
--- a/logprint/log_misc.c
+++ b/logprint/log_misc.c
@@ -510,21 +510,21 @@ xlog_print_dir2_sf(
 
 int
 xlog_print_trans_inode(
-       struct xlog     *log,
-       char            **ptr,
-       int             len,
-       int             *i,
-       int             num_ops,
-       int             continued)
+       struct xlog             *log,
+       char                    **ptr,
+       int                     len,
+       int                     *i,
+       int                     num_ops,
+       int                     continued)
 {
-    struct xfs_log_dinode dino;
-    xlog_op_header_t      *op_head;
-    xfs_inode_log_format_t dst_lbuf;
-    xfs_inode_log_format_64_t src_lbuf; /* buffer of biggest one */
-    xfs_inode_log_format_t *f;
-    int                           mode;
-    int                           size;
-    int                           skip_count;
+    struct xfs_log_dinode      dino;
+    struct xlog_op_header      *op_head;
+    struct xfs_inode_log_format        dst_lbuf;
+    struct xfs_inode_log_format        src_lbuf;
+    struct xfs_inode_log_format *f;
+    int                                mode;
+    int                                size;
+    int                                skip_count;
 
     /*
      * print inode type header region
@@ -532,15 +532,15 @@ xlog_print_trans_inode(
      * memmove to ensure 8-byte alignment for the long longs in
      * xfs_inode_log_format_t structure
      *
-     * len can be smaller than xfs_inode_log_format_32|64_t
+     * len can be smaller than xfs_inode_log_format_t
      * if format data is split over operations
      */
-    memmove(&src_lbuf, *ptr, MIN(sizeof(xfs_inode_log_format_64_t), len));
+    memmove(&src_lbuf, *ptr, MIN(sizeof(src_lbuf), len));
     (*i)++;                                    /* bump index */
     *ptr += len;
     if (!continued &&
-       (len == sizeof(xfs_inode_log_format_32_t) ||
-        len == sizeof(xfs_inode_log_format_64_t))) {
+       (len == sizeof(struct xfs_inode_log_format_32) ||
+        len == sizeof(struct xfs_inode_log_format))) {
        f = xfs_inode_item_format_convert((char*)&src_lbuf, len, &dst_lbuf);
        printf(_("INODE: "));
        printf(_("#regs: %d   ino: 0x%llx  flags: 0x%x   dsize: %d\n"),
@@ -1477,48 +1477,31 @@ end:
 }
 
 /*
- * if necessary, convert an xfs_inode_log_format struct from 32bit or 64 bit versions
- * (which can have different field alignments) to the native version
+ * if necessary, convert an xfs_inode_log_format struct from the old 32bit version
+ * (which can have different field alignments) to the native 64 bit version
  */
 xfs_inode_log_format_t *
 xfs_inode_item_format_convert(char *src_buf, uint len, xfs_inode_log_format_t *in_f)
 {
+       struct xfs_inode_log_format_32  *in_f32;
+
        /* if we have native format then just return buf without copying data */
        if (len == sizeof(xfs_inode_log_format_t)) {
                return (xfs_inode_log_format_t *)src_buf;
        }
 
-       if (len == sizeof(xfs_inode_log_format_32_t)) {
-               xfs_inode_log_format_32_t *in_f32;
-
-               in_f32 = (xfs_inode_log_format_32_t *)src_buf;
-               in_f->ilf_type = in_f32->ilf_type;
-               in_f->ilf_size = in_f32->ilf_size;
-               in_f->ilf_fields = in_f32->ilf_fields;
-               in_f->ilf_asize = in_f32->ilf_asize;
-               in_f->ilf_dsize = in_f32->ilf_dsize;
-               in_f->ilf_ino = in_f32->ilf_ino;
-               /* copy biggest */
-               memcpy(&in_f->ilf_u.ilfu_uuid, &in_f32->ilf_u.ilfu_uuid, sizeof(uuid_t));
-               in_f->ilf_blkno = in_f32->ilf_blkno;
-               in_f->ilf_len = in_f32->ilf_len;
-               in_f->ilf_boffset = in_f32->ilf_boffset;
-       } else {
-               xfs_inode_log_format_64_t *in_f64;
-
-               ASSERT(len == sizeof(xfs_inode_log_format_64_t));
-               in_f64 = (xfs_inode_log_format_64_t *)src_buf;
-               in_f->ilf_type = in_f64->ilf_type;
-               in_f->ilf_size = in_f64->ilf_size;
-               in_f->ilf_fields = in_f64->ilf_fields;
-               in_f->ilf_asize = in_f64->ilf_asize;
-               in_f->ilf_dsize = in_f64->ilf_dsize;
-               in_f->ilf_ino = in_f64->ilf_ino;
-               /* copy biggest */
-               memcpy(&in_f->ilf_u.ilfu_uuid, &in_f64->ilf_u.ilfu_uuid, sizeof(uuid_t));
-               in_f->ilf_blkno = in_f64->ilf_blkno;
-               in_f->ilf_len = in_f64->ilf_len;
-               in_f->ilf_boffset = in_f64->ilf_boffset;
-       }
+       in_f32 = (struct xfs_inode_log_format_32 *)src_buf;
+       in_f->ilf_type = in_f32->ilf_type;
+       in_f->ilf_size = in_f32->ilf_size;
+       in_f->ilf_fields = in_f32->ilf_fields;
+       in_f->ilf_asize = in_f32->ilf_asize;
+       in_f->ilf_dsize = in_f32->ilf_dsize;
+       in_f->ilf_ino = in_f32->ilf_ino;
+       /* copy biggest field of ilf_u */
+       memcpy(&in_f->ilf_u.ilfu_uuid, &in_f32->ilf_u.ilfu_uuid, sizeof(uuid_t));
+       in_f->ilf_blkno = in_f32->ilf_blkno;
+       in_f->ilf_len = in_f32->ilf_len;
+       in_f->ilf_boffset = in_f32->ilf_boffset;
+
        return in_f;
 }

diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c
index 3863ba95235e28d35772a14e6f5fed362ed3c94f..c3112ceba0defc563dbc11dc1f659c92fd956b97 100644 (file)
--- a/logprint/log_print_all.c
+++ b/logprint/log_print_all.c
@@ -288,8 +288,8 @@ xlog_recover_print_inode(
        int                     hasdata;
        int                     hasattr;
 
-       ASSERT(item->ri_buf[0].i_len == sizeof(xfs_inode_log_format_32_t) ||
-              item->ri_buf[0].i_len == sizeof(xfs_inode_log_format_64_t));
+       ASSERT(item->ri_buf[0].i_len == sizeof(struct xfs_inode_log_format_32) ||
+              item->ri_buf[0].i_len == sizeof(struct xfs_inode_log_format));
        f = xfs_inode_item_format_convert(item->ri_buf[0].i_addr, item->ri_buf[0].i_len, &f_buf);
 
        printf(_("      INODE: #regs:%d   ino:0x%llx  flags:0x%x   dsize:%d\n"),