From: Josef Bacik <josef@toxicpanda.com>

We use this helper for inode-resolve and path resolution in send, so
update this helper to properly decrypt any encrypted names it finds.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Daniel Vacek <neelx@suse.com>
---

v5: https://lore.kernel.org/linux-btrfs/365d4f820f70b7cf69b1b9cae9b949a15c3350b0.1706116485.git.josef@toxicpanda.com/
 * Adapted to btrfs_iget() now returning binode instead of vfs inode
   as before.
 * Adapted to crypt info being moved from vfs inode to FS specific inode.
---
 fs/btrfs/backref.c | 42 +++++++++++++++++++++++++++++++++++++----
 fs/btrfs/fscrypt.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/fscrypt.h | 10 ++++++++++
 3 files changed, 95 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index 9bb406f7dd30..577c3ef87791 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -20,6 +20,7 @@
 #include "extent-tree.h"
 #include "relocation.h"
 #include "tree-checker.h"
+#include "fscrypt.h"
 
 /* Just arbitrary numbers so we can be sure one of these happened. */
 #define BACKREF_FOUND_SHARED     6
@@ -2107,6 +2108,39 @@ int btrfs_find_one_extref(struct btrfs_root *root, u64 inode_objectid,
 	return ret;
 }
 
+static int copy_resolved_iref_to_buf(struct btrfs_root *fs_root,
+				     struct extent_buffer *eb, char *dest,
+				     u64 parent, unsigned long name_off,
+				     u32 name_len, s64 *bytes_left)
+{
+	struct btrfs_fs_info *fs_info = fs_root->fs_info;
+	struct fscrypt_str fname = FSTR_INIT(NULL, 0);
+	int ret;
+
+	/* No encryption, just copy the name in. */
+	if (!btrfs_fs_incompat(fs_info, ENCRYPT)) {
+		*bytes_left -= name_len;
+		if (*bytes_left >= 0)
+			read_extent_buffer(eb, dest + *bytes_left, name_off, name_len);
+		return 0;
+	}
+
+	ret = fscrypt_fname_alloc_buffer(BTRFS_NAME_LEN, &fname);
+	if (ret)
+		return ret;
+
+	ret = btrfs_decrypt_name(fs_root, eb, name_off, name_len, parent, &fname);
+	if (ret)
+		goto out;
+
+	*bytes_left -= fname.len;
+	if (*bytes_left >= 0)
+		memcpy(dest + *bytes_left, fname.name, fname.len);
+out:
+	fscrypt_fname_free_buffer(&fname);
+	return ret;
+}
+
 /*
  * this iterates to turn a name (from iref/extref) into a full filesystem path.
  * Elements of the path are separated by '/' and the path is guaranteed to be
@@ -2138,10 +2172,10 @@ char *btrfs_ref_to_path(struct btrfs_root *fs_root, struct btrfs_path *path,
 		dest[bytes_left] = '\0';
 
 	while (1) {
-		bytes_left -= name_len;
-		if (bytes_left >= 0)
-			read_extent_buffer(eb, dest + bytes_left,
-					   name_off, name_len);
+		ret = copy_resolved_iref_to_buf(fs_root, eb, dest, parent,
+						name_off, name_len, &bytes_left);
+		if (ret)
+			break;
 		if (eb != eb_in) {
 			if (!path->skip_locking)
 				btrfs_tree_read_unlock(eb);
diff --git a/fs/btrfs/fscrypt.c b/fs/btrfs/fscrypt.c
index d1a4cbb990d4..bcb86cbaa171 100644
--- a/fs/btrfs/fscrypt.c
+++ b/fs/btrfs/fscrypt.c
@@ -385,6 +385,53 @@ int btrfs_fscrypt_bio_length(struct bio *bio, u64 map_length)
 	return map_length;
 }
 
+int btrfs_decrypt_name(struct btrfs_root *root, struct extent_buffer *eb,
+		       unsigned long name_off, u32 name_len,
+		       u64 parent_ino, struct fscrypt_str *name)
+{
+	struct btrfs_inode *inode;
+	struct inode *dir;
+	struct fscrypt_str iname = FSTR_INIT(NULL, 0);
+	int ret;
+
+	ASSERT(name_len <= BTRFS_NAME_LEN);
+
+	ret = fscrypt_fname_alloc_buffer(name_len, &iname);
+	if (ret)
+		return ret;
+
+	inode = btrfs_iget(parent_ino, root);
+	if (IS_ERR(inode)) {
+		ret = PTR_ERR(inode);
+		goto out;
+	}
+	dir = &inode->vfs_inode;
+
+	/*
+	 * Directory isn't encrypted, the name isn't encrypted, we can just copy
+	 * it into the buffer.
+	 */
+	if (!IS_ENCRYPTED(dir)) {
+		read_extent_buffer(eb, name->name, name_off, name_len);
+		name->len = name_len;
+		goto out_inode;
+	}
+
+	read_extent_buffer(eb, iname.name, name_off, name_len);
+
+	ret = fscrypt_prepare_readdir(dir);
+	if (ret)
+		goto out_inode;
+
+	ASSERT(inode->i_crypt_info);
+	ret = fscrypt_fname_disk_to_usr(dir, 0, 0, &iname, name);
+out_inode:
+	iput(dir);
+out:
+	fscrypt_fname_free_buffer(&iname);
+	return ret;
+}
+
 const struct fscrypt_operations btrfs_fscrypt_ops = {
 	.inode_info_offs = (int)offsetof(struct btrfs_inode, i_crypt_info) -
 			   (int)offsetof(struct btrfs_inode, vfs_inode),
diff --git a/fs/btrfs/fscrypt.h b/fs/btrfs/fscrypt.h
index 347b34f45715..4f49ed6176d4 100644
--- a/fs/btrfs/fscrypt.h
+++ b/fs/btrfs/fscrypt.h
@@ -32,6 +32,9 @@ bool btrfs_mergeable_encrypted_bio(struct bio *bio, struct inode *inode,
 				   struct fscrypt_extent_info *fi,
 				   u64 logical_offset);
 int btrfs_fscrypt_bio_length(struct bio *bio, u64 map_length);
+int btrfs_decrypt_name(struct btrfs_root *root, struct extent_buffer *eb,
+		       unsigned long name_off, u32 name_len,
+		       u64 parent_ino, struct fscrypt_str *name);
 
 #else
 static inline void btrfs_fscrypt_save_extent_info(struct btrfs_path *path,
@@ -91,6 +94,13 @@ static inline u64 btrfs_fscrypt_bio_length(struct bio *bio, u64 map_length)
 	return map_length;
 }
 
+static inline int btrfs_decrypt_name(struct btrfs_root *root, struct extent_buffer *eb,
+				     unsigned long name_off, u32 name_len,
+				     u64 parent_ino, struct fscrypt_str *name)
+{
+	return -EINVAL;
+}
+
 #endif /* CONFIG_FS_ENCRYPTION */
 
 extern const struct fscrypt_operations btrfs_fscrypt_ops;
-- 
2.51.0

Daniel Vacek <neelx@suse.com> wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> We use this helper for inode-resolve and path resolution in send, so
> update this helper to properly decrypt any encrypted names it finds.
> 

Hi everyone,

I'm running the btrfs for-next branch, along with the encryption patch set
through my AI review prompts:

https://github.com/masoncl/review-prompts

I'm working on some btrfs specific instructions, but hopefully these reviews
are already useful.  I've kept the headers to make it easier to double check
that I'm actually replying to the right email, but please let me know if any
parts of this are incorrect:


commit 473bbb12052f731a4e2ce3c48e7d7e9f2510528b
Author: Josef Bacik <josef@toxicpanda.com>

btrfs: make btrfs_ref_to_path handle encrypted filenames

This commit updates btrfs_ref_to_path() to handle encrypted filenames by
introducing a new copy_resolved_iref_to_buf() helper that can decrypt
names when the filesystem has encryption enabled.

> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 9bb406f7dd302..577c3ef87791a 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c

[ ... ]

> @@ -2107,6 +2108,39 @@ int btrfs_find_one_extref(struct btrfs_root *root, u64 inode_objectid,
>  	return ret;
>  }
>
> +static int copy_resolved_iref_to_buf(struct btrfs_root *fs_root,
> +				     struct extent_buffer *eb, char *dest,
> +				     u64 parent, unsigned long name_off,
> +				     u32 name_len, s64 *bytes_left)
> +{
> +	struct btrfs_fs_info *fs_info = fs_root->fs_info;
> +	struct fscrypt_str fname = FSTR_INIT(NULL, 0);
> +	int ret;
> +
> +	/* No encryption, just copy the name in. */
> +	if (!btrfs_fs_incompat(fs_info, ENCRYPT)) {
> +		*bytes_left -= name_len;
> +		if (*bytes_left >= 0)
> +			read_extent_buffer(eb, dest + *bytes_left, name_off, name_len);
> +		return 0;
> +	}
> +
> +	ret = fscrypt_fname_alloc_buffer(BTRFS_NAME_LEN, &fname);
> +	if (ret)
> +		return ret;
> +
> +	ret = btrfs_decrypt_name(fs_root, eb, name_off, name_len, parent, &fname);
> +	if (ret)
> +		goto out;
> +
> +	*bytes_left -= fname.len;
> +	if (*bytes_left >= 0)
> +		memcpy(dest + *bytes_left, fname.name, fname.len);
> +out:
> +	fscrypt_fname_free_buffer(&fname);
> +	return ret;
> +}
> +
>  /*
>   * this iterates to turn a name (from iref/extref) into a full filesystem path.
>   * Elements of the path are separated by '/' and the path is guaranteed to be
> @@ -2138,10 +2172,10 @@ char *btrfs_ref_to_path(struct btrfs_root *fs_root, struct btrfs_path *path,
>  		dest[bytes_left] = '\0';
>
>  	while (1) {
> -		bytes_left -= name_len;
> -		if (bytes_left >= 0)
> -			read_extent_buffer(eb, dest + bytes_left,
> -					   name_off, name_len);
> +		ret = copy_resolved_iref_to_buf(fs_root, eb, dest, parent,
> +						name_off, name_len, &bytes_left);
> +		if (ret)
> +			break;
>  		if (eb != eb_in) {
>  			if (!path->skip_locking)
>  				btrfs_tree_read_unlock(eb);

Does this code leak the extent buffer when copy_resolved_iref_to_buf()
fails? Looking at the loop structure in btrfs_ref_to_path(), when
copy_resolved_iref_to_buf() returns an error and we break at line 2178,
the cleanup code that follows never executes:

    if (eb != eb_in) {
        if (!path->skip_locking)
            btrfs_tree_read_unlock(eb);
        free_extent_buffer(eb);
    }

This can happen in subsequent iterations where eb != eb_in (the extent
buffer was detached from path->nodes[0] and set to NULL in the previous
iteration). The cleanup at the end of the function (btrfs_release_path()
at line 2216) cannot free eb since it's no longer in the path.

This could be triggered when decryption fails in copy_resolved_iref_to_buf()
due to btrfs_iget() error, fscrypt_fname_alloc_buffer() ENOMEM, or
fscrypt_prepare_readdir() failure on an encrypted filesystem.

Before this patch, the name copy operation (read_extent_buffer) could not
fail, so the cleanup always happened before any error check.