diff mbox series

ceph: add support for encrypted snapshot names

Message ID 20230906134711.4707-1-lhenriques@suse.de
State New
Headers show
Series ceph: add support for encrypted snapshot names | expand

Commit Message

Luis Henriques Sept. 6, 2023, 1:47 p.m. UTC
Since filenames in encrypted directories are encrypted and shown as
a base64-encoded string when the directory is locked, make snapshot
names show a similar behaviour.

When creating a snapshot, .snap directories for every subdirectory will
show the snapshot name in the "long format":

  # mkdir .snap/my-snap
  # ls my-dir/.snap/
  _my-snap_1099511627782

Encrypted snapshots will need to be able to handle these by
encrypting/decrypting only the snapshot part of the string ('my-snap').

Also, since the MDS prevents snapshot names to be bigger than 240
characters it is necessary to adapt CEPH_NOHASH_NAME_MAX to accommodate
this extra limitation.

[ idryomov: drop const on !CONFIG_FS_ENCRYPTION branch too ]

Cc: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Luís Henriques <lhenriques@suse.de>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Xiubo Li <xiubli@redhat.com>
Reviewed-by: Milind Changire <mchangir@redhat.com>
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---
As reported by Dan Carpenter, the initial version of this patch wasn't
dealing correctly with the ceph_get_inode() return value: it never returns
NULL.  Simply dropped the code that handled the case where a NULL was
being returned.

Xiubo, can you please pick this new revision, if you think it's correct?

And thanks, Dan!

Cheers,
--
Luís

 fs/ceph/crypto.c | 195 +++++++++++++++++++++++++++++++++++++++--------
 fs/ceph/crypto.h |  12 +--
 fs/ceph/inode.c  |  33 +++++++-
 3 files changed, 199 insertions(+), 41 deletions(-)

Comments

Xiubo Li Sept. 7, 2023, 12:25 a.m. UTC | #1
On 9/6/23 21:47, Luís Henriques wrote:
> Since filenames in encrypted directories are encrypted and shown as
> a base64-encoded string when the directory is locked, make snapshot
> names show a similar behaviour.
>
> When creating a snapshot, .snap directories for every subdirectory will
> show the snapshot name in the "long format":
>
>    # mkdir .snap/my-snap
>    # ls my-dir/.snap/
>    _my-snap_1099511627782
>
> Encrypted snapshots will need to be able to handle these by
> encrypting/decrypting only the snapshot part of the string ('my-snap').
>
> Also, since the MDS prevents snapshot names to be bigger than 240
> characters it is necessary to adapt CEPH_NOHASH_NAME_MAX to accommodate
> this extra limitation.
>
> [ idryomov: drop const on !CONFIG_FS_ENCRYPTION branch too ]
>
> Cc: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Luís Henriques <lhenriques@suse.de>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
> Reviewed-by: Xiubo Li <xiubli@redhat.com>
> Reviewed-by: Milind Changire <mchangir@redhat.com>
> Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
> ---
> As reported by Dan Carpenter, the initial version of this patch wasn't
> dealing correctly with the ceph_get_inode() return value: it never returns
> NULL.  Simply dropped the code that handled the case where a NULL was
> being returned.
>
> Xiubo, can you please pick this new revision, if you think it's correct?

Hi Luis,

Please generate a new patch to fix this, the fscrypt corresponding 
commits have been merged already.

Thanks

- Xiubo

