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:   Tue, 03 Jul 2018 13:43:06 +0900
From:   Seung-Woo Kim <sw0312.kim@...sung.com>
To:     Greg KH <gregkh@...uxfoundation.org>
CC:     linux-kernel@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        torvalds@...ux-foundation.org, 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월 03일 13:36, Greg KH wrote:
> On Tue, Jul 03, 2018 at 12:24:59PM +0900, Seung-Woo Kim wrote:
>> Hello,
>>
>> On 2018년 05월 30일 16:32, Greg KH wrote:
>>> I'm announcing the release of the 3.18.111 kernel.
>>>
>>> All users of the 3.18 kernel series must upgrade.
>>>
>>> The updated 3.18.y git tree can be found at:
>>> 	git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git linux-3.18.y
>>> and can be browsed at the normal kernel.org git web browser:
>>> 	http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=summary
>>>
>>> thanks,
>>>
>>> greg k-h
>>>
>>> ------------
>>
>> <snip.>
>>
>>>       do d_instantiate/unlock_new_inode combinations safely
>>
>> Recent my test in 3.18.113 kernel with security smack showed following
>> crash during mkdir on ext4 fs.
>>
>> Unable to handle kernel paging request at virtual address ffffffffffffff98
>> pgd = ffffffc012411000
>> [ffffffffffffff98] *pgd=0000000000000000, *pud=0000000000000000
>> ------------[ cut here ]------------
>> Kernel BUG at ffffffc0007d9430 [verbose debug info unavailable]
>> Internal error: Oops - BUG: 96000005 [#1] PREEMPT SMP
>> CPU: 0 MPIDR: 80000000 PID: 1237 Comm: mkdir Not tainted
>> 3.18.113-00083-g1bfc02f-dirty #29-Tizen
>> task: ffffffc02cbc2340 ti: ffffffc02b7fc000 task.ti: ffffffc02b7fc000
>> PC is at down_read+0x24/0x54
>> LR is at down_read+0x24/0x54
>> [...]
>> Call trace:
>> [<ffffffc0007d9430>] down_read+0x24/0x54
>> [<ffffffc00022ff64>] ext4_xattr_get+0x74/0x1f4
>> [<ffffffc000234838>] ext4_xattr_security_get+0x28/0x38
>> [<ffffffc0001ab9f0>] generic_getxattr+0x4c/0x60
>> [<ffffffc0002786a0>] smk_fetch.isra.6+0x8c/0xe0
>> [<ffffffc000278888>] smack_d_instantiate+0x194/0x324
>> [<ffffffc000273794>] security_d_instantiate+0x24/0x30
>> [<ffffffc00019edf4>] d_instantiate_new+0x34/0x94
>> [<ffffffc0002046b4>] ext4_mkdir+0x284/0x354
>> [<ffffffc0001959bc>] vfs_mkdir+0xc0/0x150
>> [<ffffffc00019a108>] SyS_mkdirat+0x88/0xb8
>> [<ffffffc00019a150>] SyS_mkdir+0x18/0x20
>> Code: aa0003f3 b00017c0 912e1000 97e38943 (c85f7e60)
>> ---[ end trace b1ad797d63dae9c5 ]---
>>
>> It is because d_instantiate_new() added from above commit calls
>> security_d_instantiate() before calling __d_instantiate() and
>> dentry->d_inode is not yet set and null. In 3.18.113 kernel,
>> inode->i_op_getxattr() of ext4 is still generic_getxattr() and it only
>> has dentry parameter without inode, so it tries to access dentry->d_inode.
>>
>> I did not test with selinux, but selinux also calls
>> inode->i_op_getxattr() from selinux_d_instantiate(), so maybe there is
>> also same issue.
> 
> So should I revert something or do you have a proposed fix for this?

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);
---

But I am not familiar with dentry/inode locking and there is no lock
consideration at all.

Thanks,
- Seung-Woo Kim

> 
> thanks,
> 
> greg k-h
> 
> 
> 

-- 
Seung-Woo Kim
Samsung Research
--