[PATCH v2 0/4] ceph: dencrypt the dentry names early and once for readdir

[PATCH v2 0/4] ceph: dencrypt the dentry names early and once for readdir

[xiubli]

From: Xiubo Li <xiubli@redhat.com>

This is a new approach to improve the readdir and based the previous
discussion in another thread:

https://patchwork.kernel.org/project/ceph-devel/list/?series=621901

Just start a new thread for this.

As Jeff suggested, this patch series will dentrypt the dentry name
during parsing the readdir data in handle_reply(). And then in both
ceph_readdir_prepopulate() and ceph_readdir() we will use the
dencrypted name directly.

NOTE: we will base64_dencode and dencrypt the names in-place instead
of allocating tmp buffers. For base64_dencode it's safe because the
dencoded string buffer will always be shorter.


V2:
- Fix the WARN issue reported by Luis, thanks.


Xiubo Li (4):
  ceph: pass the request to parse_reply_info_readdir()
  ceph: add ceph_encode_encrypted_dname() helper
  ceph: dencrypt the dentry names early and once for readdir
  ceph: clean up the ceph_readdir() code

 fs/ceph/crypto.c     |  25 ++++++++---
 fs/ceph/crypto.h     |   2 +
 fs/ceph/dir.c        |  64 +++++++++------------------
 fs/ceph/inode.c      |  37 ++--------------
 fs/ceph/mds_client.c | 101 ++++++++++++++++++++++++++++++++++++-------
 fs/ceph/mds_client.h |   4 +-
 6 files changed, 133 insertions(+), 100 deletions(-)

-- 
2.27.0

[PATCH v2 1/4] ceph: pass the request to parse_reply_info_readdir()

[xiubli]