> And thanks, Dan!
>
> Cheers,
> --
> Luís
>
>   fs/ceph/crypto.c | 195 +++++++++++++++++++++++++++++++++++++++--------
>   fs/ceph/crypto.h |  12 +--
>   fs/ceph/inode.c  |  33 +++++++-
>   3 files changed, 199 insertions(+), 41 deletions(-)
>
> diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
> index 9494bf485f6a..7d0b9b5ccfc6 100644
> --- a/fs/ceph/crypto.c
> +++ b/fs/ceph/crypto.c
> @@ -193,17 +193,99 @@ void ceph_fscrypt_as_ctx_to_req(struct ceph_mds_request *req,
>   	swap(req->r_fscrypt_auth, as->fscrypt_auth);
>   }
>   
> -int ceph_encode_encrypted_dname(const struct inode *parent,
> -				struct qstr *d_name, char *buf)
> +/*
> + * User-created snapshots can't start with '_'.  Snapshots that start with this
> + * character are special (hint: there aren't real snapshots) and use the
> + * following format:
> + *
> + *   _<SNAPSHOT-NAME>_<INODE-NUMBER>
> + *
> + * where:
> + *  - <SNAPSHOT-NAME> - the real snapshot name that may need to be decrypted,
> + *  - <INODE-NUMBER> - the inode number (in decimal) for the actual snapshot
> + *
> + * This function parses these snapshot names and returns the inode
> + * <INODE-NUMBER>.  'name_len' will also bet set with the <SNAPSHOT-NAME>
> + * length.
> + */
> +static struct inode *parse_longname(const struct inode *parent,
> +				    const char *name, int *name_len)
>   {
> +	struct inode *dir = NULL;
> +	struct ceph_vino vino = { .snap = CEPH_NOSNAP };
> +	char *inode_number;
> +	char *name_end;
> +	int orig_len = *name_len;
> +	int ret = -EIO;
> +
> +	/* Skip initial '_' */
> +	name++;
> +	name_end = strrchr(name, '_');
> +	if (!name_end) {
> +		dout("Failed to parse long snapshot name: %s\n", name);
> +		return ERR_PTR(-EIO);
> +	}
> +	*name_len = (name_end - name);
> +	if (*name_len <= 0) {
> +		pr_err("Failed to parse long snapshot name\n");
> +		return ERR_PTR(-EIO);
> +	}
> +
> +	/* Get the inode number */
> +	inode_number = kmemdup_nul(name_end + 1,
> +				   orig_len - *name_len - 2,
> +				   GFP_KERNEL);
> +	if (!inode_number)
> +		return ERR_PTR(-ENOMEM);
> +	ret = kstrtou64(inode_number, 10, &vino.ino);
> +	if (ret) {
> +		dout("Failed to parse inode number: %s\n", name);
> +		dir = ERR_PTR(ret);
> +		goto out;
> +	}
> +
> +	/* And finally the inode */
> +	dir = ceph_find_inode(parent->i_sb, vino);
> +	if (!dir) {
> +		/* This can happen if we're not mounting cephfs on the root */
> +		dir = ceph_get_inode(parent->i_sb, vino, NULL);
> +	}
> +	if (IS_ERR(dir))
> +		dout("Can't find inode %s (%s)\n", inode_number, name);
> +
> +out:
> +	kfree(inode_number);
> +	return dir;
> +}
> +
> +int ceph_encode_encrypted_dname(struct inode *parent, struct qstr *d_name,
> +				char *buf)
> +{
> +	struct inode *dir = parent;
> +	struct qstr iname;
>   	u32 len;
> +	int name_len;
>   	int elen;
>   	int ret;
> -	u8 *cryptbuf;
> +	u8 *cryptbuf = NULL;
> +
> +	iname.name = d_name->name;
> +	name_len = d_name->len;
> +
> +	/* Handle the special case of snapshot names that start with '_' */
> +	if ((ceph_snap(dir) == CEPH_SNAPDIR) && (name_len > 0) &&
> +	    (iname.name[0] == '_')) {
> +		dir = parse_longname(parent, iname.name, &name_len);
> +		if (IS_ERR(dir))
> +			return PTR_ERR(dir);
> +		iname.name++; /* skip initial '_' */
> +	}
> +	iname.len = name_len;
>   
> -	if (!fscrypt_has_encryption_key(parent)) {
> +	if (!fscrypt_has_encryption_key(dir)) {
>   		memcpy(buf, d_name->name, d_name->len);
> -		return d_name->len;
> +		elen = d_name->len;
> +		goto out;
>   	}
>   
>   	/*
> @@ -212,19 +294,23 @@ int ceph_encode_encrypted_dname(const struct inode *parent,
>   	 *
>   	 * See: fscrypt_setup_filename
>   	 */
> -	if (!fscrypt_fname_encrypted_size(parent, d_name->len, NAME_MAX, &len))
> -		return -ENAMETOOLONG;
> +	if (!fscrypt_fname_encrypted_size(dir, iname.len, NAME_MAX, &len)) {
> +		elen = -ENAMETOOLONG;
> +		goto out;
> +	}
>   
>   	/* Allocate a buffer appropriate to hold the result */
>   	cryptbuf = kmalloc(len > CEPH_NOHASH_NAME_MAX ? NAME_MAX : len,
>   			   GFP_KERNEL);
> -	if (!cryptbuf)
> -		return -ENOMEM;
> +	if (!cryptbuf) {
> +		elen = -ENOMEM;
> +		goto out;
> +	}
>   
> -	ret = fscrypt_fname_encrypt(parent, d_name, cryptbuf, len);
> +	ret = fscrypt_fname_encrypt(dir, &iname, cryptbuf, len);
>   	if (ret) {
> -		kfree(cryptbuf);
> -		return ret;
> +		elen = ret;
> +		goto out;
>   	}
>   
>   	/* hash the end if the name is long enough */
> @@ -243,13 +329,31 @@ int ceph_encode_encrypted_dname(const struct inode *parent,
>   
>   	/* base64 encode the encrypted name */
>   	elen = ceph_base64_encode(cryptbuf, len, buf);
> -	kfree(cryptbuf);
>   	dout("base64-encoded ciphertext name = %.*s\n", elen, buf);
> +
> +	/* To understand the 240 limit, see CEPH_NOHASH_NAME_MAX comments */
> +	WARN_ON(elen > 240);
> +	if ((elen > 0) && (dir != parent)) {
> +		char tmp_buf[NAME_MAX];
> +
> +		elen = snprintf(tmp_buf, sizeof(tmp_buf), "_%.*s_%ld",
> +				elen, buf, dir->i_ino);
> +		memcpy(buf, tmp_buf, elen);
> +	}
> +
> +out:
> +	kfree(cryptbuf);
> +	if (dir != parent) {
> +		if ((dir->i_state & I_NEW))
> +			discard_new_inode(dir);
> +		else
> +			iput(dir);
> +	}
>   	return elen;
>   }
>   
> -int ceph_encode_encrypted_fname(const struct inode *parent,
> -				struct dentry *dentry, char *buf)
> +int ceph_encode_encrypted_fname(struct inode *parent, struct dentry *dentry,
> +				char *buf)
>   {
>   	WARN_ON_ONCE(!fscrypt_has_encryption_key(parent));
>   
> @@ -274,29 +378,42 @@ int ceph_encode_encrypted_fname(const struct inode *parent,
>   int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
>   		      struct fscrypt_str *oname, bool *is_nokey)
>   {
> -	int ret;
> +	struct inode *dir = fname->dir;
>   	struct fscrypt_str _tname = FSTR_INIT(NULL, 0);
>   	struct fscrypt_str iname;
> -
> -	if (!IS_ENCRYPTED(fname->dir)) {
> -		oname->name = fname->name;
> -		oname->len = fname->name_len;
> -		return 0;
> -	}
> +	char *name = fname->name;
> +	int name_len = fname->name_len;
> +	int ret;
>   
>   	/* Sanity check that the resulting name will fit in the buffer */
>   	if (fname->name_len > NAME_MAX || fname->ctext_len > NAME_MAX)
>   		return -EIO;
>   
> -	ret = ceph_fscrypt_prepare_readdir(fname->dir);
> -	if (ret < 0)
> -		return ret;
> +	/* Handle the special case of snapshot names that start with '_' */
> +	if ((ceph_snap(dir) == CEPH_SNAPDIR) && (name_len > 0) &&
> +	    (name[0] == '_')) {
> +		dir = parse_longname(dir, name, &name_len);
> +		if (IS_ERR(dir))
> +			return PTR_ERR(dir);
> +		name++; /* skip initial '_' */
> +	}
> +
> +	if (!IS_ENCRYPTED(dir)) {
> +		oname->name = fname->name;
> +		oname->len = fname->name_len;
> +		ret = 0;
> +		goto out_inode;
> +	}
> +
> +	ret = ceph_fscrypt_prepare_readdir(dir);
> +	if (ret)
> +		goto out_inode;
>   
>   	/*
>   	 * Use the raw dentry name as sent by the MDS instead of
>   	 * generating a nokey name via fscrypt.
>   	 */
> -	if (!fscrypt_has_encryption_key(fname->dir)) {
> +	if (!fscrypt_has_encryption_key(dir)) {
>   		if (fname->no_copy)
>   			oname->name = fname->name;
>   		else
> @@ -304,7 +421,8 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
>   		oname->len = fname->name_len;
>   		if (is_nokey)
>   			*is_nokey = true;
> -		return 0;
> +		ret = 0;
> +		goto out_inode;
>   	}
>   
>   	if (fname->ctext_len == 0) {
> @@ -313,12 +431,11 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
>   		if (!tname) {
>   			ret = fscrypt_fname_alloc_buffer(NAME_MAX, &_tname);
>   			if (ret)
> -				return ret;
> +				goto out_inode;
>   			tname = &_tname;
>   		}
>   
> -		declen = ceph_base64_decode(fname->name, fname->name_len,
> -					    tname->name);
> +		declen = ceph_base64_decode(name, name_len, tname->name);
>   		if (declen <= 0) {
>   			ret = -EIO;
>   			goto out;
> @@ -330,9 +447,25 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
>   		iname.len = fname->ctext_len;
>   	}
>   
> -	ret = fscrypt_fname_disk_to_usr(fname->dir, 0, 0, &iname, oname);
> +	ret = fscrypt_fname_disk_to_usr(dir, 0, 0, &iname, oname);
> +	if (!ret && (dir != fname->dir)) {
> +		char tmp_buf[CEPH_BASE64_CHARS(NAME_MAX)];
> +
> +		name_len = snprintf(tmp_buf, sizeof(tmp_buf), "_%.*s_%ld",
> +				    oname->len, oname->name, dir->i_ino);
> +		memcpy(oname->name, tmp_buf, name_len);
> +		oname->len = name_len;
> +	}
> +
>   out:
>   	fscrypt_fname_free_buffer(&_tname);
> +out_inode:
> +	if ((dir != fname->dir) && !IS_ERR(dir)) {
> +		if ((dir->i_state & I_NEW))
> +			discard_new_inode(dir);
> +		else
> +			iput(dir);
> +	}
>   	return ret;
>   }
>   
> diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
> index 6afa6d888fe0..47e0c319fc68 100644
> --- a/fs/ceph/crypto.h
> +++ b/fs/ceph/crypto.h
> @@ -102,10 +102,10 @@ 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);
> +int ceph_encode_encrypted_dname(struct inode *parent, struct qstr *d_name,
> +				char *buf);
> +int ceph_encode_encrypted_fname(struct inode *parent, struct dentry *dentry,
> +				char *buf);
>   
>   static inline int ceph_fname_alloc_buffer(struct inode *parent,
>   					  struct fscrypt_str *fname)
> @@ -194,14 +194,14 @@ static inline void ceph_fscrypt_as_ctx_to_req(struct ceph_mds_request *req,
>   {
>   }
>   
> -static inline int ceph_encode_encrypted_dname(const struct inode *parent,
> +static inline int ceph_encode_encrypted_dname(struct inode *parent,
>   					      struct qstr *d_name, char *buf)
>   {
>   	memcpy(buf, d_name->name, d_name->len);
>   	return d_name->len;
>   }
>   
> -static inline int ceph_encode_encrypted_fname(const struct inode *parent,
> +static inline int ceph_encode_encrypted_fname(struct inode *parent,
>   					      struct dentry *dentry, char *buf)
>   {
>   	return -EOPNOTSUPP;
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 054fd66609b7..83c420646f90 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -91,9 +91,15 @@ struct inode *ceph_new_inode(struct inode *dir, struct dentry *dentry,
>   	if (err < 0)
>   		goto out_err;
>   
> -	err = ceph_fscrypt_prepare_context(dir, inode, as_ctx);
> -	if (err)
> -		goto out_err;
> +	/*
> +	 * We'll skip setting fscrypt context for snapshots, leaving that for
> +	 * the handle_reply().
> +	 */
> +	if (ceph_snap(dir) != CEPH_SNAPDIR) {
> +		err = ceph_fscrypt_prepare_context(dir, inode, as_ctx);
> +		if (err)
> +			goto out_err;
> +	}
>   
>   	return inode;
>   out_err:
> @@ -159,6 +165,7 @@ struct inode *ceph_get_snapdir(struct inode *parent)
>   	};
>   	struct inode *inode = ceph_get_inode(parent->i_sb, vino, NULL);
>   	struct ceph_inode_info *ci = ceph_inode(inode);
> +	int ret = -ENOTDIR;
>   
>   	if (IS_ERR(inode))
>   		return inode;
> @@ -184,6 +191,24 @@ struct inode *ceph_get_snapdir(struct inode *parent)
>   	ci->i_rbytes = 0;
>   	ci->i_btime = ceph_inode(parent)->i_btime;
>   
> +#ifdef CONFIG_FS_ENCRYPTION
> +	/* if encrypted, just borrow fscrypt_auth from parent */
> +	if (IS_ENCRYPTED(parent)) {
> +		struct ceph_inode_info *pci = ceph_inode(parent);
> +
> +		ci->fscrypt_auth = kmemdup(pci->fscrypt_auth,
> +					   pci->fscrypt_auth_len,
> +					   GFP_KERNEL);
> +		if (ci->fscrypt_auth) {
> +			inode->i_flags |= S_ENCRYPTED;
> +			ci->fscrypt_auth_len = pci->fscrypt_auth_len;
> +		} else {
> +			dout("Failed to alloc snapdir fscrypt_auth\n");
> +			ret = -ENOMEM;
> +			goto err;
> +		}
> +	}
> +#endif
>   	if (inode->i_state & I_NEW) {
>   		inode->i_op = &ceph_snapdir_iops;
>   		inode->i_fop = &ceph_snapdir_fops;
> @@ -197,7 +222,7 @@ struct inode *ceph_get_snapdir(struct inode *parent)
>   		discard_new_inode(inode);
>   	else
>   		iput(inode);
> -	return ERR_PTR(-ENOTDIR);
> +	return ERR_PTR(ret);
>   }
>   
>   const struct inode_operations ceph_file_iops = {
>
Luis Henriques Sept. 7, 2023, 8:44 a.m. UTC | #2
Xiubo Li <xiubli@redhat.com> writes:

> On 9/6/23 21:47, Luís Henriques wrote:
>> Since filenames in encrypted directories are encrypted and shown as
>> a base64-encoded string when the directory is locked, make snapshot
>> names show a similar behaviour.
>>
>> When creating a snapshot, .snap directories for every subdirectory will
>> show the snapshot name in the "long format":
>>
>>    # mkdir .snap/my-snap
>>    # ls my-dir/.snap/
>>    _my-snap_1099511627782
>>
>> Encrypted snapshots will need to be able to handle these by
>> encrypting/decrypting only the snapshot part of the string ('my-snap').
>>
>> Also, since the MDS prevents snapshot names to be bigger than 240
>> characters it is necessary to adapt CEPH_NOHASH_NAME_MAX to accommodate
>> this extra limitation.
>>
>> [ idryomov: drop const on !CONFIG_FS_ENCRYPTION branch too ]
>>
>> Cc: Dan Carpenter <dan.carpenter@linaro.org>
>> Signed-off-by: Luís Henriques <lhenriques@suse.de>
>> Reviewed-by: Jeff Layton <jlayton@kernel.org>
>> Reviewed-by: Xiubo Li <xiubli@redhat.com>
>> Reviewed-by: Milind Changire <mchangir@redhat.com>
>> Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
>> ---
>> As reported by Dan Carpenter, the initial version of this patch wasn't
>> dealing correctly with the ceph_get_inode() return value: it never returns
>> NULL.  Simply dropped the code that handled the case where a NULL was
>> being returned.
>>
>> Xiubo, can you please pick this new revision, if you think it's correct?
>
> Hi Luis,
>
> Please generate a new patch to fix this, the fscrypt corresponding commits have
> been merged already.

Yeah, I just realised it was too late for it.  Which is great, I'm glad it
was too late, actually! :-)

Anyway, I'll send out a separate fix for this, so that it can be included
in -rc2.

Cheers,
diff mbox series

Patch

diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index 9494bf485f6a..7d0b9b5ccfc6 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -193,17 +193,99 @@  void ceph_fscrypt_as_ctx_to_req(struct ceph_mds_request *req,
 	swap(req->r_fscrypt_auth, as->fscrypt_auth);
 }
 
-int ceph_encode_encrypted_dname(const struct inode *parent,
-				struct qstr *d_name, char *buf)
+/*
+ * User-created snapshots can't start with '_'.  Snapshots that start with this
+ * character are special (hint: there aren't real snapshots) and use the
+ * following format:
+ *
+ *   _<SNAPSHOT-NAME>_<INODE-NUMBER>
+ *
+ * where:
+ *  - <SNAPSHOT-NAME> - the real snapshot name that may need to be decrypted,
+ *  - <INODE-NUMBER> - the inode number (in decimal) for the actual snapshot
+ *
+ * This function parses these snapshot names and returns the inode
+ * <INODE-NUMBER>.  'name_len' will also bet set with the <SNAPSHOT-NAME>
+ * length.
+ */
+static struct inode *parse_longname(const struct inode *parent,
+				    const char *name, int *name_len)
 {
+	struct inode *dir = NULL;
+	struct ceph_vino vino = { .snap = CEPH_NOSNAP };
+	char *inode_number;
+	char *name_end;
+	int orig_len = *name_len;
+	int ret = -EIO;
+
+	/* Skip initial '_' */
+	name++;
+	name_end = strrchr(name, '_');
+	if (!name_end) {
+		dout("Failed to parse long snapshot name: %s\n", name);
+		return ERR_PTR(-EIO);
+	}
+	*name_len = (name_end - name);
+	if (*name_len <= 0) {
+		pr_err("Failed to parse long snapshot name\n");
+		return ERR_PTR(-EIO);
+	}
+
+	/* Get the inode number */
+	inode_number = kmemdup_nul(name_end + 1,
+				   orig_len - *name_len - 2,
+				   GFP_KERNEL);
+	if (!inode_number)
+		return ERR_PTR(-ENOMEM);
+	ret = kstrtou64(inode_number, 10, &vino.ino);
+	if (ret) {
+		dout("Failed to parse inode number: %s\n", name);
+		dir = ERR_PTR(ret);
+		goto out;
+	}
+
+	/* And finally the inode */
+	dir = ceph_find_inode(parent->i_sb, vino);
+	if (!dir) {
+		/* This can happen if we're not mounting cephfs on the root */
+		dir = ceph_get_inode(parent->i_sb, vino, NULL);
+	}
+	if (IS_ERR(dir))
+		dout("Can't find inode %s (%s)\n", inode_number, name);
+
+out:
+	kfree(inode_number);
+	return dir;
+}
+
+int ceph_encode_encrypted_dname(struct inode *parent, struct qstr *d_name,
+				char *buf)
+{
+	struct inode *dir = parent;
+	struct qstr iname;
 	u32 len;
+	int name_len;
 	int elen;
 	int ret;
-	u8 *cryptbuf;
+	u8 *cryptbuf = NULL;
+
+	iname.name = d_name->name;
+	name_len = d_name->len;
+
+	/* Handle the special case of snapshot names that start with '_' */
+	if ((ceph_snap(dir) == CEPH_SNAPDIR) && (name_len > 0) &&
+	    (iname.name[0] == '_')) {
+		dir = parse_longname(parent, iname.name, &name_len);
+		if (IS_ERR(dir))
+			return PTR_ERR(dir);
+		iname.name++; /* skip initial '_' */
+	}
+	iname.len = name_len;
 
-	if (!fscrypt_has_encryption_key(parent)) {
+	if (!fscrypt_has_encryption_key(dir)) {
 		memcpy(buf, d_name->name, d_name->len);
-		return d_name->len;
+		elen = d_name->len;
+		goto out;
 	}
 
 	/*
@@ -212,19 +294,23 @@  int ceph_encode_encrypted_dname(const struct inode *parent,
 	 *
 	 * See: fscrypt_setup_filename
 	 */
-	if (!fscrypt_fname_encrypted_size(parent, d_name->len, NAME_MAX, &len))
-		return -ENAMETOOLONG;
+	if (!fscrypt_fname_encrypted_size(dir, iname.len, NAME_MAX, &len)) {
+		elen = -ENAMETOOLONG;
+		goto out;
+	}
 
 	/* Allocate a buffer appropriate to hold the result */
 	cryptbuf = kmalloc(len > CEPH_NOHASH_NAME_MAX ? NAME_MAX : len,
 			   GFP_KERNEL);
-	if (!cryptbuf)
-		return -ENOMEM;
+	if (!cryptbuf) {
+		elen = -ENOMEM;
+		goto out;
+	}
 
-	ret = fscrypt_fname_encrypt(parent, d_name, cryptbuf, len);
+	ret = fscrypt_fname_encrypt(dir, &iname, cryptbuf, len);
 	if (ret) {
-		kfree(cryptbuf);
-		return ret;
+		elen = ret;
+		goto out;
 	}
 
 	/* hash the end if the name is long enough */
@@ -243,13 +329,31 @@  int ceph_encode_encrypted_dname(const struct inode *parent,
 
 	/* base64 encode the encrypted name */
 	elen = ceph_base64_encode(cryptbuf, len, buf);
-	kfree(cryptbuf);
 	dout("base64-encoded ciphertext name = %.*s\n", elen, buf);
+
+	/* To understand the 240 limit, see CEPH_NOHASH_NAME_MAX comments */
+	WARN_ON(elen > 240);
+	if ((elen > 0) && (dir != parent)) {
+		char tmp_buf[NAME_MAX];
+
+		elen = snprintf(tmp_buf, sizeof(tmp_buf), "_%.*s_%ld",
+				elen, buf, dir->i_ino);
+		memcpy(buf, tmp_buf, elen);
+	}
+
+out:
+	kfree(cryptbuf);
+	if (dir != parent) {
+		if ((dir->i_state & I_NEW))
+			discard_new_inode(dir);
+		else
+			iput(dir);
+	}
 	return elen;
 }
 
-int ceph_encode_encrypted_fname(const struct inode *parent,
-				struct dentry *dentry, char *buf)
+int ceph_encode_encrypted_fname(struct inode *parent, struct dentry *dentry,
+				char *buf)
 {
 	WARN_ON_ONCE(!fscrypt_has_encryption_key(parent));
 
@@ -274,29 +378,42 @@  int ceph_encode_encrypted_fname(const struct inode *parent,
 int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
 		      struct fscrypt_str *oname, bool *is_nokey)
 {
-	int ret;
+	struct inode *dir = fname->dir;
 	struct fscrypt_str _tname = FSTR_INIT(NULL, 0);
 	struct fscrypt_str iname;
-
-	if (!IS_ENCRYPTED(fname->dir)) {
-		oname->name = fname->name;
-		oname->len = fname->name_len;
-		return 0;
-	}
+	char *name = fname->name;
+	int name_len = fname->name_len;
+	int ret;
 
 	/* Sanity check that the resulting name will fit in the buffer */
 	if (fname->name_len > NAME_MAX || fname->ctext_len > NAME_MAX)
 		return -EIO;
 
-	ret = ceph_fscrypt_prepare_readdir(fname->dir);
-	if (ret < 0)
-		return ret;
+	/* Handle the special case of snapshot names that start with '_' */
+	if ((ceph_snap(dir) == CEPH_SNAPDIR) && (name_len > 0) &&
+	    (name[0] == '_')) {
+		dir = parse_longname(dir, name, &name_len);
+		if (IS_ERR(dir))
+			return PTR_ERR(dir);
+		name++; /* skip initial '_' */
+	}
+
+	if (!IS_ENCRYPTED(dir)) {
+		oname->name = fname->name;
+		oname->len = fname->name_len;
+		ret = 0;
+		goto out_inode;
+	}
+
+	ret = ceph_fscrypt_prepare_readdir(dir);
+	if (ret)
+		goto out_inode;
 
 	/*
 	 * Use the raw dentry name as sent by the MDS instead of
 	 * generating a nokey name via fscrypt.
 	 */
-	if (!fscrypt_has_encryption_key(fname->dir)) {
+	if (!fscrypt_has_encryption_key(dir)) {
 		if (fname->no_copy)
 			oname->name = fname->name;
 		else
@@ -304,7 +421,8 @@  int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
 		oname->len = fname->name_len;
 		if (is_nokey)
 			*is_nokey = true;
-		return 0;
+		ret = 0;
+		goto out_inode;
 	}
 
 	if (fname->ctext_len == 0) {
@@ -313,12 +431,11 @@  int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
 		if (!tname) {
 			ret = fscrypt_fname_alloc_buffer(NAME_MAX, &_tname);
 			if (ret)
-				return ret;
+				goto out_inode;
 			tname = &_tname;
 		}
 
-		declen = ceph_base64_decode(fname->name, fname->name_len,
-					    tname->name);
+		declen = ceph_base64_decode(name, name_len, tname->name);
 		if (declen <= 0) {
 			ret = -EIO;
 			goto out;
@@ -330,9 +447,25 @@  int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
 		iname.len = fname->ctext_len;
 	}
 
-	ret = fscrypt_fname_disk_to_usr(fname->dir, 0, 0, &iname, oname);
+	ret = fscrypt_fname_disk_to_usr(dir, 0, 0, &iname, oname);
+	if (!ret && (dir != fname->dir)) {
+		char tmp_buf[CEPH_BASE64_CHARS(NAME_MAX)];
+
+		name_len = snprintf(tmp_buf, sizeof(tmp_buf), "_%.*s_%ld",
+				    oname->len, oname->name, dir->i_ino);
+		memcpy(oname->name, tmp_buf, name_len);
+		oname->len = name_len;
+	}
+
 out:
 	fscrypt_fname_free_buffer(&_tname);
+out_inode:
+	if ((dir != fname->dir) && !IS_ERR(dir)) {
+		if ((dir->i_state & I_NEW))
+			discard_new_inode(dir);
+		else
+			iput(dir);
+	}
 	return ret;
 }
 
diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
index 6afa6d888fe0..47e0c319fc68 100644
--- a/fs/ceph/crypto.h
+++ b/fs/ceph/crypto.h
@@ -102,10 +102,10 @@  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);
+int ceph_encode_encrypted_dname(struct inode *parent, struct qstr *d_name,
+				char *buf);
+int ceph_encode_encrypted_fname(struct inode *parent, struct dentry *dentry,
+				char *buf);
 
 static inline int ceph_fname_alloc_buffer(struct inode *parent,
 					  struct fscrypt_str *fname)
@@ -194,14 +194,14 @@  static inline void ceph_fscrypt_as_ctx_to_req(struct ceph_mds_request *req,
 {
 }
 
-static inline int ceph_encode_encrypted_dname(const struct inode *parent,
+static inline int ceph_encode_encrypted_dname(struct inode *parent,
 					      struct qstr *d_name, char *buf)
 {
 	memcpy(buf, d_name->name, d_name->len);
 	return d_name->len;
 }
 
-static inline int ceph_encode_encrypted_fname(const struct inode *parent,
+static inline int ceph_encode_encrypted_fname(struct inode *parent,
 					      struct dentry *dentry, char *buf)
 {
 	return -EOPNOTSUPP;
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 054fd66609b7..83c420646f90 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -91,9 +91,15 @@  struct inode *ceph_new_inode(struct inode *dir, struct dentry *dentry,
 	if (err < 0)
 		goto out_err;
 
-	err = ceph_fscrypt_prepare_context(dir, inode, as_ctx);
-	if (err)
-		goto out_err;
+	/*
+	 * We'll skip setting fscrypt context for snapshots, leaving that for
+	 * the handle_reply().
+	 */
+	if (ceph_snap(dir) != CEPH_SNAPDIR) {
+		err = ceph_fscrypt_prepare_context(dir, inode, as_ctx);
+		if (err)
+			goto out_err;
+	}
 
 	return inode;
 out_err:
@@ -159,6 +165,7 @@  struct inode *ceph_get_snapdir(struct inode *parent)
 	};
 	struct inode *inode = ceph_get_inode(parent->i_sb, vino, NULL);
 	struct ceph_inode_info *ci = ceph_inode(inode);
+	int ret = -ENOTDIR;
 
 	if (IS_ERR(inode))
 		return inode;
@@ -184,6 +191,24 @@  struct inode *ceph_get_snapdir(struct inode *parent)
 	ci->i_rbytes = 0;
 	ci->i_btime = ceph_inode(parent)->i_btime;
 
+#ifdef CONFIG_FS_ENCRYPTION
+	/* if encrypted, just borrow fscrypt_auth from parent */
+	if (IS_ENCRYPTED(parent)) {
+		struct ceph_inode_info *pci = ceph_inode(parent);
+
+		ci->fscrypt_auth = kmemdup(pci->fscrypt_auth,
+					   pci->fscrypt_auth_len,
+					   GFP_KERNEL);
+		if (ci->fscrypt_auth) {
+			inode->i_flags |= S_ENCRYPTED;
+			ci->fscrypt_auth_len = pci->fscrypt_auth_len;
+		} else {
+			dout("Failed to alloc snapdir fscrypt_auth\n");
+			ret = -ENOMEM;
+			goto err;
+		}
+	}
+#endif
 	if (inode->i_state & I_NEW) {
 		inode->i_op = &ceph_snapdir_iops;
 		inode->i_fop = &ceph_snapdir_fops;
@@ -197,7 +222,7 @@  struct inode *ceph_get_snapdir(struct inode *parent)
 		discard_new_inode(inode);
 	else
 		iput(inode);
-	return ERR_PTR(-ENOTDIR);
+	return ERR_PTR(ret);
 }
 
 const struct inode_operations ceph_file_iops = {