[PATCH v2 0/2] Rework cpacf_kma() function

[PATCH v2 0/2] Rework cpacf_kma() function

[Harald Freudenberger]

The CPACF inline function cpacf_kma() has return type void thus
indicating no direct information about success or failure. The
majori of the other CPACF inline functions do return either
failure/success or the number of bytes processed.

Rework the CPACF inline function cpacf_kma() and the only caller - the
AES GCM implementation in arch/s390/crypto/aes_s390.c to return the
value of the condition code set on completion of the instruction.

@Herbert Xu, @Heiko Carstens: As the main change is in cpacf.h and the
slight rework in aes_s390.c is petty simple I would go with delivering
this patches via s390 subsystem.

Changelog:
v1: initial version
v2: Added Holger's RB - no code changes.

Harald Freudenberger (2):
  s390/cpacf: Rework cpacf_kma() to return condition code
  s390/crypto: Handle cpacf_kma() return code

 arch/s390/crypto/aes_s390.c   | 11 +++++++----
 arch/s390/include/asm/cpacf.h | 24 ++++++++++++++++++------
 2 files changed, 25 insertions(+), 10 deletions(-)


base-commit: df2908090cda368b01ff43709f51890076c56157
--
2.43.0

[PATCH v2 1/2] s390/cpacf: Rework cpacf_kma() to return condition code

[Harald Freudenberger]

The KMA instruction can signal a verification-pattern mismatch (CC 1)
when a protected key is used, or incomplete processing (CC 2) when a
sub-block-size chunk is submitted without the appropriate LAAD/LPC
flag. Both conditions shall be visible to the caller similar like the
other CPACF instructions.

So rework the cpacf_kma() inline function to return the condition code
instead of void. However, CC 3 (partial completion) continues to be
handled internally. Also improve the function to use the CC macros for
better readability. Additionally add an kmsan_unpoison_memory()
statement to reflect the updated memory by hardware.

Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
---
 arch/s390/include/asm/cpacf.h | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/arch/s390/include/asm/cpacf.h b/arch/s390/include/asm/cpacf.h
index 6174552d856d..00aca3194b67 100644
--- a/arch/s390/include/asm/cpacf.h
+++ b/arch/s390/include/asm/cpacf.h
@@ -715,12 +715,20 @@ static inline void cpacf_pckmo(long func, void *param)
  * @src_len: length of src operand in bytes
  * @aad: address of additional authenticated data memory area
  * @aad_len: length of aad operand in bytes
+ *
+ * Returns the condition code:
+ *  0 - normal completion
+ *  1 - verification-pattern mismatch
+ *  2 - incomplete processing (see POP for details)
+ * Condition code 3 (partial completion) is handled within the asm code
+ * and never returned.
  */
-static inline void cpacf_kma(unsigned long func, void *param, u8 *dest,
-			     const u8 *src, unsigned long src_len,
-			     const u8 *aad, unsigned long aad_len)
+static inline int cpacf_kma(unsigned long func, void *param, u8 *dest,
+			    const u8 *src, unsigned long src_len,
+			    const u8 *aad, unsigned long aad_len)
 {
 	union register_pair d, s, a;
+	int cc;
 
 	d.even = (unsigned long)dest;
 	s.even = (unsigned long)src;
@@ -731,12 +739,16 @@ static inline void cpacf_kma(unsigned long func, void *param, u8 *dest,
 		"	lgr	0,%[fc]\n"
 		"	lgr	1,%[pba]\n"
 		"0:	.insn	rrf,%[opc] << 16,%[dst],%[src],%[aad],0\n"
-		"	brc	1,0b"	/* handle partial completion */
-		: [dst] "+&d" (d.pair), [src] "+&d" (s.pair),
+		"	brc	1,0b\n"	/* handle partial completion */
+		CC_IPM(cc)
+		: CC_OUT(cc, cc), [dst] "+&d" (d.pair), [src] "+&d" (s.pair),
 		  [aad] "+&d" (a.pair)
 		: [fc] "d" (func), [pba] "d" ((unsigned long)param),
 		  [opc] "i" (CPACF_KMA)
-		: "cc", "memory", "0", "1");
+		: CC_CLOBBER_LIST("memory", "0", "1"));
+
+	kmsan_unpoison_memory(dest, src_len - s.odd);
+	return CC_TRANSFORM(cc);
 }
 
 #endif	/* _ASM_S390_CPACF_H */
