Linux Btrfs filesystem development
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
Cc: Chris Mason <clm@fb.com>, Josef Bacik <josef@toxicpanda.com>,
	David Sterba <dsterba@suse.com>,
	"Theodore Y . Ts'o" <tytso@mit.edu>,
	Jaegeuk Kim <jaegeuk@kernel.org>,
	kernel-team@meta.com, linux-btrfs@vger.kernel.org,
	linux-fscrypt@vger.kernel.org
Subject: Re: [PATCH v3 14/16] fscrypt: cache list of inlinecrypt devices
Date: Sat, 12 Aug 2023 15:41:44 -0700	[thread overview]
Message-ID: <20230812224144.GB41642@sol.localdomain> (raw)
In-Reply-To: <62170e01a2c0b107619018c859250c03b6023a57.1691505882.git.sweettea-kernel@dorminy.me>

On Tue, Aug 08, 2023 at 01:08:31PM -0400, Sweet Tea Dorminy wrote:
> btrfs sometimes frees extents while holding a mutex, which makes it
> impossible to free an inlinecrypt prepared key since that requires
> taking a semaphore. Therefore, we will need to offload prepared key
> freeing into an asynchronous process (rcu is insufficient since that can
> run in softirq context which is also incompatible with taking a
> semaphore). In order to avoid use-after-free on the filesystem
> superblock for keys being freed during shutdown, we need to cache the
> list of devices that the key has been loaded into, so that we can later
> remove it without reference to the superblock.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>  fs/crypto/fscrypt_private.h | 13 +++++++++++--
>  fs/crypto/inline_crypt.c    | 20 +++++++++-----------
>  fs/crypto/keysetup.c        |  2 +-
>  3 files changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index 03be2c136c0e..aba83509c735 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -205,6 +205,16 @@ struct fscrypt_prepared_key {
>  	struct crypto_skcipher *tfm;
>  #ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
>  	struct blk_crypto_key *blk_key;
> +
> +	/*
> +	 * The list of devices that have this block key.
> +	 */
> +	struct block_device **devices;
> +
> +	/*
> +	 * The number of devices in @ci_devices.
> +	 */
> +	size_t device_count;
>  #endif
>  	enum fscrypt_prepared_key_type type;
>  };

Well, this is basically reverting commit 22e9947a4b2b, but doing it in a
slightly different way.

I worry about potentially bringing back problems from doing work after the
filesystem has already been unmounted.

Can't you just make the filesystem flush its eviction work items when it is
being unmounted?

- Eric

  reply	other threads:[~2023-08-12 22:41 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-08 17:08 [PATCH v3 00/16] fscrypt: add extent encryption Sweet Tea Dorminy
2023-08-08 17:08 ` [PATCH v3 01/16] fscrypt: factor helper for locking master key Sweet Tea Dorminy
2023-08-09 17:53   ` Josef Bacik
2023-08-08 17:08 ` [PATCH v3 02/16] fscrypt: factor getting info for a specific block Sweet Tea Dorminy
2023-08-08 17:08 ` [PATCH v3 03/16] fscrypt: adjust effective lblks based on extents Sweet Tea Dorminy
2023-08-08 17:08 ` [PATCH v3 04/16] fscrypt: add a super_block pointer to fscrypt_info Sweet Tea Dorminy
2023-08-08 17:08 ` [PATCH v3 05/16] fscrypt: setup leaf inodes for extent encryption Sweet Tea Dorminy
2023-08-08 17:08 ` [PATCH v3 06/16] fscrypt: allow infos to be owned by extents Sweet Tea Dorminy
2023-08-08 17:08 ` [PATCH v3 07/16] fscrypt: use an optional ino equivalent for per-extent infos Sweet Tea Dorminy
2023-08-09 18:51   ` Josef Bacik
2023-08-08 17:08 ` [PATCH v3 08/16] fscrypt: move function call warning of busy inodes Sweet Tea Dorminy
2023-08-08 17:08 ` [PATCH v3 09/16] fscrypt: revamp key removal for extent encryption Sweet Tea Dorminy
2023-08-12 22:54   ` Eric Biggers
2023-08-08 17:08 ` [PATCH v3 10/16] fscrypt: add creation/usage/freeing of per-extent infos Sweet Tea Dorminy
2023-08-08 17:08 ` [PATCH v3 11/16] fscrypt: allow load/save of extent contexts Sweet Tea Dorminy
2023-08-08 17:08 ` [PATCH v3 12/16] fscrypt: save session key credentials for extent infos Sweet Tea Dorminy
2023-08-12 22:34   ` Eric Biggers
2023-08-08 17:08 ` [PATCH v3 13/16] fscrypt: allow multiple extents to reference one info Sweet Tea Dorminy
2023-08-10 15:51   ` Luís Henriques
2023-08-11 16:39     ` Sweet Tea Dorminy
2023-08-08 17:08 ` [PATCH v3 14/16] fscrypt: cache list of inlinecrypt devices Sweet Tea Dorminy
2023-08-12 22:41   ` Eric Biggers [this message]
2023-08-08 17:08 ` [PATCH v3 15/16] fscrypt: allow asynchronous info freeing Sweet Tea Dorminy
2023-08-12 22:43   ` Eric Biggers
2023-08-08 17:08 ` [PATCH v3 16/16] fscrypt: update documentation for per-extent keys Sweet Tea Dorminy
2023-08-09 19:52 ` [PATCH v3 00/16] fscrypt: add extent encryption Josef Bacik
2023-08-10  4:55 ` Eric Biggers
2023-08-10  9:52   ` Neal Gompa
2023-08-10 14:11   ` Sweet Tea Dorminy
2023-08-10 17:17     ` Eric Biggers
2023-08-12 22:15 ` Eric Biggers
2023-08-15 15:12   ` Josef Bacik
2023-08-16  3:37     ` Eric Biggers
2023-08-16 14:42       ` 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=20230812224144.GB41642@sol.localdomain \
    --to=ebiggers@kernel.org \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --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=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox