linux-kernel - Re: xfs: list corruption in xfs_setup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 19 Mar 2018 14:37:22 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Dave Chinner <dchinner@...hat.com>, darrick.wong@...cle.com
Cc:     linux-xfs@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
        Christoph Hellwig <hch@....de>,
        Al Viro <viro@...iv.linux.org.uk>
Subject: Re: xfs: list corruption in xfs_setup_inode()

On Mon, Oct 30, 2017 at 2:55 PM, Cong Wang <xiyou.wangcong@...il.com> wrote:
> Hello,
>
> We triggered a list corruption (double add) warning below on our 4.9
> kernel (the 4.9 kernel we use is based on -stable release, with only a
> few unrelated networking backports):

We still keep getting this warning on 4.9 kernel. Looking into this again,
it seems xfs_setup_inode() could be called twice if an XFS inode is gotten
from disk? Once in xfs_iget() => xfs_setup_existing_inode(), and once
in xfs_ialloc().

Does the following patch (compile-only) make any sense? Again, I don't
want to pretend to understand XFS...


diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 604ee384a00a..6761b1f8fa2f 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -775,6 +775,7 @@ xfs_ialloc(
        int             error;
        struct timespec tv;
        struct inode    *inode;
+       bool            had_imode;

        /*
         * Call the space management code to pick
@@ -801,6 +802,7 @@ xfs_ialloc(
                return error;
        ASSERT(ip != NULL);
        inode = VFS_I(ip);
+       had_imode = !!inode->i_mode;

        /*
         * We always convert v1 inodes to v2 now - we only support filesystems
@@ -946,7 +948,8 @@ xfs_ialloc(
        xfs_trans_log_inode(tp, ip, flags);

        /* now that we have an i_mode we can setup the inode structure */
-       xfs_setup_inode(ip);
+       if (!had_imode)
+               xfs_setup_inode(ip);

        *ipp = ip;
        return 0;