On Tue, 2 Jun 2026 at 01:04, Eric Biggers <[email protected]> wrote: > 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.
I see, I'll clear that out. > > * 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. Agreed, this sounds better. I'll use that. > > * > > * 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. While this is true, it also (kind of) documents that the secret should already be wiped out by now in case of !extent encryption. But I can wipe the secret unconditionally here if you insist. --nX > - Eric