From: Xiubo Li <xiubli@redhat.com>

Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/mds_client.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 8d704ddd7291..c7113ce655a6 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -406,9 +406,10 @@ static int parse_reply_info_trace(void **p, void *end,
  * parse readdir results
  */
 static int parse_reply_info_readdir(void **p, void *end,
-				struct ceph_mds_reply_info_parsed *info,
-				u64 features)
+				    struct ceph_mds_request *req,
+				    u64 features)
 {
+	struct ceph_mds_reply_info_parsed *info = &req->r_reply_info;
 	u32 num, i = 0;
 	int err;
 
@@ -650,15 +651,16 @@ static int parse_reply_info_getvxattr(void **p, void *end,
  * parse extra results
  */
 static int parse_reply_info_extra(void **p, void *end,
-				  struct ceph_mds_reply_info_parsed *info,
+				  struct ceph_mds_request *req,
 				  u64 features, struct ceph_mds_session *s)
 {
+	struct ceph_mds_reply_info_parsed *info = &req->r_reply_info;
 	u32 op = le32_to_cpu(info->head->op);
 
 	if (op == CEPH_MDS_OP_GETFILELOCK)
 		return parse_reply_info_filelock(p, end, info, features);
 	else if (op == CEPH_MDS_OP_READDIR || op == CEPH_MDS_OP_LSSNAP)
-		return parse_reply_info_readdir(p, end, info, features);
+		return parse_reply_info_readdir(p, end, req, features);
 	else if (op == CEPH_MDS_OP_CREATE)
 		return parse_reply_info_create(p, end, info, features, s);
 	else if (op == CEPH_MDS_OP_GETVXATTR)
@@ -671,9 +673,9 @@ static int parse_reply_info_extra(void **p, void *end,
  * parse entire mds reply
  */
 static int parse_reply_info(struct ceph_mds_session *s, struct ceph_msg *msg,
-			    struct ceph_mds_reply_info_parsed *info,
-			    u64 features)
+			    struct ceph_mds_request *req, u64 features)
 {
+	struct ceph_mds_reply_info_parsed *info = &req->r_reply_info;
 	void *p, *end;
 	u32 len;
 	int err;
@@ -695,7 +697,7 @@ static int parse_reply_info(struct ceph_mds_session *s, struct ceph_msg *msg,
 	ceph_decode_32_safe(&p, end, len, bad);
 	if (len > 0) {
 		ceph_decode_need(&p, end, len, bad);
-		err = parse_reply_info_extra(&p, p+len, info, features, s);
+		err = parse_reply_info_extra(&p, p+len, req, features, s);
 		if (err < 0)
 			goto out_bad;
 	}
@@ -3426,14 +3428,14 @@ static void handle_reply(struct ceph_mds_session *session, struct ceph_msg *msg)
 	}
 
 	dout("handle_reply tid %lld result %d\n", tid, result);
-	rinfo = &req->r_reply_info;
 	if (test_bit(CEPHFS_FEATURE_REPLY_ENCODING, &session->s_features))
-		err = parse_reply_info(session, msg, rinfo, (u64)-1);
+		err = parse_reply_info(session, msg, req, (u64)-1);
 	else
-		err = parse_reply_info(session, msg, rinfo, session->s_con.peer_features);
+		err = parse_reply_info(session, msg, req, session->s_con.peer_features);
 	mutex_unlock(&mdsc->mutex);
 
 	/* Must find target inode outside of mutexes to avoid deadlocks */
+	rinfo = &req->r_reply_info;
 	if ((err >= 0) && rinfo->head->is_target) {
 		struct inode *in;
 		struct ceph_vino tvino = {
-- 
2.27.0

[PATCH v2 2/4] ceph: add ceph_encode_encrypted_dname() helper

[xiubli]

From: Xiubo Li <xiubli@redhat.com>

Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/crypto.c | 13 +++++++++----
 fs/ceph/crypto.h |  1 +
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index 560481b6c964..21bb0924bd2a 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -128,7 +128,7 @@ void ceph_fscrypt_as_ctx_to_req(struct ceph_mds_request *req, struct ceph_acl_se
 	swap(req->r_fscrypt_auth, as->fscrypt_auth);
 }
 
-int ceph_encode_encrypted_fname(const struct inode *parent, struct dentry *dentry, char *buf)
+int ceph_encode_encrypted_dname(const struct inode *parent, struct qstr *d_name, char *buf)
 {
 	u32 len;
 	int elen;
@@ -138,13 +138,13 @@ int ceph_encode_encrypted_fname(const struct inode *parent, struct dentry *dentr
 	WARN_ON_ONCE(!fscrypt_has_encryption_key(parent));
 
 	/*
-	 * convert cleartext dentry name to ciphertext
+	 * convert cleartext d_name to ciphertext
 	 * if result is longer than CEPH_NOKEY_NAME_MAX,
 	 * sha256 the remaining bytes
 	 *
 	 * See: fscrypt_setup_filename
 	 */
-	if (!fscrypt_fname_encrypted_size(parent, dentry->d_name.len, NAME_MAX, &len))
+	if (!fscrypt_fname_encrypted_size(parent, d_name->len, NAME_MAX, &len))
 		return -ENAMETOOLONG;
 
 	/* Allocate a buffer appropriate to hold the result */
@@ -152,7 +152,7 @@ int ceph_encode_encrypted_fname(const struct inode *parent, struct dentry *dentr
 	if (!cryptbuf)
 		return -ENOMEM;
 
-	ret = fscrypt_fname_encrypt(parent, &dentry->d_name, cryptbuf, len);
+	ret = fscrypt_fname_encrypt(parent, d_name, cryptbuf, len);
 	if (ret) {
 		kfree(cryptbuf);
 		return ret;
@@ -176,6 +176,11 @@ int ceph_encode_encrypted_fname(const struct inode *parent, struct dentry *dentr
 	return elen;
 }
 
+int ceph_encode_encrypted_fname(const struct inode *parent, struct dentry *dentry, char *buf)
+{
+	return ceph_encode_encrypted_dname(parent, &dentry->d_name, buf);
+}
+
 /**
  * ceph_fname_to_usr - convert a filename for userland presentation
  * @fname: ceph_fname to be converted
diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
index 1e08f8a64ad6..f462b96e119b 100644
--- a/fs/ceph/crypto.h
+++ b/fs/ceph/crypto.h
@@ -90,6 +90,7 @@ void ceph_fscrypt_free_dummy_policy(struct ceph_fs_client *fsc);
 int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
 				 struct ceph_acl_sec_ctx *as);
 void ceph_fscrypt_as_ctx_to_req(struct ceph_mds_request *req, struct ceph_acl_sec_ctx *as);
+int ceph_encode_encrypted_dname(const struct inode *parent, struct qstr *d_name, char *buf);
 int ceph_encode_encrypted_fname(const struct inode *parent, struct dentry *dentry, char *buf);
 
 static inline int ceph_fname_alloc_buffer(struct inode *parent, struct fscrypt_str *fname)
-- 
2.27.0

[PATCH v2 3/4] ceph: dencrypt the dentry names early and once for readdir

[xiubli]

From: Xiubo Li <xiubli@redhat.com>

This will dencrypt the dentry names when parsing readdir data.
And will always store the dencyrpted dentry names in the struct
ceph_mds_reply_dir_entry. The cryptext(altname) makes no sense
any more after this if it has and it will be overwrited by the
dencrypted dentry name.

Then in both ceph_readdir_prepopulate() and ceph_readdir() we
will use the dencrypted name directly.

Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/crypto.c     | 12 +++++--
 fs/ceph/crypto.h     |  1 +
 fs/ceph/dir.c        | 41 +++++++----------------
 fs/ceph/inode.c      | 37 +++------------------
 fs/ceph/mds_client.c | 79 ++++++++++++++++++++++++++++++++++++++++----
 fs/ceph/mds_client.h |  4 +--
 6 files changed, 102 insertions(+), 72 deletions(-)

diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index 21bb0924bd2a..c125a79019b3 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -135,7 +135,10 @@ int ceph_encode_encrypted_dname(const struct inode *parent, struct qstr *d_name,
 	int ret;
 	u8 *cryptbuf;
 
-	WARN_ON_ONCE(!fscrypt_has_encryption_key(parent));
+	if (!fscrypt_has_encryption_key(parent)) {
+		memcpy(buf, d_name->name, d_name->len);
+		return d_name->len;
+	}
 
 	/*
 	 * convert cleartext d_name to ciphertext
@@ -178,6 +181,8 @@ int ceph_encode_encrypted_dname(const struct inode *parent, struct qstr *d_name,
 
 int ceph_encode_encrypted_fname(const struct inode *parent, struct dentry *dentry, char *buf)
 {
+	WARN_ON_ONCE(!fscrypt_has_encryption_key(parent));
+
 	return ceph_encode_encrypted_dname(parent, &dentry->d_name, buf);
 }
 
@@ -222,7 +227,10 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
 	 * generating a nokey name via fscrypt.
 	 */
 	if (!fscrypt_has_encryption_key(fname->dir)) {
-		memcpy(oname->name, fname->name, fname->name_len);
+		if (fname->no_copy)
+			oname->name = fname->name;
+		else
+			memcpy(oname->name, fname->name, fname->name_len);
 		oname->len = fname->name_len;
 		if (is_nokey)
 			*is_nokey = true;
diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
index f462b96e119b..ee593b8ad07c 100644
--- a/fs/ceph/crypto.h
+++ b/fs/ceph/crypto.h
@@ -23,6 +23,7 @@ struct ceph_fname {
 	unsigned char	*ctext;		// binary crypttext (if any)
 	u32		name_len;	// length of name buffer
 	u32		ctext_len;	// length of crypttext
+	bool		no_copy;
 };
 
 /*
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 6df2a91af236..6f9af69b11c7 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -316,8 +316,6 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 	int err;
 	unsigned frag = -1;
 	struct ceph_mds_reply_info_parsed *rinfo;
-	struct fscrypt_str tname = FSTR_INIT(NULL, 0);
-	struct fscrypt_str oname = FSTR_INIT(NULL, 0);
 
 	dout("readdir %p file %p pos %llx\n", inode, file, ctx->pos);
 	if (dfi->file_info.flags & CEPH_F_ATEND)
@@ -347,7 +345,7 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 
 	err = fscrypt_prepare_readdir(inode);
 	if (err)
-		goto out;
+		return err;
 
 	spin_lock(&ci->i_ceph_lock);
 	/* request Fx cap. if have Fx, we don't need to release Fs cap
@@ -369,14 +367,6 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 		spin_unlock(&ci->i_ceph_lock);
 	}
 
-	err = ceph_fname_alloc_buffer(inode, &tname);
-	if (err < 0)
-		goto out;
-
-	err = ceph_fname_alloc_buffer(inode, &oname);
-	if (err < 0)
-		goto out;
-
 	/* proceed with a normal readdir */
 more:
 	/* do we have the correct frag content buffered? */
@@ -421,12 +411,21 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 			req->r_inode_drop = CEPH_CAP_FILE_EXCL;
 		}
 		if (dfi->last_name) {
-			req->r_path2 = kstrdup(dfi->last_name, GFP_KERNEL);
+			struct qstr d_name = { .name = dfi->last_name,
+					       .len = strlen(dfi->last_name) };
+
+			req->r_path2 = kzalloc(NAME_MAX + 1, GFP_KERNEL);
 			if (!req->r_path2) {
 				ceph_mdsc_put_request(req);
 				err = -ENOMEM;
 				goto out;
 			}
+
+			err = ceph_encode_encrypted_dname(inode, &d_name, req->r_path2);
+			if (err < 0) {
+				ceph_mdsc_put_request(req);
+				goto out;
+			}
 		} else if (is_hash_order(ctx->pos)) {
 			req->r_args.readdir.offset_hash =
 				cpu_to_le32(fpos_hash(ctx->pos));
@@ -530,19 +529,6 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 	}
 	for (; i < rinfo->dir_nr; i++) {
 		struct ceph_mds_reply_dir_entry *rde = rinfo->dir_entries + i;
-		struct ceph_fname fname = { .dir	= inode,
-					    .name	= rde->name,
-					    .name_len	= rde->name_len,
-					    .ctext	= rde->altname,
-					    .ctext_len	= rde->altname_len };
-		u32 olen = oname.len;
-
-		err = ceph_fname_to_usr(&fname, &tname, &oname, NULL);
-		if (err) {
-			pr_err("%s unable to decode %.*s, got %d\n", __func__,
-			       rde->name_len, rde->name, err);
-			goto out;
-		}
 
 		BUG_ON(rde->offset < ctx->pos);
 		BUG_ON(!rde->inode.in);
@@ -552,7 +538,7 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 		     i, rinfo->dir_nr, ctx->pos,
 		     rde->name_len, rde->name, &rde->inode.in);
 
-		if (!dir_emit(ctx, oname.name, oname.len,
+		if (!dir_emit(ctx, rde->name, rde->name_len,
 			      ceph_present_ino(inode->i_sb, le64_to_cpu(rde->inode.in->ino)),
 			      le32_to_cpu(rde->inode.in->mode) >> 12)) {
 			/*
@@ -567,7 +553,6 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 		}
 
 		/* Reset the lengths to their original allocated vals */
-		oname.len = olen;
 		ctx->pos++;
 	}
 
@@ -625,8 +610,6 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 	err = 0;
 	dout("readdir %p file %p done.\n", inode, file);
 out:
-	ceph_fname_free_buffer(inode, &tname);
-	ceph_fname_free_buffer(inode, &oname);
 	return err;
 }
 
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index b573a0f33450..7b670e2405c1 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -1829,8 +1829,6 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req,
 	u32 last_hash = 0;
 	u32 fpos_offset;
 	struct ceph_readdir_cache_control cache_ctl = {};
-	struct fscrypt_str tname = FSTR_INIT(NULL, 0);
-	struct fscrypt_str oname = FSTR_INIT(NULL, 0);
 
 	if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags))
 		return readdir_prepopulate_inodes_only(req, session);
@@ -1882,45 +1880,20 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req,
 	cache_ctl.index = req->r_readdir_cache_idx;
 	fpos_offset = req->r_readdir_offset;
 
-	err = ceph_fname_alloc_buffer(inode, &tname);
-	if (err < 0)
-		goto out;
-
-	err = ceph_fname_alloc_buffer(inode, &oname);
-	if (err < 0)
-		goto out;
-
 	/* FIXME: release caps/leases if error occurs */
 	for (i = 0; i < rinfo->dir_nr; i++) {
-		bool is_nokey = false;
 		struct ceph_mds_reply_dir_entry *rde = rinfo->dir_entries + i;
 		struct ceph_vino tvino;
-		u32 olen = oname.len;
-		struct ceph_fname fname = { .dir	= inode,
-					    .name	= rde->name,
-					    .name_len	= rde->name_len,
-					    .ctext	= rde->altname,
-					    .ctext_len	= rde->altname_len };
-
-		err = ceph_fname_to_usr(&fname, &tname, &oname, &is_nokey);
-		if (err) {
-			pr_err("%s unable to decode %.*s, got %d\n", __func__,
-			       rde->name_len, rde->name, err);
-			goto out;
-		}
 
-		dname.name = oname.name;
-		dname.len = oname.len;
+		dname.name = rde->name;
+		dname.len = rde->name_len;
 		dname.hash = full_name_hash(parent, dname.name, dname.len);
-		oname.len = olen;
 
 		tvino.ino = le64_to_cpu(rde->inode.in->ino);
 		tvino.snap = le64_to_cpu(rde->inode.in->snapid);
 
 		if (rinfo->hash_order) {
-			u32 hash = ceph_str_hash(ci->i_dir_layout.dl_dir_hash,
-						 rde->name, rde->name_len);
-			hash = ceph_frag_value(hash);
+			u32 hash = ceph_frag_value(rde->raw_hash);
 			if (hash != last_hash)
 				fpos_offset = 2;
 			last_hash = hash;
@@ -1943,7 +1916,7 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req,
 				err = -ENOMEM;
 				goto out;
 			}
-			if (is_nokey) {
+			if (rde->is_nokey) {
 				spin_lock(&dn->d_lock);
 				dn->d_flags |= DCACHE_NOKEY_NAME;
 				spin_unlock(&dn->d_lock);
@@ -2036,8 +2009,6 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req,
 		req->r_readdir_cache_idx = cache_ctl.index;
 	}
 	ceph_readdir_cache_release(&cache_ctl);
-	ceph_fname_free_buffer(inode, &tname);
-	ceph_fname_free_buffer(inode, &oname);
 	dout("readdir_prepopulate done\n");
 	return err;
 }
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index c7113ce655a6..c51b07ec72cf 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -439,20 +439,87 @@ static int parse_reply_info_readdir(void **p, void *end,
 
 	info->dir_nr = num;
 	while (num) {
+		struct inode *inode = d_inode(req->r_dentry);
+		struct ceph_inode_info *ci = ceph_inode(inode);
 		struct ceph_mds_reply_dir_entry *rde = info->dir_entries + i;
+		struct fscrypt_str tname = FSTR_INIT(NULL, 0);
+		struct fscrypt_str oname = FSTR_INIT(NULL, 0);
+		struct ceph_fname fname;
+		u32 altname_len, _name_len;
+		u8 *altname, *_name;
+
 		/* dentry */
-		ceph_decode_32_safe(p, end, rde->name_len, bad);
-		ceph_decode_need(p, end, rde->name_len, bad);
-		rde->name = *p;
-		*p += rde->name_len;
-		dout("parsed dir dname '%.*s'\n", rde->name_len, rde->name);
+		ceph_decode_32_safe(p, end, _name_len, bad);
+		ceph_decode_need(p, end, _name_len, bad);
+		_name = *p;
+		*p += _name_len;
+		dout("parsed dir dname '%.*s'\n", _name_len, _name);
+
+		if (info->hash_order)
+			rde->raw_hash = ceph_str_hash(ci->i_dir_layout.dl_dir_hash,
+						      _name, _name_len);
 
 		/* dentry lease */
 		err = parse_reply_info_lease(p, end, &rde->lease, features,
-					     &rde->altname_len, &rde->altname);
+					     &altname_len, &altname);
 		if (err)
 			goto out_bad;
 
+		/*
+		 * Try to dencrypt the dentry names and update them
+		 * in the ceph_mds_reply_dir_entry struct.
+		 */
+		fname.dir = inode;
+		fname.name = _name;
+		fname.name_len = _name_len;
+		fname.ctext = altname;
+		fname.ctext_len = altname_len;
+		/*
+		 * The _name_len maybe larger than altname_len, such as
+		 * when the human readable name length is in range of
+		 * (CEPH_NOHASH_NAME_MAX, CEPH_NOHASH_NAME_MAX + SHA256_DIGEST_SIZE),
+		 * then the copy in ceph_fname_to_usr will corrupt the
+		 * data if there has no encryption key.
+		 *
+		 * Just set the no_copy flag and then if there has no
+		 * encryption key the oname.name will be assigned to
+		 * _name always.
+		 */
+		fname.no_copy = true;
+		if (altname_len == 0) {
+			/*
+			 * Set tname to _name, and this will be used
+			 * to do the base64_decode in-place. It's
+			 * safe because the decoded string should
+			 * always be shorter, which is 3/4 of origin
+			 * string.
+			 */
+			tname.name = _name;
+
+			/*
+			 * Set oname to _name too, and this will be
+			 * used to do the dencryption in-place.
+			 */
+			oname.name = _name;
+			oname.len = _name_len;
+		} else {
+			/*
+			 * This will do the decryption only in-place
+			 * from altname cryptext directly.
+			 */
+			oname.name = altname;
+			oname.len = altname_len;
+		}
+		rde->is_nokey = false;
+		err = ceph_fname_to_usr(&fname, &tname, &oname, &rde->is_nokey);
+		if (err) {
+			pr_err("%s unable to decode %.*s, got %d\n", __func__,
+			       _name_len, _name, err);
+			goto out_bad;
+		}
+		rde->name = oname.name;
+		rde->name_len = oname.len;
+
 		/* inode */
 		err = parse_reply_info_in(p, end, &rde->inode, features);
 		if (err < 0)
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index 0dfe24f94567..e297bf98c39f 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -96,10 +96,10 @@ struct ceph_mds_reply_info_in {
 };
 
 struct ceph_mds_reply_dir_entry {
+	bool			      is_nokey;
 	char                          *name;
-	u8			      *altname;
 	u32                           name_len;
-	u32			      altname_len;
+	u32			      raw_hash;
 	struct ceph_mds_reply_lease   *lease;
 	struct ceph_mds_reply_info_in inode;
 	loff_t			      offset;
-- 
2.27.0

[PATCH v2 4/4] ceph: clean up the ceph_readdir() code

[xiubli]

From: Xiubo Li <xiubli@redhat.com>

Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/dir.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 6f9af69b11c7..5ae5cb778389 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -394,14 +394,13 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 		dout("readdir fetching %llx.%llx frag %x offset '%s'\n",
 		     ceph_vinop(inode), frag, dfi->last_name);
 		req = ceph_mdsc_create_request(mdsc, op, USE_AUTH_MDS);
-		if (IS_ERR(req)) {
-			err = PTR_ERR(req);
-			goto out;
-		}
+		if (IS_ERR(req))
+			return PTR_ERR(req);
+
 		err = ceph_alloc_readdir_reply_buffer(req, inode);
 		if (err) {
 			ceph_mdsc_put_request(req);
-			goto out;
+			return err;
 		}
 		/* hints to request -> mds selection code */
 		req->r_direct_mode = USE_AUTH_MDS;
@@ -417,14 +416,13 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 			req->r_path2 = kzalloc(NAME_MAX + 1, GFP_KERNEL);
 			if (!req->r_path2) {
 				ceph_mdsc_put_request(req);
-				err = -ENOMEM;
-				goto out;
+				return -ENOMEM;
 			}
 
 			err = ceph_encode_encrypted_dname(inode, &d_name, req->r_path2);
 			if (err < 0) {
 				ceph_mdsc_put_request(req);
-				goto out;
+				return err;
 			}
 		} else if (is_hash_order(ctx->pos)) {
 			req->r_args.readdir.offset_hash =
@@ -445,7 +443,7 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 		err = ceph_mdsc_do_request(mdsc, NULL, req);
 		if (err < 0) {
 			ceph_mdsc_put_request(req);
-			goto out;
+			return err;
 		}
 		dout("readdir got and parsed readdir result=%d on "
 		     "frag %x, end=%d, complete=%d, hash_order=%d\n",
@@ -500,7 +498,7 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 			if (err) {
 				ceph_mdsc_put_request(dfi->last_readdir);
 				dfi->last_readdir = NULL;
-				goto out;
+				return err;
 			}
 		} else if (req->r_reply_info.dir_end) {
 			dfi->next_offset = 2;
@@ -548,8 +546,7 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 			 * it will continue.
 			 */
 			dout("filldir stopping us...\n");
-			err = 0;
-			goto out;
+			return 0;
 		}
 
 		/* Reset the lengths to their original allocated vals */
@@ -607,10 +604,8 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 					dfi->dir_ordered_count);
 		spin_unlock(&ci->i_ceph_lock);
 	}
-	err = 0;
 	dout("readdir %p file %p done.\n", inode, file);
-out:
-	return err;
+	return 0;
 }
 
 static void reset_readdir(struct ceph_dir_file_info *dfi)
-- 
2.27.0

Re: [PATCH v2 0/4] ceph: dencrypt the dentry names early and once for readdir

[Jeff Layton]

On Mon, 2022-03-14 at 10:28 +0800, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
> 
> This is a new approach to improve the readdir and based the previous
> discussion in another thread:
> 
> https://patchwork.kernel.org/project/ceph-devel/list/?series=621901
> 
> Just start a new thread for this.
> 
> As Jeff suggested, this patch series will dentrypt the dentry name
> during parsing the readdir data in handle_reply(). And then in both
> ceph_readdir_prepopulate() and ceph_readdir() we will use the
> dencrypted name directly.
> 
> NOTE: we will base64_dencode and dencrypt the names in-place instead
> of allocating tmp buffers. For base64_dencode it's safe because the
> dencoded string buffer will always be shorter.
> 
> 
> V2:
> - Fix the WARN issue reported by Luis, thanks.
> 
> 
> Xiubo Li (4):
>   ceph: pass the request to parse_reply_info_readdir()
>   ceph: add ceph_encode_encrypted_dname() helper
>   ceph: dencrypt the dentry names early and once for readdir
>   ceph: clean up the ceph_readdir() code
> 
>  fs/ceph/crypto.c     |  25 ++++++++---
>  fs/ceph/crypto.h     |   2 +
>  fs/ceph/dir.c        |  64 +++++++++------------------
>  fs/ceph/inode.c      |  37 ++--------------
>  fs/ceph/mds_client.c | 101 ++++++++++++++++++++++++++++++++++++-------
>  fs/ceph/mds_client.h |   4 +-
>  6 files changed, 133 insertions(+), 100 deletions(-)
> 

This looks good, Xiubo. I did some testing with these earlier and they
seemed to work great.

I've gone ahead and merged these into the wip-fscrypt branch. It may be
best to eventually squash these down, but it's probably fine to leave
them on top as well.

Thanks!  
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2 0/4] ceph: dencrypt the dentry names early and once for readdir

[Xiubo Li]

On 3/15/22 2:38 AM, Jeff Layton wrote:
> On Mon, 2022-03-14 at 10:28 +0800, xiubli@redhat.com wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> This is a new approach to improve the readdir and based the previous
>> discussion in another thread:
>>
>> https://patchwork.kernel.org/project/ceph-devel/list/?series=621901
>>
>> Just start a new thread for this.
>>
>> As Jeff suggested, this patch series will dentrypt the dentry name
>> during parsing the readdir data in handle_reply(). And then in both
>> ceph_readdir_prepopulate() and ceph_readdir() we will use the
>> dencrypted name directly.
>>
>> NOTE: we will base64_dencode and dencrypt the names in-place instead
>> of allocating tmp buffers. For base64_dencode it's safe because the
>> dencoded string buffer will always be shorter.
>>
>>
>> V2:
>> - Fix the WARN issue reported by Luis, thanks.
>>
>>
>> Xiubo Li (4):
>>    ceph: pass the request to parse_reply_info_readdir()
>>    ceph: add ceph_encode_encrypted_dname() helper
>>    ceph: dencrypt the dentry names early and once for readdir
>>    ceph: clean up the ceph_readdir() code
>>
>>   fs/ceph/crypto.c     |  25 ++++++++---
>>   fs/ceph/crypto.h     |   2 +
>>   fs/ceph/dir.c        |  64 +++++++++------------------
>>   fs/ceph/inode.c      |  37 ++--------------
>>   fs/ceph/mds_client.c | 101 ++++++++++++++++++++++++++++++++++++-------
>>   fs/ceph/mds_client.h |   4 +-
>>   6 files changed, 133 insertions(+), 100 deletions(-)
>>
> This looks good, Xiubo. I did some testing with these earlier and they
> seemed to work great.
>
> I've gone ahead and merged these into the wip-fscrypt branch. It may be
> best to eventually squash these down, but it's probably fine to leave
> them on top as well.

Sure Jeff, thanks.

- Xiubo

> Thanks!