From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Seung-Woo Kim <sw0312.kim@samsung.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>,
Linus Torvalds <torvalds@linux-foundation.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 12:11:58 +0200 [thread overview]
Message-ID: <20180810101158.GA31002@kroah.com> (raw)
In-Reply-To: <20180810064258epcas1p3eb68d29dbd60b705d0473b3c645496ff~Jcs48Q55D0165001650epcas1p3e@epcas1p3.samsung.com>
On Fri, Aug 10, 2018 at 03:43:02PM +0900, Seung-Woo Kim wrote:
> 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.
So are the 5 patches you sent ok to apply to the 3.18-stable tree? Or
do we need to do something else?
thanks,
greg k-h
next prev parent reply other threads:[~2018-08-10 10:12 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
2018-08-10 10:11 ` Greg Kroah-Hartman [this message]
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=20180810101158.GA31002@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=akpm@linux-foundation.org \
--cc=jslaby@suse.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=lwn@lwn.net \
--cc=stable@vger.kernel.org \
--cc=sw0312.kim@samsung.com \
--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).