linux-kernel - [RFC] guarantee inode of alias's parent

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-next>] [day] [month] [year] [list]

Date:   Fri,  7 Jul 2017 15:28:11 +0200
From:   Gioh Kim <gi-oh.kim@...fitbricks.com>
To:     viro@...iv.linux.org.uk
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        Gioh Kim <gi-oh.kim@...fitbricks.com>
Subject: [RFC] guarantee inode of alias's parent

Hi,

My server with v4.4 generated panic at __d_unalias() function.
It is following line.

if (!mutex_trylock(&alias->d_parent->d_inode->i_mutex))

I checked panic message and found i_mutex pointer was garbage value.

Call trace is like following:
[<ffffffff811ae628>] d_splice_alias+0x148/0x2a0
[<ffffffff81209231>] kernfs_iop_lookup+0x91/0xc0
[<ffffffff8119fe08>] lookup_real+0x18/0x60
[<ffffffff811a0193>] __lookup_hash+0x33/0x40
[<ffffffff811a2bb8>] walk_component+0x238/0x500
[<ffffffff811a2fec>] link_path_walk+0x16c/0x580
[<ffffffff811a1987>] ? path_init+0x1f7/0x3c0
[<ffffffff811a43bb>] path_openat+0xab/0x14c0
[<ffffffff811a6a8c>] do_filp_open+0x7c/0xd0
[<ffffffff811b345a>] ? __alloc_fd+0x3a/0x170
[<ffffffff81195c52>] do_sys_open+0x132/0x220
[<ffffffff81195d59>] SyS_open+0x19/0x20
[<ffffffff81804997>] entry_SYSCALL_64_fastpath+0x12/0x6a

Before __d_unalias(), the dentry and parent of the dentry
are locked. But I wonder how I can assure of existence of the
alias and the parent of the alias.

Where is the code to lock alias and alias->d_parent?
What will happend if alias->d_parent be deleted by another process?

I checked kernel code of v4.10. It also does not have any lock before
accessing alias->d_parent. Therefore I'm attaching a patch to show my idea,
not solve a problem.
This patch is based on v4.10.0

Signed-off-by: Gioh Kim <gi-oh.kim@...fitbricks.com>
---
 fs/dcache.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/dcache.c b/fs/dcache.c
index 95d71eda8142..f7909447849e 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -2899,6 +2899,7 @@ static int __d_unalias(struct inode *inode,
 	struct mutex *m1 = NULL;
 	struct rw_semaphore *m2 = NULL;
 	int ret = -ESTALE;
+	struct dentry *alias_parent;
 
 	/* If alias and dentry share a parent, then no extra locks required */
 	if (alias->d_parent == dentry->d_parent)
@@ -2908,6 +2909,9 @@ static int __d_unalias(struct inode *inode,
 	if (!mutex_trylock(&dentry->d_sb->s_vfs_rename_mutex))
 		goto out_err;
 	m1 = &dentry->d_sb->s_vfs_rename_mutex;
+	alias_parent = dget_parent(alias->d_parent);
+	if (!alias_parent)
+		goto out_err;
 	if (!inode_trylock_shared(alias->d_parent->d_inode))
 		goto out_err;
 	m2 = &alias->d_parent->d_inode->i_rwsem;
@@ -2919,6 +2923,7 @@ static int __d_unalias(struct inode *inode,
 		up_read(m2);
 	if (m1)
 		mutex_unlock(m1);
+	dput(alias_parent);
 	return ret;
 }
 
-- 
2.11.0