Received-SPF: pass (google.com: best guess record for domain of linux-crypto-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH 2/3] s390/crypto: Rework on paes implementation
To:     Herbert Xu <herbert@gondor.apana.org.au>
Cc:     linux-crypto@vger.kernel.org, ebiggers@kernel.org,
        heiko.carstens@de.ibm.com, gor@linux.ibm.com
References: <20191113105523.8007-1-freude@linux.ibm.com>
 <20191113105523.8007-3-freude@linux.ibm.com>
 <20191122081338.6bdjevtyttpdzzwl@gondor.apana.org.au>
From:   Harald Freudenberger <freude@linux.ibm.com>
Date:   Fri, 22 Nov 2019 10:54:50 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.9.0
MIME-Version: 1.0
In-Reply-To: <20191122081338.6bdjevtyttpdzzwl@gondor.apana.org.au>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Message-Id: <87e9dbee-4024-602c-7717-051df3ac644d@linux.ibm.com>
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

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