[PATCH v1] crypto: s390/phmac - Do not modify the req->nbytes value

[PATCH v1] crypto: s390/phmac - Do not modify the req->nbytes value

[Harald Freudenberger]

There was a failure reported by the phmac only in combination
with dm-crypt where the phmac is used as the integrity check
mechanism. A pseudo phmac which was implemented just as an
asynchronous wrapper around the synchronous hmac algorithm did
not show this failure. After some debugging the reason is clear:
The crypto aead layer obvious uses the req->nbytes value after
the verification algorithm is through and finished with the
request. If the req->nbytes value has been modified the aead
layer will react with -EBADMSG to the caller (dm-crypt).

Unfortunately the phmac implementation used the req->nbytes
field on combined operations (finup, digest) to track the
state: with req->nbytes > 0 the update needs to be processed,
while req->nbytes == 0 means to do the final operation. For
this purpose the req->nbytes field was set to 0 after successful
update operation. This worked fine and all tests succeeded but
only failed with aead use as dm-crypt with verify uses it.

Fixed by a slight rework on the phmac implementation. There is
now a new field async_op in the request context which tracks
the (asynch) operation to process. So the 'state' via req->nbytes
is not needed any more and now this field is untouched and may
be evaluated even after a request is processed by the phmac
implementation.

Fixes: cbbc675506cc ("crypto: s390 - New s390 specific protected key hash phmac")
Reported-by: Ingo Franzki <ifranzki@linux.ibm.com>
Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
---
 arch/s390/crypto/phmac_s390.c | 52 +++++++++++++++++++++++------------
 1 file changed, 34 insertions(+), 18 deletions(-)

diff --git a/arch/s390/crypto/phmac_s390.c b/arch/s390/crypto/phmac_s390.c
index 7ecfdc4fba2d..5d38a48cc45d 100644
--- a/arch/s390/crypto/phmac_s390.c
+++ b/arch/s390/crypto/phmac_s390.c
@@ -169,11 +169,18 @@ struct kmac_sha2_ctx {
 	u64 buflen[2];
 };
 
