On 22.11.19 09:13, Herbert Xu wrote: > On Wed, Nov 13, 2019 at 11:55:22AM +0100, Harald Freudenberger wrote: >> @@ -129,6 +128,7 @@ static int ecb_paes_init(struct crypto_skcipher *tfm) >> struct s390_paes_ctx *ctx = crypto_skcipher_ctx(tfm); >> >> ctx->kb.key = NULL; >> + spin_lock_init(&ctx->pk_lock); > This makes no sense. The context is per-tfm, and each tfm should > have a single key at any time. The synchronisation of setkey vs. > crypto operations is left to the user of the tfm, not the > implementor. > > So why do you need this spin lock at all? > > Cheers, The setkey() sets the base key material (usually a secure key) to an tfm instance. From this key a 'protected key' (pkey) is derived which may get invalid at any time and may need to get re-derived from the base key material. An tfm instance may be shared, so the context where the pkey is stored into is also shared. So when a pkey gets invalid there is a need to update the pkey value within the context struct. This update needs to be done atomic as another thread may concurrently use this pkey value. That's all what this spinlock does. Make sure read and write operations on the pkey within the context are atomic. It is still possible that two threads copy the pkey, try to use it, find out that it is invalid and needs refresh, re-derive and both update the pkey memory serialized by the spinlock. But this is no issue. The spinlock makes sure the stored pkey is always a consistent pkey (which may be valid or invalid but not corrupted). Hope this makes it clear Thanks
