Re: [PATCH v10 2/5] s390/crypto: New s390 specific protected key hash phmac

[Harald Freudenberger <freude@linux.ibm.com>]

On 2025-02-09 17:34, Eric Biggers wrote:
> On Sun, Feb 09, 2025 at 04:47:57PM +0800, Herbert Xu wrote:
>> On Wed, Jan 15, 2025 at 05:22:28PM +0100, Harald Freudenberger wrote:
>> >
>> > +static int s390_phmac_init(struct ahash_request *req)
>> > +{
>> > +	struct s390_phmac_req_ctx *req_ctx = ahash_request_ctx(req);
>> > +	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
>> > +	struct s390_kmac_sha2_ctx *ctx = &req_ctx->sha2_ctx;
>> > +	int rc;
>> > +
>> > +	/*
>> > +	 * First try synchronous. If this fails for any reason
>> > +	 * schedule this request asynchronous via workqueue.
>> > +	 */
>> > +
>> > +	rc = phmac_init(tfm, ctx, false);
>> > +	if (!rc)
>> > +		goto out;
>> > +
>> > +	req_ctx->req = req;
>> > +	INIT_DELAYED_WORK(&req_ctx->work, phmac_wq_init_fn);
>> > +	schedule_delayed_work(&req_ctx->work, 0);
>> > +	rc = -EINPROGRESS;
>> 
>> This creates a resource problem because there is no limit on how
>> many requests that can be delayed in this manner for a given tfm.
>> 
>> When we hit this case, I presume this is a system-wide issue and
>> all requests would go pending? If that is the case, I suggest
>> allocating a system-wide queue through crypto_engine and using
>> that to limit how many requests that can become EINPROGRESS.
> 
> Or just make it synchronous which would be way easier, and the calling 
> code uses
> it synchronously anyway.
> 
> - Eric

A word about synchronous vs asynchronous...

As a synchronous hash (or chipher or whatever) MUST NOT sleep I can't
really implement the pkey stuff in a synchronous way:

The issue with pkey (We call it "protected key") is that it is some kind
of hardware based key. As such it needs some special preparation action
to be done upfront in the hardware/firmware to use such a pkey.
Now think about KVM live guest migration where a guest suddenly awakes
(Well the guest is not even aware of this) on a new machine with another
hardware. So out of the sudden a hardware based crypto operation fails
with an indication that the hardware/firmware can't deal with this
key object and needs re-preparation. Usually this preparation step is
some kind of asynchronous operation (write some pci registers or run
some DMA sequences or refresh the working key material via an HSM
communication...) and as such may take some time and involve even
sleeping on a mutex or completion until another kernel thread is done.
Please note this is not unique to pkey on system z but may apply
to all kinds of hardware/firmware based keys in situations like
KVM live guest migration or suspend/resume.

So for the pkey algorithms I can't guarantee that all the crypto
operations do never sleep and thus an asynchronous implementation
is the only way and makes sense to me.

There are other downsides with the asynch implementations:
They are much more complex and thus expensive and - how the hell can a
test cover all the code branches?
Next is the thing with the CRYPTO_ALG_ALLOCATES_MEMORY flag. If I
want to have a hash implementation usable for dm-integrity the alg
implementation must NOT set this flag and must not allocate any memory
during crypto operations (includung setkey()). As of now I am still not
through with the phmac code for this. And I only have a faint idea on
how to implement this on the pkey and maybe zcrypt code...

kind regards
Harald Freudenberger