Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <CGME20180530073304epcas3p4967df82d2d086fd08fd354781df61861@epcas3p4.samsung.com>
 <20180530073211.GA22422@kroah.com> <20180703032456epcas1p29c4570ae3f6bb3f8d7c2d475e1ba4658~9vfIpgNT51989519895epcas1p2d@epcas1p2.samsung.com>
 <20180703043655.GA9793@kroah.com> <20180703044302epcas1p4ff5961129c010a2df3548cf678b6274c~9wjUy-vFW2959129591epcas1p4p@epcas1p4.samsung.com>
In-Reply-To: <20180703044302epcas1p4ff5961129c010a2df3548cf678b6274c~9wjUy-vFW2959129591epcas1p4p@epcas1p4.samsung.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Mon, 2 Jul 2018 22:01:25 -0700
Message-ID: <CA+55aFz-K8s-Sh86PqxRaXTD5pG=YsVUCxf9ue8XOSaGmg-brw@mail.gmail.com>
Subject: Re: Linux 3.18.111
To:     Seung-Woo Kim <sw0312.kim@samsung.com>,
        Al Viro <viro@zeniv.linux.org.uk>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        stable <stable@vger.kernel.org>, lwn@lwn.net,
        Jiri Slaby <jslaby@suse.cz>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Jul 2, 2018 at 9:43 PM Seung-Woo Kim <sw0312.kim@samsung.com> wrote:
>
> I think the commit itself is required. Simple, but not reliable,
> workaround fix is like below:
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index a34d401..7c751f2 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -1879,6 +1879,8 @@ void d_instantiate_new(struct dentry *entry,
> struct inode *inode)
>         BUG_ON(!hlist_unhashed(&entry->d_u.d_alias));
>         BUG_ON(!inode);
>         lockdep_annotate_inode_mutex_key(inode);
> +       /* WORKAROUND for calling security_d_instantiate() */
> +       entry->d_inode = inode;
>         security_d_instantiate(entry, inode);
>         spin_lock(&inode->i_lock);
>         __d_instantiate(entry, inode);

Ugh. That looks horrible even if it might avoid the oops.

I think a much better solution is to back-port commit b296821a7c42
("xattr_handler: pass dentry and inode as separate arguments of
->get()") to older kernels. Then the inode is passed down all the way,
and you don't have people try to get it from the (not yet initialized)
dentry.

But there might be other parts missing too, and I didn't look at how
easy/painful that backport would be.

Al - comments? This is all because of commit 1e2e547a93a0 ("do
d_instantiate/unlock_new_inode combinations safely") being marked for
stable, and various cases of security_d_instantiate() calling down to
getxattr. Which used to not get the inode at all, so those older
kernels use d_inode(dentry), which doesn't work in this path since
dentry->d_inode hasn't been instantiated yet..

                Linus