linux-kernel - Re: Linux 3.18.111

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:   Wed, 08 Aug 2018 19:06:26 +0900
From:   Seung-Woo Kim <sw0312.kim@...sung.com>
To:     Al Viro <viro@...IV.linux.org.uk>
CC:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        stable <stable@...r.kernel.org>, lwn@....net,
        Jiri Slaby <jslaby@...e.cz>,
        Seung-Woo Kim <sw0312.kim@...sung.com>
Subject: Re: Linux 3.18.111

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@...sung.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?

-- 
Seung-Woo Kim
Samsung Research
--