From: Seung-Woo Kim <sw0312.kim@samsung.com>
To: sw0312.kim@samsung.com
Cc: Al Viro <viro@ZenIV.linux.org.uk>,
Linus Torvalds <torvalds@linux-foundation.org>,
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>
Subject: Re: Linux 3.18.111
Date: Fri, 10 Aug 2018 15:43:02 +0900 [thread overview]
Message-ID: <20180810064258epcas1p3eb68d29dbd60b705d0473b3c645496ff~Jcs48Q55D0165001650epcas1p3e@epcas1p3.samsung.com> (raw)
In-Reply-To: <20180808100622epcas1p1d4e3d6f858a9de3014f6d79bf284ff2c~I4L612J9p0428204282epcas1p1X@epcas1p1.samsung.com>
On 2018년 08월 08일 19:06, Seung-Woo Kim wrote:
> On 2018년 07월 05일 09:52, Al Viro wrote:
>> On Mon, Jul 02, 2018 at 10:01:25PM -0700, Linus Torvalds wrote:
>>> 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..
>>
>> You also want b96809173e94 and ce23e6401334 there...
>
> For above two commits, also b296821a7c42 is required. And after
> backport, smack still crashed because setxattr. To fix it, 5930122683df
> and 3767e255b390 are also required.
>
> By the way, does no one have met this kind getxattr crash issue with
> selinux from 3.18.y?
>
I have checked with selinux, and it is confirmed that there is no crash
because selinux_d_instantiate() has null check for inode. So, it is only
security smack issue.
--
Seung-Woo Kim
Samsung Research
--
next prev parent reply other threads:[~2018-08-10 6:43 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20180530073304epcas3p4967df82d2d086fd08fd354781df61861@epcas3p4.samsung.com>
2018-05-30 7:32 ` Linux 3.18.111 Greg KH
2018-05-30 7:32 ` Greg KH
2018-07-03 3:24 ` Seung-Woo Kim
2018-07-03 4:36 ` Greg KH
2018-07-03 4:43 ` Seung-Woo Kim
2018-07-03 5:01 ` Linus Torvalds
2018-07-05 0:52 ` Al Viro
2018-08-08 10:06 ` Seung-Woo Kim
2018-08-10 6:43 ` Seung-Woo Kim [this message]
2018-08-10 10:11 ` Greg Kroah-Hartman
2018-08-13 0:24 ` Seung-Woo Kim
[not found] ` <CGME20180809095342epcas1p1dde47ac63e7c4af1e389a524ff4f3a52@epcas1p1.samsung.com>
2018-08-09 9:53 ` [PATCH 3.18.y 1/5] xattr_handler: pass dentry and inode as separate arguments of ->get() Seung-Woo Kim
[not found] ` <1533808424-20649-1-git-send-email-sw0312.kim@samsung.com>
[not found] ` <CGME20180809095342epcas1p49a6d26e336b6e7f0f120583c410d2afb@epcas1p4.samsung.com>
2018-08-09 9:53 ` [PATCH 3.18.y 2/5] ->getxattr(): pass dentry and inode as separate arguments Seung-Woo Kim
[not found] ` <CGME20180809095342epcas1p425d6ada22d3863459e44c335b51801de@epcas1p4.samsung.com>
2018-08-09 9:53 ` [PATCH 3.18.y 3/5] security_d_instantiate(): move to the point prior to attaching dentry to inode Seung-Woo Kim
[not found] ` <CGME20180809095342epcas1p21503f49bbf9e4814ea76c509c873a559@epcas1p2.samsung.com>
2018-08-09 9:53 ` [PATCH 3.18.y 4/5] switch xattr_handler->set() to passing dentry and inode separately Seung-Woo Kim
[not found] ` <CGME20180809095342epcas1p4481a285a4a96ee10b26d94fc27c9e3fe@epcas1p4.samsung.com>
2018-08-09 9:53 ` [PATCH 3.18.y 5/5] switch ->setxattr() " Seung-Woo Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='20180810064258epcas1p3eb68d29dbd60b705d0473b3c645496ff~Jcs48Q55D0165001650epcas1p3e@epcas1p3.samsung.com' \
--to=sw0312.kim@samsung.com \
--cc=akpm@linux-foundation.org \
--cc=gregkh@linuxfoundation.org \
--cc=jslaby@suse.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=lwn@lwn.net \
--cc=stable@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=viro@ZenIV.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).