-- 
2.43.0

[PATCH v2 2/2] s390/crypto: Handle cpacf_kma() return code

[Harald Freudenberger]

Now that cpacf_kma() returns the condition code, check it and bail out
with -EIO on any non-zero result. In practice CC 1 and CC 2 cannot
occur: the GCM function codes are clear-key only, and the scatterwalk
loop ensures block-aligned chunks with LAAD/LPC set on the final call.
However, it is a defensive improvement, not a bug fix.

Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
---
 arch/s390/crypto/aes_s390.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/s390/crypto/aes_s390.c b/arch/s390/crypto/aes_s390.c
index 62edc66d5478..0e35cca35914 100644
--- a/arch/s390/crypto/aes_s390.c
+++ b/arch/s390/crypto/aes_s390.c
@@ -889,10 +889,13 @@ static int gcm_aes_crypt(struct aead_request *req, unsigned int flags)
 		if (aad_bytes > 0)
 			memcpy(gw_out.ptr, gw_in.ptr, aad_bytes);
 
-		cpacf_kma(ctx->fc | flags, &param,
-			  gw_out.ptr + aad_bytes,
-			  gw_in.ptr + aad_bytes, pc_bytes,
-			  gw_in.ptr, aad_bytes);
+		if (cpacf_kma(ctx->fc | flags, &param,
+			      gw_out.ptr + aad_bytes,
+			      gw_in.ptr + aad_bytes, pc_bytes,
+			      gw_in.ptr, aad_bytes)) {
+			ret = -EIO;
+			goto out;
+		}
 
 		n = aad_bytes + pc_bytes;
 		if (gcm_in_walk_done(&gw_in, n) != n)
-- 
2.43.0

Re: [PATCH v2 1/2] s390/cpacf: Rework cpacf_kma() to return condition code

[sashiko-bot]

> The KMA instruction can signal a verification-pattern mismatch (CC 1)
> when a protected key is used, or incomplete processing (CC 2) when a
> sub-block-size chunk is submitted without the appropriate LAAD/LPC
> flag. Both conditions shall be visible to the caller similar like the
> other CPACF instructions.
> 
> So rework the cpacf_kma() inline function to return the condition code
> instead of void. However, CC 3 (partial completion) continues to be
> handled internally. Also improve the function to use the CC macros for
> better readability. Additionally add an kmsan_unpoison_memory()
> statement to reflect the updated memory by hardware.
> 
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008105242.20730-1-freude@linux.ibm.com?part=1

Re: [PATCH v2 2/2] s390/crypto: Handle cpacf_kma() return code

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] crypto: s390: missing 'out' label causes build failure
- [Low] s390/crypto: build error due to missing 'out' label
--

commit 45b229c79189e8165da09c03b30411703a7bf4ae
Author: Harald Freudenberger <freude@linux.ibm.com>

s390/crypto: Handle cpacf_kma() return code

This commit updates gcm_aes_crypt() to check the return code of cpacf_kma()
and bail out with -EIO on any non-zero result.

> diff --git a/arch/s390/crypto/aes_s390.c b/arch/s390/crypto/aes_s390.c
> index 62edc66d54788..0e35cca359146 100644
> --- a/arch/s390/crypto/aes_s390.c
> +++ b/arch/s390/crypto/aes_s390.c
> @@ -889,10 +889,13 @@ static int gcm_aes_crypt(struct aead_request *req, unsigned int flags)
>  		if (aad_bytes > 0)
>  			memcpy(gw_out.ptr, gw_in.ptr, aad_bytes);
>  
> -		cpacf_kma(ctx->fc | flags, &param,
> -			  gw_out.ptr + aad_bytes,
> -			  gw_in.ptr + aad_bytes, pc_bytes,
> -			  gw_in.ptr, aad_bytes);
> +		if (cpacf_kma(ctx->fc | flags, &param,
> +			      gw_out.ptr + aad_bytes,
> +			      gw_in.ptr + aad_bytes, pc_bytes,
> +			      gw_in.ptr, aad_bytes)) {
> +			ret = -EIO;
> +			goto out;

[Severity: Critical]
Does this code fail to compile? There does not appear to be an out label
defined anywhere within gcm_aes_crypt().

> +		}
>  
>  		n = aad_bytes + pc_bytes;
>  		if (gcm_in_walk_done(&gw_in, n) != n)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008105242.20730-1-freude@linux.ibm.com?part=2