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