linux-kernel - [RFC PATCH v2 2/2] fscrypt: enable RCU-walk path for .d

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

Message-ID: <20181106122052.18507-2-gaoxiang25@huawei.com>
Date:   Tue, 6 Nov 2018 20:20:52 +0800
From:   Gao Xiang <gaoxiang25@...wei.com>
To:     "Theodore Y . Ts'o" <tytso@....edu>,
        Jaegeuk Kim <jaegeuk@...nel.org>,
        Eric Biggers <ebiggers@...gle.com>
CC:     <linux-fscrypt@...r.kernel.org>, <linux-fsdevel@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, Chao Yu <yuchao0@...wei.com>,
        Miao Xie <miaoxie@...wei.com>, <weidu.du@...wei.com>,
        Gao Xiang <gaoxiang25@...wei.com>
Subject: [RFC PATCH v2 2/2] fscrypt: enable RCU-walk path for .d_revalidate

This patch attempts to enable RCU-walk for fscrypt.
It looks harmless at glance and could have better
performance than do ref-walk only.

Signed-off-by: Gao Xiang <gaoxiang25@...wei.com>
---

p.s.
  In my opinion, it is better to retry for the case of
  READ_ONCE(dentry->d_parent) != dir rather than just
  return 0; and then do real lookup for ref-walk path...
  It behaves much like a seqlock and I tend to avoid
  taking d_lock as well..
  Please kindly correct me if I am wrong... Thanks in advance.

Thanks,
Gao Xiang

 fs/crypto/crypto.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index b38c574f70ac..2cc26fe9c43d 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -319,20 +319,29 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 {
 	struct dentry *dir;
 	int dir_has_key, cached_with_key;
+	struct inode *dir_inode;
 
-	if (flags & LOOKUP_RCU)
-		return -ECHILD;
+repeat:
+	rcu_read_lock();
+	dir = READ_ONCE(dentry->d_parent);
 
-	dir = dget_parent(dentry);
-	if (!IS_ENCRYPTED(d_inode(dir))) {
-		dput(dir);
+	dir_inode = d_inode_rcu(dir);
+	if (!dir_inode || !IS_ENCRYPTED(dir_inode)) {
+		rcu_read_unlock();
 		return 0;
 	}
+	dir_has_key = (dir_inode->i_crypt_info != NULL);
+
+	/* original dir becomes invalid after sampling all? */
+	if (unlikely(__lockref_is_dead(&dir->d_lockref) ||
+		     READ_ONCE(dentry->d_parent) != dir)) {
+		rcu_read_unlock();
+		goto repeat;
+	}
+	rcu_read_unlock();
 
 	cached_with_key = READ_ONCE(dentry->d_flags) &
 		DCACHE_ENCRYPTED_WITH_KEY;
-	dir_has_key = (d_inode(dir)->i_crypt_info != NULL);
-	dput(dir);
 
 	/*
 	 * If the dentry was cached without the key, and it is a
-- 
2.14.4