Re: [PATCH v4 46/46] btrfs: load the inode context before sending writes

Eric Biggers
2023-12-05T05:54:41Z

On Fri, Dec 01, 2023 at 05:11:43PM -0500, Josef Bacik wrote:
> For send we will read the pages and copy them into our buffer. Use the
> fscrypt_inode_open helper to make sure the key is loaded properly before
> trying to read from the inode so the contents are properly decrypted.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
> fs/btrfs/send.c | 35 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index de77321777f4..3475b4cea09d 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -5392,6 +5392,37 @@ static int put_file_data(struct send_ctx *sctx, u64 offset, u32 len)
> return ret;
> }
>
> +static int load_fscrypt_context(struct send_ctx *sctx)
> +{
> + struct btrfs_root *root = sctx->send_root;
> + struct name_cache_entry *nce;
> + struct inode *dir;
> + int ret;
> +
> + if (!IS_ENCRYPTED(sctx->cur_inode))
> + return 0;
> +
> + /*
> + * We're encrypted, we need to load the parent inode in order to make
> + * sure the encryption context is loaded, we use this after calling
> + * get_cur_path() so our nce for the current inode should be here. If
> + * not handle it, but ASSERT() for developers.
> + */
> + nce = name_cache_search(sctx, sctx->cur_ino, sctx->cur_inode_gen);
> + if (!nce) {
> + ASSERT(nce);
> + return -EINVAL;
> + }
> +
> + dir = btrfs_iget(root->fs_info->sb, nce->parent_ino, root);
> + if (IS_ERR(dir))
> + return PTR_ERR(dir);
> +
> + ret = fscrypt_inode_open(dir, sctx->cur_inode);
> + iput(dir);
> + return ret;

fscrypt_file_open() is called even on unencrypted files, which results in strong
enforcement that encrypted directories don't contain unencrypted files.

The code above doesn't do that with fscrypt_inode_open(). That seems like a
bug; the rules for "send" internally "opening" a file should be the same as a
standard open, right? Or did you do it this way intentionally?

- Eric