On 4/11/23 00:05, Eric Biggers wrote:
On Mon, Apr 10, 2023 at 03:40:03PM -0400, Sweet Tea Dorminy wrote:So far, it has sufficed to allocate and prepare the block key or the TFM completely before ever setting the relevant field in the prepared key. This is necessary for mode keys -- because multiple inodes could be trying to set up the same per-mode prepared key at the same time on different threads, we currently must not set the prepared key's tfm or block key pointer until that key is completely set up. Otherwise, another inode could see the key to be present and attempt to use it before it is fully set up. But when using pooled prepared keys, we'll have pre-allocated fields, and if we separate allocating the fields of a prepared key from preparing the fields, that inherently sets the fields before they're ready to use. So, either pooled prepared keys must use different allocation and setup functions, or we can split allocation and preparation for all prepared keys and use some other mechanism to signal that the key is fully prepared. In order to avoid having similar yet different functions, this function adds a new field to the prepared key to explicitly track which parts of it are prepared, setting it explicitly. The same acquire/release semantics are used to check it in the case of shared mode keys; the cost lies in the extra byte per prepared key recording which members are fully prepared. Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@xxxxxxxxxx> --- fs/crypto/fscrypt_private.h | 26 +++++++++++++++----------- fs/crypto/inline_crypt.c | 8 +------- fs/crypto/keysetup.c | 36 ++++++++++++++++++++++++++---------- 3 files changed, 42 insertions(+), 28 deletions(-)I wonder if this is overcomplicating things and we should simply add a new rw_semaphore to struct fscrypt_master_key and use it to protect the per-mode key preparation, instead of trying to keep the fast path lockless? So the flow (for setting up a file that uses a per-mode key) would look like: down_read(&mk->mk_mode_key_prep_sem); if key already prepared, unlock and return up_read(&mk->mk_mode_key_prep_sem); down_write(&mk->mk_mode_key_prep_sem); if key already prepared, unlock and return prepare the key up_write(&mk->mk_mode_key_prep_sem); Lockless algorithms are nice, but we shouldn't take them too far if they cause too much trouble...
You're noting that we only really need preparedness for per-mode keys, and that's a point I didn't explicitly grasp before; everywhere else we know whether it's merely allocated or fully prepared. Two other thoughts on ways we could pull the preparedness out of fscrypt_prepared_key and still keep locklessness:
fscrypt_master_key could setup both block key and tfm (if block key is applicable) when it sets up a prepared key, so we can use just one bit of preparedness information, and keep a bitmap recording which prepared keys are ready in fscrypt_master_key.
Or we could have
struct fscrypt_master_key_prepared_key {
u8 preparedness;
struct fscrypt_prepared_key prep_key;
}
and similarly isolate the preparedness tracking from per-file keys.
Do either of those sound appealing as alternatives to the semaphore?
|