On Wed, May 13, 2026 at 10:52:38AM +0200, Daniel Vacek wrote:
> From: Josef Bacik <[email protected]>
>
> Previously we were wiping the master key secret when we do
> FS_IOC_REMOVE_ENCRYPTION_KEY, and then using the fact that it was
> cleared as the mechanism from keeping new users from being setup.
This seems to be describing the state prior to commit 15baf55481de, not
the current tip of tree. We do still wipe on
FS_IOC_REMOVE_ENCRYPTION_KEY, but there's a separate boolean that
maintains the status and everyone looks at that now.
> * FSCRYPT_KEY_STATUS_INCOMPLETELY_REMOVED
> * Removal of this key has been initiated, but some inodes that were
> - * unlocked with it are still in-use. Like ABSENT, ->mk_secret is wiped,
> - * and the key can no longer be used to unlock inodes. Unlike ABSENT, the
> - * key is still in the keyring; ->mk_decrypted_inodes is nonempty; and
> + * unlocked with it are still in-use.
> + * For filesystems using per-extent encryption ->mk_secret is still
> + * being kept as the per-extent keys are derived at writeout time.
> + * Otherwise, like ABSENT, ->mk_secret is wiped, and the key can
> + * no longer be used to unlock inodes. Unlike ABSENT, the key is
> + * still in the keyring; ->mk_decrypted_inodes is nonempty; and
> * ->mk_active_refs > 0, being equal to the size of ->mk_decrypted_inodes.
This is saying that a bunch of things don't apply to extent-based
encryption, due to being after the "Otherwise," when in fact they
actually still do. So the wording here needs improvement. How about:
->mk_secret exists only if the filesystem uses extent-based
encryption, to support key derivation during file data writeback;
otherwise it is wiped. Either way, the key can no longer be used to
unlock inodes. Unlike ABSENT, the key is still in the keyring;
->mk_decrypted_inodes is nonempty; and ->mk_active_refs
> 0, being equal to the size of ->mk_decrypted_inodes.
> *
> * This state transitions to ABSENT if ->mk_decrypted_inodes becomes empty,
> diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
> index be8e6e8011f2..796e02a0db25 100644
> --- a/fs/crypto/keyring.c
> +++ b/fs/crypto/keyring.c
> @@ -110,6 +110,14 @@ void fscrypt_put_master_key_activeref(struct super_block
> *sb,
> WARN_ON_ONCE(mk->mk_present);
> WARN_ON_ONCE(!list_empty(&mk->mk_decrypted_inodes));
>
> + /* We can't wipe the master key secret until the last activeref is
> + * dropped on the master key with per-extent encryption since the key
> + * derivation continues to happen as long as there are active refs.
> + * Wipe it here now that we're done using it.
> + */
> + if (sb->s_cop->has_per_extent_encryption)
> + wipe_master_key_secret(&mk->mk_secret);
wipe_master_key_secret() is idempotent, so we might as well just do it
unconditionally here.
- Eric
