kernel/linux.git/fs/crypto/keyring.c, branch v6.6.131

fscrypt: use WARN_ON_ONCE instead of WARN_ON

2023-03-28T04:15:50+00:00

As per Linus's suggestion (https://lore.kernel.org/r/CAHk-=whefxRGyNGzCzG6BVeM=5vnvgb-XhSeFJVxJyAxAF8XRA@mail.gmail.com), use WARN_ON_ONCE instead of WARN_ON. This barely adds any extra overhead, and it makes it so that if any of these ever becomes reachable (they shouldn't, but that's the point), the logs can't be flooded. Link: https://lore.kernel.org/r/20230320233943.73600-1-ebiggers@kernel.org Signed-off-by: Eric Biggers

fscrypt: check for NULL keyring in fscrypt_put_master_key_activeref()

2023-03-19T04:08:03+00:00

It is a bug for fscrypt_put_master_key_activeref() to see a NULL keyring. But it used to be possible due to the bug, now fixed, where fscrypt_destroy_keyring() was called before security_sb_delete(). To be consistent with how fscrypt_destroy_keyring() uses WARN_ON for the same issue, WARN and leak the fscrypt_master_key if the keyring is NULL instead of dereferencing the NULL pointer. This is a robustness improvement, not a fix. Link: https://lore.kernel.org/r/20230313221231.272498-4-ebiggers@kernel.org Signed-off-by: Eric Biggers

fscrypt: improve fscrypt_destroy_keyring() documentation

2023-03-19T04:08:00+00:00

Document that fscrypt_destroy_keyring() must be called after all potentially-encrypted inodes have been evicted. Link: https://lore.kernel.org/r/20230313221231.272498-3-ebiggers@kernel.org Signed-off-by: Eric Biggers

fscrypt: clean up fscrypt_add_test_dummy_key()

2023-02-08T06:30:30+00:00

Now that fscrypt_add_test_dummy_key() is only called by setup_file_encryption_key() and not by the individual filesystems, un-export it. Also change its prototype to take the fscrypt_key_specifier directly, as the caller already has it. Signed-off-by: Eric Biggers Link: https://lore.kernel.org/r/20230208062107.199831-6-ebiggers@kernel.org

fscrypt: pass super_block to fscrypt_put_master_key_activeref()

2022-11-16T01:19:29+00:00

As this code confused Linus [1], pass the super_block as an argument to fscrypt_put_master_key_activeref(). This removes the need to have the back-pointer ->mk_sb, so remove that. [1] https://lore.kernel.org/linux-fscrypt/CAHk-=wgud4Bc_um+htgfagYpZAnOoCb3NUoW67hc9LhOKsMtJg@mail.gmail.com Signed-off-by: Eric Biggers Link: https://lore.kernel.org/r/20221110082942.351615-1-ebiggers@kernel.org

fscrypt: fix keyring memory leak on mount failure

2022-10-20T03:54:43+00:00

Commit d7e7b9af104c ("fscrypt: stop using keyrings subsystem for fscrypt_master_key") moved the keyring destruction from __put_super() to generic_shutdown_super() so that the filesystem's block device(s) are still available. Unfortunately, this causes a memory leak in the case where a mount is attempted with the test_dummy_encryption mount option, but the mount fails after the option has already been processed. To fix this, attempt the keyring destruction in both places. Reported-by: syzbot+104c2a89561289cec13e@syzkaller.appspotmail.com Fixes: d7e7b9af104c ("fscrypt: stop using keyrings subsystem for fscrypt_master_key") Signed-off-by: Eric Biggers Reviewed-by: Christian Brauner (Microsoft) Link: https://lore.kernel.org/r/20221011213838.209879-1-ebiggers@kernel.org

fscrypt: stop holding extra request_queue references

2022-09-22T03:33:06+00:00

Now that the fscrypt_master_key lifetime has been reworked to not be subject to the quirks of the keyrings subsystem, blk_crypto_evict_key() no longer gets called after the filesystem has already been unmounted. Therefore, there is no longer any need to hold extra references to the filesystem's request_queue(s). (And these references didn't always do their intended job anyway, as pinning a request_queue doesn't necessarily pin the corresponding blk_crypto_profile.) Stop taking these extra references. Instead, just pass the super_block to fscrypt_destroy_inline_crypt_key(), and use it to get the list of block devices the key needs to be evicted from. Signed-off-by: Eric Biggers Link: https://lore.kernel.org/r/20220901193208.138056-3-ebiggers@kernel.org

fscrypt: stop using keyrings subsystem for fscrypt_master_key

2022-09-22T03:33:06+00:00

