* [PATCH 1/4] ceph: pass the request to parse_reply_info_readdir()
2022-03-11 10:45 [PATCH 0/4] ceph: dencrypt the dentry names early and once for readdir xiubli
@ 2022-03-11 10:45 ` xiubli
2022-03-11 10:45 ` [PATCH 2/4] ceph: add ceph_encode_encrypted_dname() helper xiubli
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: xiubli @ 2022-03-11 10:45 UTC (permalink / raw)
To: jlayton; +Cc: idryomov, vshankar, lhenriques, ceph-devel, Xiubo Li
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/4] ceph: add ceph_encode_encrypted_dname() helper
2022-03-11 10:45 [PATCH 0/4] ceph: dencrypt the dentry names early and once for readdir xiubli
2022-03-11 10:45 ` [PATCH 1/4] ceph: pass the request to parse_reply_info_readdir() xiubli
@ 2022-03-11 10:45 ` xiubli
2022-03-11 10:45 ` [PATCH 3/4] ceph: dencrypt the dentry names early and once for readdir xiubli
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: xiubli @ 2022-03-11 10:45 UTC (permalink / raw)
To: jlayton; +Cc: idryomov, vshankar, lhenriques, ceph-devel, Xiubo Li
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/4] ceph: dencrypt the dentry names early and once for readdir
2022-03-11 10:45 [PATCH 0/4] ceph: dencrypt the dentry names early and once for readdir xiubli
2022-03-11 10:45 ` [PATCH 1/4] ceph: pass the request to parse_reply_info_readdir() xiubli
2022-03-11 10:45 ` [PATCH 2/4] ceph: add ceph_encode_encrypted_dname() helper xiubli
@ 2022-03-11 10:45 ` xiubli
2022-03-11 10:45 ` [PATCH 4/4] ceph: clean up the ceph_readdir() code xiubli
2022-03-11 16:02 ` [PATCH 0/4] ceph: dencrypt the dentry names early and once for readdir Luís Henriques
4 siblings, 0 replies; 7+ messages in thread
From: xiubli @ 2022-03-11 10:45 UTC (permalink / raw)
To: jlayton; +Cc: idryomov, vshankar, lhenriques, ceph-devel, Xiubo Li
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 | 5 ++-
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, 96 insertions(+), 71 deletions(-)
diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index 21bb0924bd2a..b5afeb881d9a 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -222,7 +222,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..d006ad675c49 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, 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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 4/4] ceph: clean up the ceph_readdir() code
2022-03-11 10:45 [PATCH 0/4] ceph: dencrypt the dentry names early and once for readdir xiubli
` (2 preceding siblings ...)
2022-03-11 10:45 ` [PATCH 3/4] ceph: dencrypt the dentry names early and once for readdir xiubli
@ 2022-03-11 10:45 ` xiubli
2022-03-11 16:02 ` [PATCH 0/4] ceph: dencrypt the dentry names early and once for readdir Luís Henriques
4 siblings, 0 replies; 7+ messages in thread
From: xiubli @ 2022-03-11 10:45 UTC (permalink / raw)
To: jlayton; +Cc: idryomov, vshankar, lhenriques, ceph-devel, Xiubo Li
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 d006ad675c49..959756d44597 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, 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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 0/4] ceph: dencrypt the dentry names early and once for readdir
2022-03-11 10:45 [PATCH 0/4] ceph: dencrypt the dentry names early and once for readdir xiubli
` (3 preceding siblings ...)
2022-03-11 10:45 ` [PATCH 4/4] ceph: clean up the ceph_readdir() code xiubli
@ 2022-03-11 16:02 ` Luís Henriques
2022-03-12 0:21 ` Xiubo Li
4 siblings, 1 reply; 7+ messages in thread
From: Luís Henriques @ 2022-03-11 16:02 UTC (permalink / raw)
To: xiubli; +Cc: jlayton, idryomov, vshankar, ceph-devel
Hi Xiubo,
I've started reviewing this patchset but, while running some quick tests,
I've seen found a bug. I was testing with fstests, but I can easily
reproduce it simply using 'src/dirhash_collide', from fstests:
- mount filesystem and create an encrypted directory
- run that application on the encrypted (unlocked) directory:
# cd mydir
# $FSTESTSDIR/src/dirhash_collide -d -n 10000 .
- umount filesystem and mount it again
- do an 'ls' in 'mydir' and I get:
[ 115.807181] ------------[ cut here ]------------
[ 115.807865] WARNING: CPU: 0 PID: 220 at fs/ceph/crypto.c:138 ceph_encode_encrypted_dname+0x1e6/0x240 [ceph]
[ 115.809298] Modules linked in: ceph libceph
[ 115.809881] CPU: 0 PID: 220 Comm: ls Not tainted 5.17.0-rc6+ #91
[ 115.810720] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b-rebuilt.opensuse.org 04/01/2014
[ 115.812298] RIP: 0010:ceph_encode_encrypted_dname+0x1e6/0x240 [ceph]
[ 115.813238] Code: 48 8b 44 24 50 48 89 85 ad 00 00 00 48 8b 44 24 58 48 89 85 b5 00 00 00 e9 2f ff ff ff 48 89 ef e8 6f dd 17 e1 e9 45 ff ff ff <0f> 0b e9 a2 fe ff ff 417
[ 115.815800] RSP: 0018:ffffc90000487bc0 EFLAGS: 00010246
[ 115.816519] RAX: 0000000000000000 RBX: 1ffff92000090f78 RCX: ffffffffa014654e
[ 115.817502] RDX: dffffc0000000000 RSI: ffffc90000487d58 RDI: ffff8880791b3230
[ 115.818466] RBP: ffffc90000487e90 R08: 0000000000000007 R09: ffff8880605d8748
[ 115.819445] R10: fffffbfff05a79b3 R11: 0000000000000001 R12: ffff8880791b2fe8
[ 115.820437] R13: ffff88807f88ae00 R14: ffffc90000487d58 R15: 0000000000000000
[ 115.821424] FS: 00007fe454541800(0000) GS:ffff888060c00000(0000) knlGS:0000000000000000
[ 115.822526] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 115.823308] CR2: 00007fe454530058 CR3: 000000000d6a0000 CR4: 00000000000006b0
[ 115.824296] Call Trace:
[ 115.824646] <TASK>
[ 115.824963] ? ceph_fscrypt_as_ctx_to_req+0x40/0x40 [ceph]
[ 115.825774] ? create_object.isra.0+0x34a/0x4b0
[ 115.826418] ? preempt_count_sub+0x18/0xc0
[ 115.827069] ? _raw_spin_unlock_irqrestore+0x28/0x40
[ 115.827815] ceph_readdir+0xe24/0x1fa0 [ceph]
[ 115.828459] ? preempt_count_sub+0x18/0xc0
[ 115.829028] ? preempt_count_sub+0x18/0xc0
[ 115.829604] ? ceph_d_revalidate+0x970/0x970 [ceph]
[ 115.830314] ? down_write_killable+0xb2/0x110
[ 115.830921] ? __down_killable+0x200/0x200
[ 115.831488] iterate_dir+0xe9/0x2a0
[ 115.831995] __x64_sys_getdents64+0xf2/0x1c0
[ 115.832587] ? __x64_sys_getdents+0x1c0/0x1c0
[ 115.833203] ? handle_mm_fault+0x1c3/0x230
[ 115.833778] ? compat_fillonedir+0x1b0/0x1b0
[ 115.834419] ? do_user_addr_fault+0x32b/0x940
[ 115.835048] do_syscall_64+0x43/0x90
[ 115.835575] entry_SYSCALL_64_after_hwframe+0x44/0xae
After this, I also see some KASAN NULL pointers, but I assume it's the
result of the above bug.
Cheers,
--
Luís
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 0/4] ceph: dencrypt the dentry names early and once for readdir
2022-03-11 16:02 ` [PATCH 0/4] ceph: dencrypt the dentry names early and once for readdir Luís Henriques
@ 2022-03-12 0:21 ` Xiubo Li
0 siblings, 0 replies; 7+ messages in thread
From: Xiubo Li @ 2022-03-12 0:21 UTC (permalink / raw)
To: Luís Henriques; +Cc: jlayton, idryomov, vshankar, ceph-devel
On 3/12/22 12:02 AM, Luís Henriques wrote:
> Hi Xiubo,
>
> I've started reviewing this patchset but, while running some quick tests,
> I've seen found a bug. I was testing with fstests, but I can easily
> reproduce it simply using 'src/dirhash_collide', from fstests:
>
> - mount filesystem and create an encrypted directory
>
> - run that application on the encrypted (unlocked) directory:
> # cd mydir
> # $FSTESTSDIR/src/dirhash_collide -d -n 10000 .
>
> - umount filesystem and mount it again
>
> - do an 'ls' in 'mydir' and I get:
>
> [ 115.807181] ------------[ cut here ]------------
> [ 115.807865] WARNING: CPU: 0 PID: 220 at fs/ceph/crypto.c:138 ceph_encode_encrypted_dname+0x1e6/0x240 [ceph]
> [ 115.809298] Modules linked in: ceph libceph
> [ 115.809881] CPU: 0 PID: 220 Comm: ls Not tainted 5.17.0-rc6+ #91
> [ 115.810720] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b-rebuilt.opensuse.org 04/01/2014
> [ 115.812298] RIP: 0010:ceph_encode_encrypted_dname+0x1e6/0x240 [ceph]
> [ 115.813238] Code: 48 8b 44 24 50 48 89 85 ad 00 00 00 48 8b 44 24 58 48 89 85 b5 00 00 00 e9 2f ff ff ff 48 89 ef e8 6f dd 17 e1 e9 45 ff ff ff <0f> 0b e9 a2 fe ff ff 417
> [ 115.815800] RSP: 0018:ffffc90000487bc0 EFLAGS: 00010246
> [ 115.816519] RAX: 0000000000000000 RBX: 1ffff92000090f78 RCX: ffffffffa014654e
> [ 115.817502] RDX: dffffc0000000000 RSI: ffffc90000487d58 RDI: ffff8880791b3230
> [ 115.818466] RBP: ffffc90000487e90 R08: 0000000000000007 R09: ffff8880605d8748
> [ 115.819445] R10: fffffbfff05a79b3 R11: 0000000000000001 R12: ffff8880791b2fe8
> [ 115.820437] R13: ffff88807f88ae00 R14: ffffc90000487d58 R15: 0000000000000000
> [ 115.821424] FS: 00007fe454541800(0000) GS:ffff888060c00000(0000) knlGS:0000000000000000
> [ 115.822526] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 115.823308] CR2: 00007fe454530058 CR3: 000000000d6a0000 CR4: 00000000000006b0
> [ 115.824296] Call Trace:
> [ 115.824646] <TASK>
> [ 115.824963] ? ceph_fscrypt_as_ctx_to_req+0x40/0x40 [ceph]
> [ 115.825774] ? create_object.isra.0+0x34a/0x4b0
> [ 115.826418] ? preempt_count_sub+0x18/0xc0
> [ 115.827069] ? _raw_spin_unlock_irqrestore+0x28/0x40
> [ 115.827815] ceph_readdir+0xe24/0x1fa0 [ceph]
> [ 115.828459] ? preempt_count_sub+0x18/0xc0
> [ 115.829028] ? preempt_count_sub+0x18/0xc0
> [ 115.829604] ? ceph_d_revalidate+0x970/0x970 [ceph]
> [ 115.830314] ? down_write_killable+0xb2/0x110
> [ 115.830921] ? __down_killable+0x200/0x200
> [ 115.831488] iterate_dir+0xe9/0x2a0
> [ 115.831995] __x64_sys_getdents64+0xf2/0x1c0
> [ 115.832587] ? __x64_sys_getdents+0x1c0/0x1c0
> [ 115.833203] ? handle_mm_fault+0x1c3/0x230
> [ 115.833778] ? compat_fillonedir+0x1b0/0x1b0
> [ 115.834419] ? do_user_addr_fault+0x32b/0x940
> [ 115.835048] do_syscall_64+0x43/0x90
> [ 115.835575] entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> After this, I also see some KASAN NULL pointers, but I assume it's the
> result of the above bug.
Hi Luis,
Thanks for your feedback.
Yesterday I have test this by using both the fsstress and xfstests for 1
hour, didn't see this call trace.
I will check it.
- XIubo
> Cheers,
^ permalink raw reply [flat|nested] 7+ messages in thread