Commit 20413e37 authored by Dave Chinner's avatar Dave Chinner Committed by Darrick J. Wong

xfs: Don't log uninitialised fields in inode structures

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: default avatarDave Chinner <dchinner@redhat.com>
Tested-by: default avatarTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
Reviewed-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
parent e12199f8
...@@ -270,6 +270,7 @@ typedef struct xfs_inode_log_format { ...@@ -270,6 +270,7 @@ typedef struct xfs_inode_log_format {
uint32_t ilf_fields; /* flags for fields logged */ uint32_t ilf_fields; /* flags for fields logged */
uint16_t ilf_asize; /* size of attr d/ext/root */ uint16_t ilf_asize; /* size of attr d/ext/root */
uint16_t ilf_dsize; /* size of data/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 */ uint64_t ilf_ino; /* inode number */
union { union {
uint32_t ilfu_rdev; /* rdev value for dev inode*/ uint32_t ilfu_rdev; /* rdev value for dev inode*/
...@@ -280,29 +281,17 @@ typedef struct xfs_inode_log_format { ...@@ -280,29 +281,17 @@ typedef struct xfs_inode_log_format {
int32_t ilf_boffset; /* off of inode in buffer */ int32_t ilf_boffset; /* off of inode in buffer */
} xfs_inode_log_format_t; } xfs_inode_log_format_t;
typedef struct xfs_inode_log_format_32 { /*
uint16_t ilf_type; /* inode log item type */ * Old 32 bit systems will log in this format without the 64 bit
uint16_t ilf_size; /* size of this item */ * alignment padding. Recovery will detect this and convert it to the
uint32_t ilf_fields; /* flags for fields logged */ * correct format.
uint16_t ilf_asize; /* size of attr d/ext/root */ */
uint16_t ilf_dsize; /* size of data/ext/root */ struct xfs_inode_log_format_32 {
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 {
uint16_t ilf_type; /* inode log item type */ uint16_t ilf_type; /* inode log item type */
uint16_t ilf_size; /* size of this item */ uint16_t ilf_size; /* size of this item */
uint32_t ilf_fields; /* flags for fields logged */ uint32_t ilf_fields; /* flags for fields logged */
uint16_t ilf_asize; /* size of attr d/ext/root */ uint16_t ilf_asize; /* size of attr d/ext/root */
uint16_t ilf_dsize; /* size of data/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 */ uint64_t ilf_ino; /* inode number */
union { union {
uint32_t ilfu_rdev; /* rdev value for dev inode*/ uint32_t ilfu_rdev; /* rdev value for dev inode*/
...@@ -311,7 +300,7 @@ typedef struct xfs_inode_log_format_64 { ...@@ -311,7 +300,7 @@ typedef struct xfs_inode_log_format_64 {
int64_t ilf_blkno; /* blkno of inode buffer */ int64_t ilf_blkno; /* blkno of inode buffer */
int32_t ilf_len; /* len of inode buffer */ int32_t ilf_len; /* len of inode buffer */
int32_t ilf_boffset; /* off of inode in buffer */ int32_t ilf_boffset; /* off of inode in buffer */
} xfs_inode_log_format_64_t; } __attribute__((packed));
/* /*
......
...@@ -364,6 +364,9 @@ xfs_inode_to_log_dinode( ...@@ -364,6 +364,9 @@ xfs_inode_to_log_dinode(
to->di_dmstate = from->di_dmstate; to->di_dmstate = from->di_dmstate;
to->di_flags = from->di_flags; to->di_flags = from->di_flags;
/* log a dummy value to ensure log structure is fully initialised */
to->di_next_unlinked = NULLAGINO;
if (from->di_version == 3) { if (from->di_version == 3) {
to->di_changecount = inode->i_version; to->di_changecount = inode->i_version;
to->di_crtime.t_sec = from->di_crtime.t_sec; to->di_crtime.t_sec = from->di_crtime.t_sec;
...@@ -404,6 +407,11 @@ xfs_inode_item_format_core( ...@@ -404,6 +407,11 @@ xfs_inode_item_format_core(
* the second with the on-disk inode structure, and a possible third and/or * the second with the on-disk inode structure, and a possible third and/or
* fourth with the inode data/extents/b-tree root and inode attributes * fourth with the inode data/extents/b-tree root and inode attributes
* data/extents/b-tree root. * data/extents/b-tree root.
*
* Note: Always use the 64 bit inode log format structure so we don't
* leave an uninitialised hole in the format item on 64 bit systems. Log
* recovery on 32 bit systems handles this just fine, so there's no reason
* for not using an initialising the properly padded structure all the time.
*/ */
STATIC void STATIC void
xfs_inode_item_format( xfs_inode_item_format(
...@@ -412,8 +420,8 @@ xfs_inode_item_format( ...@@ -412,8 +420,8 @@ xfs_inode_item_format(
{ {
struct xfs_inode_log_item *iip = INODE_ITEM(lip); struct xfs_inode_log_item *iip = INODE_ITEM(lip);
struct xfs_inode *ip = iip->ili_inode; struct xfs_inode *ip = iip->ili_inode;
struct xfs_inode_log_format *ilf;
struct xfs_log_iovec *vecp = NULL; struct xfs_log_iovec *vecp = NULL;
struct xfs_inode_log_format *ilf;
ASSERT(ip->i_d.di_version > 1); ASSERT(ip->i_d.di_version > 1);
...@@ -425,7 +433,17 @@ xfs_inode_item_format( ...@@ -425,7 +433,17 @@ xfs_inode_item_format(
ilf->ilf_boffset = ip->i_imap.im_boffset; ilf->ilf_boffset = ip->i_imap.im_boffset;
ilf->ilf_fields = XFS_ILOG_CORE; ilf->ilf_fields = XFS_ILOG_CORE;
ilf->ilf_size = 2; /* format + core */ ilf->ilf_size = 2; /* format + core */
xlog_finish_iovec(lv, vecp, sizeof(struct xfs_inode_log_format));
/*
* make sure we don't leak uninitialised data into the log in the case
* when we don't log every field in the inode.
*/
ilf->ilf_dsize = 0;
ilf->ilf_asize = 0;
ilf->ilf_pad = 0;
uuid_copy(&ilf->ilf_u.ilfu_uuid, &uuid_null);
xlog_finish_iovec(lv, vecp, sizeof(*ilf));
xfs_inode_item_format_core(ip, lv, &vecp); xfs_inode_item_format_core(ip, lv, &vecp);
xfs_inode_item_format_data_fork(iip, ilf, lv, &vecp); xfs_inode_item_format_data_fork(iip, ilf, lv, &vecp);
...@@ -855,44 +873,29 @@ xfs_istale_done( ...@@ -855,44 +873,29 @@ xfs_istale_done(
} }
/* /*
* convert an xfs_inode_log_format struct from either 32 or 64 bit versions * convert an xfs_inode_log_format struct from the old 32 bit version
* (which can have different field alignments) to the native version * (which can have different field alignments) to the native 64 bit version
*/ */
int int
xfs_inode_item_format_convert( xfs_inode_item_format_convert(
xfs_log_iovec_t *buf, struct xfs_log_iovec *buf,
xfs_inode_log_format_t *in_f) struct xfs_inode_log_format *in_f)
{ {
if (buf->i_len == sizeof(xfs_inode_log_format_32_t)) { struct xfs_inode_log_format_32 *in_f32 = buf->i_addr;
xfs_inode_log_format_32_t *in_f32 = buf->i_addr;
if (buf->i_len != sizeof(*in_f32))
in_f->ilf_type = in_f32->ilf_type; return -EFSCORRUPTED;
in_f->ilf_size = in_f32->ilf_size;
in_f->ilf_fields = in_f32->ilf_fields; in_f->ilf_type = in_f32->ilf_type;
in_f->ilf_asize = in_f32->ilf_asize; in_f->ilf_size = in_f32->ilf_size;
in_f->ilf_dsize = in_f32->ilf_dsize; in_f->ilf_fields = in_f32->ilf_fields;
in_f->ilf_ino = in_f32->ilf_ino; in_f->ilf_asize = in_f32->ilf_asize;
/* copy biggest field of ilf_u */ in_f->ilf_dsize = in_f32->ilf_dsize;
uuid_copy(&in_f->ilf_u.ilfu_uuid, &in_f32->ilf_u.ilfu_uuid); in_f->ilf_ino = in_f32->ilf_ino;
in_f->ilf_blkno = in_f32->ilf_blkno; /* copy biggest field of ilf_u */
in_f->ilf_len = in_f32->ilf_len; uuid_copy(&in_f->ilf_u.ilfu_uuid, &in_f32->ilf_u.ilfu_uuid);
in_f->ilf_boffset = in_f32->ilf_boffset; in_f->ilf_blkno = in_f32->ilf_blkno;
return 0; in_f->ilf_len = in_f32->ilf_len;
} else if (buf->i_len == sizeof(xfs_inode_log_format_64_t)){ in_f->ilf_boffset = in_f32->ilf_boffset;
xfs_inode_log_format_64_t *in_f64 = buf->i_addr; return 0;
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 field of ilf_u */
uuid_copy(&in_f->ilf_u.ilfu_uuid, &in_f64->ilf_u.ilfu_uuid);
in_f->ilf_blkno = in_f64->ilf_blkno;
in_f->ilf_len = in_f64->ilf_len;
in_f->ilf_boffset = in_f64->ilf_boffset;
return 0;
}
return -EFSCORRUPTED;
} }
...@@ -134,7 +134,7 @@ xfs_check_ondisk_structs(void) ...@@ -134,7 +134,7 @@ xfs_check_ondisk_structs(void)
XFS_CHECK_STRUCT_SIZE(struct xfs_icreate_log, 28); XFS_CHECK_STRUCT_SIZE(struct xfs_icreate_log, 28);
XFS_CHECK_STRUCT_SIZE(struct xfs_ictimestamp, 8); XFS_CHECK_STRUCT_SIZE(struct xfs_ictimestamp, 8);
XFS_CHECK_STRUCT_SIZE(struct xfs_inode_log_format_32, 52); XFS_CHECK_STRUCT_SIZE(struct xfs_inode_log_format_32, 52);
XFS_CHECK_STRUCT_SIZE(struct xfs_inode_log_format_64, 56); XFS_CHECK_STRUCT_SIZE(struct xfs_inode_log_format, 56);
XFS_CHECK_STRUCT_SIZE(struct xfs_qoff_logformat, 20); XFS_CHECK_STRUCT_SIZE(struct xfs_qoff_logformat, 20);
XFS_CHECK_STRUCT_SIZE(struct xfs_trans_header, 16); XFS_CHECK_STRUCT_SIZE(struct xfs_trans_header, 16);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment