Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1683183 > unrolled thread

[RFC] guarantee inode of alias's parent

Back to article view | Back to linux.kernel

Contents

  [RFC] guarantee inode of alias's parent Gioh Kim <gi-oh.kim@profitbricks.com> - 2017-07-07 15:30 +0200
    Re: [RFC] guarantee inode of alias's parent Al Viro <viro@ZenIV.linux.org.uk> - 2017-07-07 16:20 +0200

#1683183 — [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@profitbricks.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

[toc] | [next] | [standalone]

#1683216

On Fri, Jul 07, 2017 at 03:28:11PM +0200, Gioh Kim wrote:

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

Nowhere.  Why the hell would you want to lock them?

> What will happend if alias->d_parent be deleted by another process?

Huh?  As soon as you've grabbed ->s_vfs_rename_mutex, ->d_parent of everything on
that filesystem is stable - and positive.  Moreover, any positive dentry with
references held (in particular, anyone's parent) is going to remain such for
as long as those references are held.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web