+enum async_op {
+	OP_NOP = 0,
+	OP_UPDATE,
+	OP_FINAL,
+	OP_FINUP,
+};
+
 /* phmac request context */
 struct phmac_req_ctx {
 	struct hash_walk_helper hwh;
 	struct kmac_sha2_ctx kmac_ctx;
-	bool final;
+	int async_op;
 };
 
 /*
@@ -610,6 +617,7 @@ static int phmac_update(struct ahash_request *req)
 	 * using engine to serialize requests.
 	 */
 	if (rc == 0 || rc == -EKEYEXPIRED) {
+		req_ctx->async_op = OP_UPDATE;
 		atomic_inc(&tfm_ctx->via_engine_ctr);
 		rc = crypto_transfer_hash_request_to_engine(phmac_crypto_engine, req);
 		if (rc != -EINPROGRESS)
@@ -647,8 +655,7 @@ static int phmac_final(struct ahash_request *req)
 	 * using engine to serialize requests.
 	 */
 	if (rc == 0 || rc == -EKEYEXPIRED) {
-		req->nbytes = 0;
-		req_ctx->final = true;
+		req_ctx->async_op = OP_FINAL;
 		atomic_inc(&tfm_ctx->via_engine_ctr);
 		rc = crypto_transfer_hash_request_to_engine(phmac_crypto_engine, req);
 		if (rc != -EINPROGRESS)
@@ -676,13 +683,16 @@ static int phmac_finup(struct ahash_request *req)
 	if (rc)
 		goto out;
 
+	req_ctx->async_op = OP_FINUP;
+
 	/* Try synchronous operations if no active engine usage */
 	if (!atomic_read(&tfm_ctx->via_engine_ctr)) {
 		rc = phmac_kmac_update(req, false);
 		if (rc == 0)
-			req->nbytes = 0;
+			req_ctx->async_op = OP_FINAL;
 	}
-	if (!rc && !req->nbytes && !atomic_read(&tfm_ctx->via_engine_ctr)) {
+	if (!rc && req_ctx->async_op == OP_FINAL &&
+	    !atomic_read(&tfm_ctx->via_engine_ctr)) {
 		rc = phmac_kmac_final(req, false);
 		if (rc == 0)
 			goto out;
@@ -694,7 +704,7 @@ static int phmac_finup(struct ahash_request *req)
 	 * using engine to serialize requests.
 	 */
 	if (rc == 0 || rc == -EKEYEXPIRED) {
-		req_ctx->final = true;
+		/* req->async_op has been set to either OP_FINUP or OP_FINAL */
 		atomic_inc(&tfm_ctx->via_engine_ctr);
 		rc = crypto_transfer_hash_request_to_engine(phmac_crypto_engine, req);
 		if (rc != -EINPROGRESS)
@@ -855,15 +865,16 @@ static int phmac_do_one_request(struct crypto_engine *engine, void *areq)
 
 	/*
 	 * Three kinds of requests come in here:
-	 * update when req->nbytes > 0 and req_ctx->final is false
-	 * final when req->nbytes = 0 and req_ctx->final is true
-	 * finup when req->nbytes > 0 and req_ctx->final is true
-	 * For update and finup the hwh walk needs to be prepared and
-	 * up to date but the actual nr of bytes in req->nbytes may be
-	 * any non zero number. For final there is no hwh walk needed.
+	 * 1. req->async_op == OP_UPDATE with req->nbytes > 0
+	 * 2. req->async_op == OP_FINUP with req->nbytes > 0
+	 * 3. req->async_op == OP_FINAL
+	 * For update and finup the hwh walk has already been prepared
+	 * by the caller. For final there is no hwh walk needed.
 	 */
 
-	if (req->nbytes) {
+	switch (req_ctx->async_op) {
+	case OP_UPDATE:
+	case OP_FINUP:
 		rc = phmac_kmac_update(req, true);
 		if (rc == -EKEYEXPIRED) {
 			/*
@@ -880,10 +891,11 @@ static int phmac_do_one_request(struct crypto_engine *engine, void *areq)
 			hwh_advance(hwh, rc);
 			goto out;
 		}
-		req->nbytes = 0;
-	}
-
-	if (req_ctx->final) {
+		if (req_ctx->async_op == OP_UPDATE)
+			break;
+		req_ctx->async_op = OP_FINAL;
+		fallthrough;
+	case OP_FINAL:
 		rc = phmac_kmac_final(req, true);
 		if (rc == -EKEYEXPIRED) {
 			/*
@@ -897,10 +909,14 @@ static int phmac_do_one_request(struct crypto_engine *engine, void *areq)
 			cond_resched();
 			return -ENOSPC;
 		}
+		break;
+	default:
+		/* unknown/unsupported/unimplemented asynch op */
+		return -EOPNOTSUPP;
 	}
 
 out:
-	if (rc || req_ctx->final)
+	if (rc || req_ctx->async_op == OP_FINAL)
 		memzero_explicit(kmac_ctx, sizeof(*kmac_ctx));
 	pr_debug("request complete with rc=%d\n", rc);
 	local_bh_disable();
-- 
2.43.0

Re: [PATCH v1] crypto: s390/phmac - Do not modify the req->nbytes value

[Ingo Franzki]

On 09.10.2025 18:01, Harald Freudenberger wrote:
> There was a failure reported by the phmac only in combination
> with dm-crypt where the phmac is used as the integrity check
> mechanism. A pseudo phmac which was implemented just as an
> asynchronous wrapper around the synchronous hmac algorithm did
> not show this failure. After some debugging the reason is clear:
> The crypto aead layer obvious uses the req->nbytes value after
> the verification algorithm is through and finished with the
> request. If the req->nbytes value has been modified the aead
> layer will react with -EBADMSG to the caller (dm-crypt).
> 
> Unfortunately the phmac implementation used the req->nbytes
> field on combined operations (finup, digest) to track the
> state: with req->nbytes > 0 the update needs to be processed,
> while req->nbytes == 0 means to do the final operation. For
> this purpose the req->nbytes field was set to 0 after successful
> update operation. This worked fine and all tests succeeded but
> only failed with aead use as dm-crypt with verify uses it.
> 
> Fixed by a slight rework on the phmac implementation. There is
> now a new field async_op in the request context which tracks
> the (asynch) operation to process. So the 'state' via req->nbytes
> is not needed any more and now this field is untouched and may
> be evaluated even after a request is processed by the phmac
> implementation.
> 
> Fixes: cbbc675506cc ("crypto: s390 - New s390 specific protected key hash phmac")
> Reported-by: Ingo Franzki <ifranzki@linux.ibm.com>
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
> ---
>  arch/s390/crypto/phmac_s390.c | 52 +++++++++++++++++++++++------------
>  1 file changed, 34 insertions(+), 18 deletions(-)

I can confirm that this fixes the failures reported at https://lore.kernel.org/dm-devel/fec83aad-c38b-4617-bb9a-0b9827125d79@linux.ibm.com/T/#u
Also, having the operation in the request context makes the code even more clear IMHO.

Tested-by: Ingo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: Ingo Franzki <ifranzki@linux.ibm.com>


-- 
Ingo Franzki
eMail: ifranzki@linux.ibm.com  
Linux on IBM Z Development, Schoenaicher Str. 220, 71032 Boeblingen, Germany

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM DATA Privacy Statement: https://www.ibm.com/privacy/us/en/

Re: [PATCH v1] crypto: s390/phmac - Do not modify the req->nbytes value

[Ingo Franzki]

On 10.10.2025 09:55, Ingo Franzki wrote:
> On 09.10.2025 18:01, Harald Freudenberger wrote:
>> There was a failure reported by the phmac only in combination
>> with dm-crypt where the phmac is used as the integrity check
>> mechanism. A pseudo phmac which was implemented just as an
>> asynchronous wrapper around the synchronous hmac algorithm did
>> not show this failure. After some debugging the reason is clear:
>> The crypto aead layer obvious uses the req->nbytes value after
>> the verification algorithm is through and finished with the
>> request. If the req->nbytes value has been modified the aead
>> layer will react with -EBADMSG to the caller (dm-crypt).

I guess the place where nbytes is used after the verification is in
crypto_authenc_decrypt_tail().  This is called either directly by
crypto_authenc_decrypt() if crypto_ahash_digest() completed synchronously,
or in case or async completion, via the authenc_verify_ahash_done() callback.

crypto_authenc_decrypt_tail() then does:

	scatterwalk_map_and_copy(ihash, req->src, ahreq->nbytes, authsize, 0);

	if (crypto_memneq(ihash, ahreq->result, authsize))
		return -EBADMSG;

Where the scatterwalk_map_and_copy() uses the ahreq->nbytes value to know the
amount of bytes to copy. If this is zero then the copied ihash value is wrong and
thus it returns -EBADMSG.

Not sure if the number of bytes could be saved elsewhere, to allow that the
requests's nbytes can be modified by the digest (if this is desired to be allowed)?
Otherwise, a comment in the header file that nbytes must not be modified might be useful.

>>
>> Unfortunately the phmac implementation used the req->nbytes
>> field on combined operations (finup, digest) to track the
>> state: with req->nbytes > 0 the update needs to be processed,
>> while req->nbytes == 0 means to do the final operation. For
>> this purpose the req->nbytes field was set to 0 after successful
>> update operation. This worked fine and all tests succeeded but
>> only failed with aead use as dm-crypt with verify uses it.
>>
>> Fixed by a slight rework on the phmac implementation. There is
>> now a new field async_op in the request context which tracks
>> the (asynch) operation to process. So the 'state' via req->nbytes
>> is not needed any more and now this field is untouched and may
>> be evaluated even after a request is processed by the phmac
>> implementation.
>>
>> Fixes: cbbc675506cc ("crypto: s390 - New s390 specific protected key hash phmac")
>> Reported-by: Ingo Franzki <ifranzki@linux.ibm.com>
>> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
>> ---
>>  arch/s390/crypto/phmac_s390.c | 52 +++++++++++++++++++++++------------
>>  1 file changed, 34 insertions(+), 18 deletions(-)
> 
> I can confirm that this fixes the failures reported at https://lore.kernel.org/dm-devel/fec83aad-c38b-4617-bb9a-0b9827125d79@linux.ibm.com/T/#u
> Also, having the operation in the request context makes the code even more clear IMHO.
> 
> Tested-by: Ingo Franzki <ifranzki@linux.ibm.com>
> Reviewed-by: Ingo Franzki <ifranzki@linux.ibm.com>
> 
> 


-- 
Ingo Franzki
eMail: ifranzki@linux.ibm.com  
Linux on IBM Z Development, Schoenaicher Str. 220, 71032 Boeblingen, Germany

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM DATA Privacy Statement: https://www.ibm.com/privacy/us/en/

Re: [PATCH v1] crypto: s390/phmac - Do not modify the req->nbytes value

[Holger Dengler]

On 09/10/2025 18:01, Harald Freudenberger wrote:
> There was a failure reported by the phmac only in combination
> with dm-crypt where the phmac is used as the integrity check
> mechanism. A pseudo phmac which was implemented just as an
> asynchronous wrapper around the synchronous hmac algorithm did
> not show this failure. After some debugging the reason is clear:

In my opinion, the information up to here should not be part of the commit message. If you want to keep it, I would suggest to move it to the cover letter.

> The crypto aead layer obvious uses the req->nbytes value after
> the verification algorithm is through and finished with the
> request. If the req->nbytes value has been modified the aead
> layer will react with -EBADMSG to the caller (dm-crypt).
> 
> Unfortunately the phmac implementation used the req->nbytes
> field on combined operations (finup, digest) to track the
> state: with req->nbytes > 0 the update needs to be processed,
> while req->nbytes == 0 means to do the final operation. For
> this purpose the req->nbytes field was set to 0 after successful
> update operation. This worked fine and all tests succeeded but
> only failed with aead use as dm-crypt with verify uses it.
> 
> Fixed by a slight rework on the phmac implementation. There is
> now a new field async_op in the request context which tracks
> the (asynch) operation to process. So the 'state' via req->nbytes
> is not needed any more and now this field is untouched and may
> be evaluated even after a request is processed by the phmac
> implementation.
> 
> Fixes: cbbc675506cc ("crypto: s390 - New s390 specific protected key hash phmac")
> Reported-by: Ingo Franzki <ifranzki@linux.ibm.com>
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>

See my comments below.

> ---
>  arch/s390/crypto/phmac_s390.c | 52 +++++++++++++++++++++++------------
>  1 file changed, 34 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/s390/crypto/phmac_s390.c b/arch/s390/crypto/phmac_s390.c
> index 7ecfdc4fba2d..5d38a48cc45d 100644
> --- a/arch/s390/crypto/phmac_s390.c
> +++ b/arch/s390/crypto/phmac_s390.c
> @@ -169,11 +169,18 @@ struct kmac_sha2_ctx {
>  	u64 buflen[2];
>  };
>  
> +enum async_op {
> +	OP_NOP = 0,

The async_op element in struct phmac_req_ctx is implicitly initialized with OP_NOP. Only the functions update, final and finup will set another (valid) operation.  Can it ever happen, that do_one_request() is called *before* any of update, final or finup is called? If this is a valid case, the OP_NOP should be handled correctly in do_one_request(), otherwise we get a -ENOTSUPP (see my comment to phmac_do_one_request()).

If do_one_request() is never called before update/finup/final(), no change is required.

> +	OP_UPDATE,
> +	OP_FINAL,
> +	OP_FINUP,
> +};
> +
>  /* phmac request context */
>  struct phmac_req_ctx {
>  	struct hash_walk_helper hwh;
>  	struct kmac_sha2_ctx kmac_ctx;
> -	bool final;
> +	int async_op;

I know, that the compiler is happy with an int. But I would prefer to use the enum for the element.

enum async_op async_op;

>  };
>  
>  /*
[...]> @@ -855,15 +865,16 @@ static int phmac_do_one_request(struct crypto_engine *engine, void *areq)
>  
>  	/*
>  	 * Three kinds of requests come in here:
> -	 * update when req->nbytes > 0 and req_ctx->final is false
> -	 * final when req->nbytes = 0 and req_ctx->final is true
> -	 * finup when req->nbytes > 0 and req_ctx->final is true
> -	 * For update and finup the hwh walk needs to be prepared and
> -	 * up to date but the actual nr of bytes in req->nbytes may be
> -	 * any non zero number. For final there is no hwh walk needed.
> +	 * 1. req->async_op == OP_UPDATE with req->nbytes > 0
> +	 * 2. req->async_op == OP_FINUP with req->nbytes > 0
> +	 * 3. req->async_op == OP_FINAL
> +	 * For update and finup the hwh walk has already been prepared
> +	 * by the caller. For final there is no hwh walk needed.
>  	 */
>  
> -	if (req->nbytes) {
> +	switch (req_ctx->async_op) {
> +	case OP_UPDATE:
> +	case OP_FINUP:
>  		rc = phmac_kmac_update(req, true);
>  		if (rc == -EKEYEXPIRED) {
>  			/*
> @@ -880,10 +891,11 @@ static int phmac_do_one_request(struct crypto_engine *engine, void *areq)
>  			hwh_advance(hwh, rc);
>  			goto out;
>  		}
> -		req->nbytes = 0;
> -	}
> -
> -	if (req_ctx->final) {
> +		if (req_ctx->async_op == OP_UPDATE)
> +			break;
> +		req_ctx->async_op = OP_FINAL;
> +		fallthrough;
> +	case OP_FINAL:
>  		rc = phmac_kmac_final(req, true);
>  		if (rc == -EKEYEXPIRED) {
>  			/*
> @@ -897,10 +909,14 @@ static int phmac_do_one_request(struct crypto_engine *engine, void *areq)
>  			cond_resched();
>  			return -ENOSPC;
>  		}
> +		break;
> +	default:
> +		/* unknown/unsupported/unimplemented asynch op */
> +		return -EOPNOTSUPP;

If it is a valid case, that do_one_request() is called before update(), final() or finup() is called, we should handle OP_NOP here and not return with an error.
If do_one_request() is never called before update/finup/final(), no change is required.

[...]

-- 
Mit freundlichen Grüßen / Kind regards
Holger Dengler
--
IBM Systems, Linux on IBM Z Development
dengler@linux.ibm.com

Re: [PATCH v1] crypto: s390/phmac - Do not modify the req->nbytes value

[Harald Freudenberger]

On 2025-10-14 11:19, Holger Dengler wrote:
> On 09/10/2025 18:01, Harald Freudenberger wrote:
>> There was a failure reported by the phmac only in combination
>> with dm-crypt where the phmac is used as the integrity check
>> mechanism. A pseudo phmac which was implemented just as an
>> asynchronous wrapper around the synchronous hmac algorithm did
>> not show this failure. After some debugging the reason is clear:
> 
> In my opinion, the information up to here should not be part of the
> commit message. If you want to keep it, I would suggest to move it to
> the cover letter.
> 

Ok, will remove this.

>> The crypto aead layer obvious uses the req->nbytes value after
>> the verification algorithm is through and finished with the
>> request. If the req->nbytes value has been modified the aead
>> layer will react with -EBADMSG to the caller (dm-crypt).
>> 
>> Unfortunately the phmac implementation used the req->nbytes
>> field on combined operations (finup, digest) to track the
>> state: with req->nbytes > 0 the update needs to be processed,
>> while req->nbytes == 0 means to do the final operation. For
>> this purpose the req->nbytes field was set to 0 after successful
>> update operation. This worked fine and all tests succeeded but
>> only failed with aead use as dm-crypt with verify uses it.
>> 
>> Fixed by a slight rework on the phmac implementation. There is
>> now a new field async_op in the request context which tracks
>> the (asynch) operation to process. So the 'state' via req->nbytes
>> is not needed any more and now this field is untouched and may
>> be evaluated even after a request is processed by the phmac
>> implementation.
>> 
>> Fixes: cbbc675506cc ("crypto: s390 - New s390 specific protected key 
>> hash phmac")
>> Reported-by: Ingo Franzki <ifranzki@linux.ibm.com>
>> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
> 
> See my comments below.
> 
>> ---
>>  arch/s390/crypto/phmac_s390.c | 52 
>> +++++++++++++++++++++++------------
>>  1 file changed, 34 insertions(+), 18 deletions(-)
>> 
>> diff --git a/arch/s390/crypto/phmac_s390.c 
>> b/arch/s390/crypto/phmac_s390.c
>> index 7ecfdc4fba2d..5d38a48cc45d 100644
>> --- a/arch/s390/crypto/phmac_s390.c
>> +++ b/arch/s390/crypto/phmac_s390.c
>> @@ -169,11 +169,18 @@ struct kmac_sha2_ctx {
>>  	u64 buflen[2];
>>  };
>> 
>> +enum async_op {
>> +	OP_NOP = 0,
> 
> The async_op element in struct phmac_req_ctx is implicitly initialized
> with OP_NOP. Only the functions update, final and finup will set
> another (valid) operation.  Can it ever happen, that do_one_request()
> is called *before* any of update, final or finup is called? If this is
> a valid case, the OP_NOP should be handled correctly in
> do_one_request(), otherwise we get a -ENOTSUPP (see my comment to
> phmac_do_one_request()).
> 
> If do_one_request() is never called before update/finup/final(), no
> change is required.
> 

Well, do_one_request() is only called via a postponed request pushed
from one of the "front" functions (init/update/final/finup/digest) to
the engine instance. So a request is always first seen by these
functions and these have a chance to update the async_op field.

>> +	OP_UPDATE,
>> +	OP_FINAL,
>> +	OP_FINUP,
>> +};
>> +
>>  /* phmac request context */
>>  struct phmac_req_ctx {
>>  	struct hash_walk_helper hwh;
>>  	struct kmac_sha2_ctx kmac_ctx;
>> -	bool final;
>> +	int async_op;
> 
> I know, that the compiler is happy with an int. But I would prefer to
> use the enum for the element.
> 
> enum async_op async_op;
> 

Catched - my first experiences with C where at a time where enums where
not supported. So I am still not familiar with this kind of stuff :-)

>>  };
>> 
>>  /*
> [...]> @@ -855,15 +865,16 @@ static int phmac_do_one_request(struct
> crypto_engine *engine, void *areq)
>> 
>>  	/*
>>  	 * Three kinds of requests come in here:
>> -	 * update when req->nbytes > 0 and req_ctx->final is false
>> -	 * final when req->nbytes = 0 and req_ctx->final is true
>> -	 * finup when req->nbytes > 0 and req_ctx->final is true
>> -	 * For update and finup the hwh walk needs to be prepared and
>> -	 * up to date but the actual nr of bytes in req->nbytes may be
>> -	 * any non zero number. For final there is no hwh walk needed.
>> +	 * 1. req->async_op == OP_UPDATE with req->nbytes > 0
>> +	 * 2. req->async_op == OP_FINUP with req->nbytes > 0
>> +	 * 3. req->async_op == OP_FINAL
>> +	 * For update and finup the hwh walk has already been prepared
>> +	 * by the caller. For final there is no hwh walk needed.
>>  	 */
>> 
>> -	if (req->nbytes) {
>> +	switch (req_ctx->async_op) {
>> +	case OP_UPDATE:
>> +	case OP_FINUP:
>>  		rc = phmac_kmac_update(req, true);
>>  		if (rc == -EKEYEXPIRED) {
>>  			/*
>> @@ -880,10 +891,11 @@ static int phmac_do_one_request(struct 
>> crypto_engine *engine, void *areq)
>>  			hwh_advance(hwh, rc);
>>  			goto out;
>>  		}
>> -		req->nbytes = 0;
>> -	}
>> -
>> -	if (req_ctx->final) {
>> +		if (req_ctx->async_op == OP_UPDATE)
>> +			break;
>> +		req_ctx->async_op = OP_FINAL;
>> +		fallthrough;
>> +	case OP_FINAL:
>>  		rc = phmac_kmac_final(req, true);
>>  		if (rc == -EKEYEXPIRED) {
>>  			/*
>> @@ -897,10 +909,14 @@ static int phmac_do_one_request(struct 
>> crypto_engine *engine, void *areq)
>>  			cond_resched();
>>  			return -ENOSPC;
>>  		}
>> +		break;
>> +	default:
>> +		/* unknown/unsupported/unimplemented asynch op */
>> +		return -EOPNOTSUPP;
> 
> If it is a valid case, that do_one_request() is called before
> update(), final() or finup() is called, we should handle OP_NOP here
> and not return with an error.
> If do_one_request() is never called before update/finup/final(), no
> change is required.
> 

As wrote above the "front" functions always see a request first before
it is postponed to the engine and appears here. So the OP_NOP case
must not happen here and thus is only covered with the default arm.

> [...]

Re: [PATCH v1] crypto: s390/phmac - Do not modify the req->nbytes value

[Holger Dengler]

On 14/10/2025 12:31, Harald Freudenberger wrote:
> On 2025-10-14 11:19, Holger Dengler wrote:
>> On 09/10/2025 18:01, Harald Freudenberger wrote:
[...]>>> +enum async_op {
>>> +    OP_NOP = 0,
>>
>> The async_op element in struct phmac_req_ctx is implicitly initialized
>> with OP_NOP. Only the functions update, final and finup will set
>> another (valid) operation.  Can it ever happen, that do_one_request()
>> is called *before* any of update, final or finup is called? If this is
>> a valid case, the OP_NOP should be handled correctly in
>> do_one_request(), otherwise we get a -ENOTSUPP (see my comment to
>> phmac_do_one_request()).
>>
>> If do_one_request() is never called before update/finup/final(), no
>> change is required.
>>
> 
> Well, do_one_request() is only called via a postponed request pushed
> from one of the "front" functions (init/update/final/finup/digest) to
> the engine instance. So a request is always first seen by these
> functions and these have a chance to update the async_op field.

Ok, then you can leave it that way.

[...]

-- 
Mit freundlichen Grüßen / Kind regards
Holger Dengler
--
IBM Systems, Linux on IBM Z Development
dengler@linux.ibm.com