From: "Luís Henriques" <lhenriques@suse.de>
To: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
Cc: "Theodore Y. Ts'o" <tytso@mit.edu>,
Jaegeuk Kim <jaegeuk@kernel.org>,
Eric Biggers <ebiggers@kernel.org>, Chris Mason <clm@fb.com>,
Josef Bacik <josef@toxicpanda.com>,
David Sterba <dsterba@suse.com>,
linux-fscrypt@vger.kernel.org, linux-btrfs@vger.kernel.org,
kernel-team@meta.com, Omar Sandoval <osandov@osandov.com>
Subject: Re: [PATCH v2 04/17] btrfs: start using fscrypt hooks
Date: Mon, 17 Jul 2023 16:34:17 +0100 [thread overview]
Message-ID: <875y6iwnsr.fsf@suse.de> (raw)
In-Reply-To: 0afc75247f4312d78ce663382e7d89854c353e9b.1689564024.git.sweettea-kernel@dorminy.me
(Sorry for the send. Somehow, my MUA is failing to set the CC header.)
Sweet Tea Dorminy <sweettea-kernel@dorminy.me> writes:
> From: Omar Sandoval <osandov@osandov.com>
>
> In order to appropriately encrypt, create, open, rename, and various
> symlink operations must call fscrypt hooks. These determine whether the
> inode should be encrypted and do other preparatory actions. The
> superblock must have fscrypt operations registered, so implement the
> minimal set also, and introduce the new fscrypt.[ch] files to hold the
> fscrypt-specific functionality. Also add the key prefix for fscrypt v1
> keys.
>
> Signed-off-by: Omar Sandoval <osandov@osandov.com>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
> fs/btrfs/Makefile | 1 +
> fs/btrfs/btrfs_inode.h | 1 +
> fs/btrfs/file.c | 3 ++
> fs/btrfs/fscrypt.c | 8 ++++
> fs/btrfs/fscrypt.h | 10 +++++
> fs/btrfs/inode.c | 87 +++++++++++++++++++++++++++++++++++-------
> fs/btrfs/super.c | 2 +
> 7 files changed, 99 insertions(+), 13 deletions(-)
> create mode 100644 fs/btrfs/fscrypt.c
> create mode 100644 fs/btrfs/fscrypt.h
>
> diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
> index 90d53209755b..90d30b1e044a 100644
> --- a/fs/btrfs/Makefile
> +++ b/fs/btrfs/Makefile
> @@ -40,6 +40,7 @@ btrfs-$(CONFIG_BTRFS_FS_CHECK_INTEGRITY) += check-integrity.o
> btrfs-$(CONFIG_BTRFS_FS_REF_VERIFY) += ref-verify.o
> btrfs-$(CONFIG_BLK_DEV_ZONED) += zoned.o
> btrfs-$(CONFIG_FS_VERITY) += verity.o
> +btrfs-$(CONFIG_FS_ENCRYPTION) += fscrypt.o
>
> btrfs-$(CONFIG_BTRFS_FS_RUN_SANITY_TESTS) += tests/free-space-tests.o \
> tests/extent-buffer-tests.o tests/btrfs-tests.o \
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index d47a927b3504..ec4a06a78aff 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -448,6 +448,7 @@ struct btrfs_new_inode_args {
> struct posix_acl *default_acl;
> struct posix_acl *acl;
> struct fscrypt_name fname;
> + bool encrypt;
> };
>
> int btrfs_new_inode_prepare(struct btrfs_new_inode_args *args,
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index fd03e689a6be..73038908876a 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -3698,6 +3698,9 @@ static int btrfs_file_open(struct inode *inode, struct file *filp)
>
> filp->f_mode |= FMODE_NOWAIT | FMODE_BUF_RASYNC | FMODE_BUF_WASYNC |
> FMODE_CAN_ODIRECT;
> + ret = fscrypt_file_open(inode, filp);
> + if (ret)
> + return ret;
>
> ret = fsverity_file_open(inode, filp);
> if (ret)
> diff --git a/fs/btrfs/fscrypt.c b/fs/btrfs/fscrypt.c
Nit: both ext4 and ubifs (and in a (hopefully!) near future, ceph) use
'crypto.c' for naming this file. I'd prefer consistency, but meh.
> new file mode 100644
> index 000000000000..3a53dc59c1e4
> --- /dev/null
> +++ b/fs/btrfs/fscrypt.c
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "ctree.h"
> +#include "fscrypt.h"
> +
> +const struct fscrypt_operations btrfs_fscrypt_ops = {
> + .key_prefix = "btrfs:"
> +};
> diff --git a/fs/btrfs/fscrypt.h b/fs/btrfs/fscrypt.h
> new file mode 100644
> index 000000000000..7f4e6888bd43
> --- /dev/null
> +++ b/fs/btrfs/fscrypt.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef BTRFS_FSCRYPT_H
> +#define BTRFS_FSCRYPT_H
> +
> +#include <linux/fscrypt.h>
> +
> +extern const struct fscrypt_operations btrfs_fscrypt_ops;
> +
> +#endif /* BTRFS_FSCRYPT_H */
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 6e622328f2b5..d8484752b905 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -5361,6 +5361,7 @@ void btrfs_evict_inode(struct inode *inode)
> trace_btrfs_inode_evict(inode);
>
> if (!root) {
> + fscrypt_put_encryption_info(inode);
> fsverity_cleanup_inode(inode);
> clear_inode(inode);
Small suggestion: maybe it's time to start thinking adding a new label in
this function and use a 'goto' here.
> return;
> @@ -5464,6 +5465,7 @@ void btrfs_evict_inode(struct inode *inode)
> * to retry these periodically in the future.
> */
> btrfs_remove_delayed_node(BTRFS_I(inode));
> + fscrypt_put_encryption_info(inode);
> fsverity_cleanup_inode(inode);
> clear_inode(inode);
> }
> @@ -6214,6 +6216,10 @@ int btrfs_new_inode_prepare(struct btrfs_new_inode_args *args,
> return ret;
> }
>
> + ret = fscrypt_prepare_new_inode(dir, inode, &args->encrypt);
> + if (ret)
We're leaking args->fname here.
Cheers,
--
Luís
> + return ret;
> +
> /* 1 to add inode item */
> *trans_num_items = 1;
> /* 1 to add compression property */
> @@ -6690,9 +6696,13 @@ static int btrfs_link(struct dentry *old_dentry, struct inode *dir,
> if (inode->i_nlink >= BTRFS_LINK_MAX)
> return -EMLINK;
>
> + err = fscrypt_prepare_link(old_dentry, dir, dentry);
> + if (err)
> + return err;
> +
> err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &fname);
> if (err)
> - goto fail;
> + return err;
>
> err = btrfs_set_inode_index(BTRFS_I(dir), &index);
> if (err)
> @@ -8636,6 +8646,7 @@ void btrfs_test_destroy_inode(struct inode *inode)
>
> void btrfs_free_inode(struct inode *inode)
> {
> + fscrypt_free_inode(inode);
> kmem_cache_free(btrfs_inode_cachep, BTRFS_I(inode));
> }
>
> @@ -8706,8 +8717,7 @@ int btrfs_drop_inode(struct inode *inode)
> /* the snap/subvol tree is on deleting */
> if (btrfs_root_refs(&root->root_item) == 0)
> return 1;
> - else
> - return generic_drop_inode(inode);
> + return generic_drop_inode(inode) || fscrypt_drop_inode(inode);
> }
>
> static void init_once(void *foo)
> @@ -9298,6 +9308,11 @@ static int btrfs_rename2(struct mnt_idmap *idmap, struct inode *old_dir,
> if (flags & ~(RENAME_NOREPLACE | RENAME_EXCHANGE | RENAME_WHITEOUT))
> return -EINVAL;
>
> + ret = fscrypt_prepare_rename(old_dir, old_dentry, new_dir, new_dentry,
> + flags);
> + if (ret)
> + return ret;
> +
> if (flags & RENAME_EXCHANGE)
> ret = btrfs_rename_exchange(old_dir, old_dentry, new_dir,
> new_dentry);
> @@ -9517,15 +9532,22 @@ static int btrfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
> };
> unsigned int trans_num_items;
> int err;
> - int name_len;
> int datasize;
> unsigned long ptr;
> struct btrfs_file_extent_item *ei;
> struct extent_buffer *leaf;
> + struct fscrypt_str disk_link;
> + u32 name_len = strlen(symname);
>
> - name_len = strlen(symname);
> - if (name_len > BTRFS_MAX_INLINE_DATA_SIZE(fs_info))
> - return -ENAMETOOLONG;
> + /*
> + * fscrypt sets disk_link.len to be len + 1, including a NUL terminator, but we
> + * don't store that '\0' character.
> + */
> + err = fscrypt_prepare_symlink(dir, symname, name_len,
> + BTRFS_MAX_INLINE_DATA_SIZE(fs_info) + 1,
> + &disk_link);
> + if (err)
> + return err;
>
> inode = new_inode(dir->i_sb);
> if (!inode)
> @@ -9534,8 +9556,8 @@ static int btrfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
> inode->i_op = &btrfs_symlink_inode_operations;
> inode_nohighmem(inode);
> inode->i_mapping->a_ops = &btrfs_aops;
> - btrfs_i_size_write(BTRFS_I(inode), name_len);
> - inode_set_bytes(inode, name_len);
> + btrfs_i_size_write(BTRFS_I(inode), disk_link.len - 1);
> + inode_set_bytes(inode, disk_link.len - 1);
>
> new_inode_args.inode = inode;
> err = btrfs_new_inode_prepare(&new_inode_args, &trans_num_items);
> @@ -9562,10 +9584,23 @@ static int btrfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
> inode = NULL;
> goto out;
> }
> +
> + if (IS_ENCRYPTED(inode)) {
> + err = fscrypt_encrypt_symlink(inode, symname, name_len,
> + &disk_link);
> + if (err) {
> + btrfs_abort_transaction(trans, err);
> + btrfs_free_path(path);
> + discard_new_inode(inode);
> + inode = NULL;
> + goto out;
> + }
> + }
> +
> key.objectid = btrfs_ino(BTRFS_I(inode));
> key.offset = 0;
> key.type = BTRFS_EXTENT_DATA_KEY;
> - datasize = btrfs_file_extent_calc_inline_size(name_len);
> + datasize = btrfs_file_extent_calc_inline_size(disk_link.len - 1);
> err = btrfs_insert_empty_item(trans, root, path, &key,
> datasize);
> if (err) {
> @@ -9584,10 +9619,10 @@ static int btrfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
> btrfs_set_file_extent_encryption(leaf, ei, 0);
> btrfs_set_file_extent_compression(leaf, ei, 0);
> btrfs_set_file_extent_other_encoding(leaf, ei, 0);
> - btrfs_set_file_extent_ram_bytes(leaf, ei, name_len);
> + btrfs_set_file_extent_ram_bytes(leaf, ei, disk_link.len - 1);
>
> ptr = btrfs_file_extent_inline_start(ei);
> - write_extent_buffer(leaf, symname, ptr, name_len);
> + write_extent_buffer(leaf, disk_link.name, ptr, disk_link.len - 1);
> btrfs_mark_buffer_dirty(leaf);
> btrfs_free_path(path);
>
> @@ -9604,6 +9639,29 @@ static int btrfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
> return err;
> }
>
> +static const char *btrfs_get_link(struct dentry *dentry, struct inode *inode,
> + struct delayed_call *done)
> +{
> + struct page *cpage;
> + const char *paddr;
> + struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> +
> + if (!IS_ENCRYPTED(inode))
> + return page_get_link(dentry, inode, done);
> +
> + if (!dentry)
> + return ERR_PTR(-ECHILD);
> +
> + cpage = read_mapping_page(inode->i_mapping, 0, NULL);
> + if (IS_ERR(cpage))
> + return ERR_CAST(cpage);
> +
> + paddr = fscrypt_get_symlink(inode, page_address(cpage),
> + BTRFS_MAX_INLINE_DATA_SIZE(fs_info), done);
> + put_page(cpage);
> + return paddr;
> +}
> +
> static struct btrfs_trans_handle *insert_prealloc_file_extent(
> struct btrfs_trans_handle *trans_in,
> struct btrfs_inode *inode,
> @@ -11082,7 +11140,7 @@ static const struct inode_operations btrfs_special_inode_operations = {
> .update_time = btrfs_update_time,
> };
> static const struct inode_operations btrfs_symlink_inode_operations = {
> - .get_link = page_get_link,
> + .get_link = btrfs_get_link,
> .getattr = btrfs_getattr,
> .setattr = btrfs_setattr,
> .permission = btrfs_permission,
> @@ -11092,4 +11150,7 @@ static const struct inode_operations btrfs_symlink_inode_operations = {
>
> const struct dentry_operations btrfs_dentry_operations = {
> .d_delete = btrfs_dentry_delete,
> +#ifdef CONFIG_FS_ENCRYPTION
> + .d_revalidate = fscrypt_d_revalidate,
> +#endif
> };
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index cffdd6f7f8e8..08b1e2ded5be 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -48,6 +48,7 @@
> #include "tests/btrfs-tests.h"
> #include "block-group.h"
> #include "discard.h"
> +#include "fscrypt.h"
> #include "qgroup.h"
> #include "raid56.h"
> #include "fs.h"
> @@ -1144,6 +1145,7 @@ static int btrfs_fill_super(struct super_block *sb,
> sb->s_vop = &btrfs_verityops;
> #endif
> sb->s_xattr = btrfs_xattr_handlers;
> + fscrypt_set_ops(sb, &btrfs_fscrypt_ops);
> sb->s_time_gran = 1;
> #ifdef CONFIG_BTRFS_FS_POSIX_ACL
> sb->s_flags |= SB_POSIXACL;
> --
>
> 2.40.1
>
next prev parent reply other threads:[~2023-07-17 15:38 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-17 3:52 [PATCH v2 00/17] btrfs: add encryption feature Sweet Tea Dorminy
2023-07-17 3:52 ` [PATCH v2 01/17] btrfs: disable various operations on encrypted inodes Sweet Tea Dorminy
2023-07-17 3:52 ` [PATCH v2 02/17] btrfs: disable verity " Sweet Tea Dorminy
2023-07-17 3:52 ` [PATCH v2 03/17] fscrypt: expose fscrypt_nokey_name Sweet Tea Dorminy
2023-07-17 3:52 ` [PATCH v2 04/17] btrfs: start using fscrypt hooks Sweet Tea Dorminy
2023-07-17 15:34 ` Luís Henriques [this message]
2023-07-17 17:28 ` David Sterba
2023-07-18 8:36 ` Luís Henriques
2023-07-17 3:52 ` [PATCH v2 05/17] btrfs: add inode encryption contexts Sweet Tea Dorminy
2023-07-17 15:41 ` Josef Bacik
2023-07-17 3:52 ` [PATCH v2 06/17] btrfs: add new FEATURE_INCOMPAT_ENCRYPT flag Sweet Tea Dorminy
2023-07-17 15:42 ` Josef Bacik
2023-07-17 3:52 ` [PATCH v2 07/17] btrfs: adapt readdir for encrypted and nokey names Sweet Tea Dorminy
2023-07-17 15:34 ` Luís Henriques
2023-07-17 17:46 ` Josef Bacik
2023-07-17 3:52 ` [PATCH v2 08/17] btrfs: use correct name hash for " Sweet Tea Dorminy
2023-07-17 3:52 ` [PATCH v2 09/17] btrfs: implement fscrypt ioctls Sweet Tea Dorminy
2023-07-17 3:52 ` [PATCH v2 10/17] btrfs: add encryption to CONFIG_BTRFS_DEBUG Sweet Tea Dorminy
2023-07-17 3:52 ` [PATCH v2 11/17] btrfs: add get_devices hook for fscrypt Sweet Tea Dorminy
2023-07-17 17:51 ` Josef Bacik
2023-07-17 3:52 ` [PATCH v2 12/17] btrfs: turn on inlinecrypt mount option for encrypt Sweet Tea Dorminy
2023-07-17 15:34 ` Luís Henriques
2023-07-17 17:55 ` Josef Bacik
2023-07-17 3:52 ` [PATCH v2 13/17] btrfs: turn on the encryption ioctls Sweet Tea Dorminy
2023-07-17 3:52 ` [PATCH v2 14/17] btrfs: create and free extent fscrypt_infos Sweet Tea Dorminy
2023-07-17 17:58 ` Josef Bacik
2023-07-17 3:52 ` [PATCH v2 15/17] btrfs: start tracking extent encryption context info Sweet Tea Dorminy
2023-07-17 18:11 ` Josef Bacik
2023-07-17 3:52 ` [PATCH v2 16/17] btrfs: explicitly track file extent length and encryption Sweet Tea Dorminy
2023-07-17 15:30 ` Josef Bacik
2023-07-17 18:12 ` Josef Bacik
2023-07-17 3:52 ` [PATCH v2 17/17] btrfs: save and load fscrypt extent contexts Sweet Tea Dorminy
2023-07-17 18:15 ` Josef Bacik
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=875y6iwnsr.fsf@suse.de \
--to=lhenriques@suse.de \
--cc=clm@fb.com \
--cc=dsterba@suse.com \
--cc=ebiggers@kernel.org \
--cc=jaegeuk@kernel.org \
--cc=josef@toxicpanda.com \
--cc=kernel-team@meta.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-fscrypt@vger.kernel.org \
--cc=osandov@osandov.com \
--cc=sweettea-kernel@dorminy.me \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.