Thread (14 messages) 14 messages, 4 authors, 2017-09-20

Re: xfs: Uninitialized memory read at xlog_write

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: 2017-09-15 11:19:39

Dave Chinner wrote:
On Thu, Sep 14, 2017 at 07:15:38PM +0900, Tetsuo Handa wrote:
quoted
----------
         Starting Load/Save Random Seed...

         Starting Configure read-only root support...

[ 1106.927991] ptr=ffffc90001c08218 reg->i_addr=ffff880134c7fda8 copy_off=0 copy_len=16
[ 1106.928022] ptr=ffffc90001c08234 reg->i_addr=ffff88013395f858 copy_off=0 copy_len=56
[ 1106.928100] ptr=ffffc90001c08278 reg->i_addr=ffff88013395f890 copy_off=0 copy_len=96
[ 1106.932354] WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (ffff88013395f860)
Hold on- that warning has come from the /prior/ region copy, not
the current region!

i.e. 2nd last copy region was ffff88013395f858 for 0x38 bytes, and
this spans address that failed ffff88013395f860. i.e. it's 8 bytes
into that region. It's 48 bytes before the region that we were
copying when kmemcheck triggered!

I'm going to guess that we've got a log op header, followed by
a inode format header, follow by something like a v2 inode core.
i.e. log op headers are 16 bytes, xfs_inode_log_format are 56 bytes,
and a v2 inode core is 96 bytes.

So, the inode log format header:

struct xfs_inode_log_format {
        uint16_t                   ilf_type;             /*     0     2 */
        uint16_t                   ilf_size;             /*     2     2 */
        uint32_t                   ilf_fields;           /*     4     4 */
        uint16_t                   ilf_asize;            /*     8     2 */
        uint16_t                   ilf_dsize;            /*    10     2 */

        /* XXX 4 bytes hole, try to pack */

        uint64_t                   ilf_ino;              /*    16     8 */
        union {
                uint32_t           ilfu_rdev;            /*           4 */
                uuid_t             ilfu_uuid;            /*          16 */
        } ilf_u;                                         /*    24    16 */
        int64_t                    ilf_blkno;            /*    40     8 */
        int32_t                    ilf_len;              /*    48     4 */
        int32_t                    ilf_boffset;          /*    52     4 */

        /* size: 56, cachelines: 1, members: 10 */
        /* sum members: 52, holes: 1, sum holes: 4 */
        /* last cacheline: 56 bytes */
};

Oh, that's right. Someone, a long, long time ago, screwed up the on
disk inode log structure but nobody noticed it due to the Irix MIPS
compilers padding the structure identically on 32 and 64 bit
systems. Port to linux, and i386/x86-64 padded it differently. The
fix at the time was to make it right in recovery and continue to
use the native structure at runtime. Ok, let's fix that properly
now.

Try the patch below.
Bingo! This patch fixes the XFS splat. Thank you.
Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

xfs: Don't log uninitialised fields in inode structures

From: Dave Chinner <redacted>

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 aused b cerbated
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 <redacted>
---
 fs/xfs/libxfs/xfs_log_format.h | 27 +++++----------
 fs/xfs/xfs_inode_item.c        | 79 ++++++++++++++++++++++--------------------
 fs/xfs/xfs_ondisk.h            |  2 +-
 3 files changed, 50 insertions(+), 58 deletions(-)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help