The approach of fs/crypto/ internally managing the fscrypt_master_key structs as the payloads of "struct key" objects contained in a "struct key" keyring has outlived its usefulness. The original idea was to simplify the code by reusing code from the keyrings subsystem. However, several issues have arisen that can't easily be resolved: - When a master key struct is destroyed, blk_crypto_evict_key() must be called on any per-mode keys embedded in it. (This started being the case when inline encryption support was added.) Yet, the keyrings subsystem can arbitrarily delay the destruction of keys, even past the time the filesystem was unmounted. Therefore, currently there is no easy way to call blk_crypto_evict_key() when a master key is destroyed. Currently, this is worked around by holding an extra reference to the filesystem's request_queue(s). But it was overlooked that the request_queue reference is *not* guaranteed to pin the corresponding blk_crypto_profile too; for device-mapper devices that support inline crypto, it doesn't. This can cause a use-after-free. - When the last inode that was using an incompletely-removed master key is evicted, the master key removal is completed by removing the key struct from the keyring. Currently this is done via key_invalidate(). Yet, key_invalidate() takes the key semaphore. This can deadlock when called from the shrinker, since in fscrypt_ioctl_add_key(), memory is allocated with GFP_KERNEL under the same semaphore. - More generally, the fact that the keyrings subsystem can arbitrarily delay the destruction of keys (via garbage collection delay, or via random processes getting temporary key references) is undesirable, as it means we can't strictly guarantee that all secrets are ever wiped. - Doing the master key lookups via the keyrings subsystem results in the key_permission LSM hook being called. fscrypt doesn't want this, as all access control for encrypted files is designed to happen via the files themselves, like any other files. The workaround which SELinux users are using is to change their SELinux policy to grant key search access to all domains. This works, but it is an odd extra step that shouldn't really have to be done. The fix for all these issues is to change the implementation to what I should have done originally: don't use the keyrings subsystem to keep track of the filesystem's fscrypt_master_key structs. Instead, just store them in a regular kernel data structure, and rework the reference counting, locking, and lifetime accordingly. Retain support for RCU-mode key lookups by using a hash table. Replace fscrypt_sb_free() with fscrypt_sb_delete(), which releases the keys synchronously and runs a bit earlier during unmount, so that block devices are still available. A side effect of this patch is that neither the master keys themselves nor the filesystem keyrings will be listed in /proc/keys anymore. ("Master key users" and the master key users keyrings will still be listed.) However, this was mostly an implementation detail, and it was intended just for debugging purposes. I don't know of anyone using it. This patch does *not* change how "master key users" (->mk_users) works; that still uses the keyrings subsystem. That is still needed for key quotas, and changing that isn't necessary to solve the issues listed above. If we decide to change that too, it would be a separate patch. I've marked this as fixing the original commit that added the fscrypt keyring, but as noted above the most important issue that this patch fixes wasn't introduced until the addition of inline encryption support. Fixes: 22d94f493bfb ("fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl") Signed-off-by: Eric Biggers Link: https://lore.kernel.org/r/20220901193208.138056-2-ebiggers@kernel.org

fscrypt: add new helper functions for test_dummy_encryption

2022-05-09T23:18:54+00:00

Unfortunately the design of fscrypt_set_test_dummy_encryption() doesn't work properly for the new mount API, as it combines too many steps into one function: - Parse the argument to test_dummy_encryption - Check the setting against the filesystem instance - Apply the setting to the filesystem instance The new mount API has split these into separate steps. ext4 partially worked around this by duplicating some of the logic, but it still had some bugs. To address this, add some new helper functions that split up the steps of fscrypt_set_test_dummy_encryption(): - fscrypt_parse_test_dummy_encryption() - fscrypt_dummy_policies_equal() - fscrypt_add_test_dummy_key() While we're add it, also add a function fscrypt_is_dummy_policy_set() which will be useful to avoid some #ifdef's. Signed-off-by: Eric Biggers Link: https://lore.kernel.org/r/20220501050857.538984-5-ebiggers@kernel.org

fscrypt: simplify master key locking

2020-11-24T23:29:47+00:00

The stated reasons for separating fscrypt_master_key::mk_secret_sem from the standard semaphore contained in every 'struct key' no longer apply. First, due to commit a992b20cd4ee ("fscrypt: add fscrypt_prepare_new_inode() and fscrypt_set_context()"), fscrypt_get_encryption_info() is no longer called from within a filesystem transaction. Second, due to commit d3ec10aa9581 ("KEYS: Don't write out to userspace while holding key semaphore"), the semaphore for the "keyring" key type no longer ranks above page faults. That leaves performance as the only possible reason to keep the separate mk_secret_sem. Specifically, having mk_secret_sem reduces the contention between setup_file_encryption_key() and FS_IOC_{ADD,REMOVE}_ENCRYPTION_KEY. However, these ioctls aren't executed often, so this doesn't seem to be worth the extra complexity. Therefore, simplify the locking design by just using key->sem instead of mk_secret_sem. Link: https://lore.kernel.org/r/20201117032626.320275-1-ebiggers@kernel.org Signed-off-by: Eric Biggers