Re: [PATCH v6 2/9] s390/crypto: Fix missing scrub of temp buffers with PAES algorithm

On 2026-08-21 10:58, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Missing explicit scrub of local stack buffer `buf` in
> `ctr_aes_crypt()` (standard AES algorithm).
> - [High] Missing explicit scrub of temp buffers in `gcm_sg_walk`
> structures and AES keys in `gcm_aes_crypt()`.
> --
> 
> --- Patch [2]: [PATCH v6 2/9] s390/crypto: Fix missing scrub of temp
> buffers with PAES algorithm ---
> commit 0f77835bffb943e6b87472b41b755be562bb23c4
> Author: Harald Freudenberger <[email protected]>
> 
> s390/crypto: Fix missing scrub of temp buffers with PAES algorithm
> 
> This commit explicitly zeroizes temporary stack buffers used for 
> partial
> block processing in the paes algorithm. It also updates the scrubbing
> logic in the xts parameter preparation function to clear the entire
> parameter block rather than just the key field.
> 
>> diff --git a/arch/s390/crypto/paes_s390.c 
>> b/arch/s390/crypto/paes_s390.c
>> index ccc7da106c3a0..d075b0241f1f5 100644
>> --- a/arch/s390/crypto/paes_s390.c
>> +++ b/arch/s390/crypto/paes_s390.c
>> @@ -1044,6 +1044,7 @@ static int ctr_paes_do_crypt(struct 
>> s390_paes_ctx *ctx,
>>  	}
>> 
>>  out:
>> +	memzero_explicit(buf, sizeof(buf));
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but while you are fixing the 
> missing
> scrub of buf here, there appears to be an identical pre-existing issue 
> in the
> standard aes implementation.
> 
> In arch/s390/crypto/aes_s390.c:ctr_aes_crypt(), the local stack buffer 
> buf
> is populated with cryptographic data when processing a final partial 
> block,
> but it returns without scrubbing the buffer:
> 
> 		ret = skcipher_walk_done(&walk, 0);
> 	}
> 
> 	return ret;
> }
> 
> Since this path is reachable by unprivileged users invoking the 
> ctr(aes)
> algorithm with unaligned lengths, does ctr_aes_crypt() need a similar
> memzero_explicit() call before returning to avoid leaving sensitive 
> data on
> the kernel stack?
> 
> [Severity: High]
> This is another pre-existing issue in the same subsystem, but looking 
> at
> gcm_aes_crypt() in arch/s390/crypto/aes_s390.c, there are error paths 
> that
> bypass the existing memzero_explicit() cleanup completely:
> 
> 			return -ENOMEM;
> 		if (gcm_out_walk_done(&gw_out, n) != n)
> 			return -ENOMEM;
> 
> If these error paths are taken when processing fragmented input data, 
> does this
> leave the aes key and hash subkey on the kernel stack?
> 
> Additionally, the normal exit path cleans the param block but doesn't 
> appear
> to clean the buf fields in the gcm_sg_walk structures (gw_in and 
> gw_out):
> 
> 	memzero_explicit(&param, sizeof(param));
> 	return ret;
> }
> 
> Should these temporary buffers and early return paths also be scrubbed?
> 
>>  	pr_debug("rc=%d\n", rc);
>>  	return rc;
>>  }
>> @@ -1377,7 +1378,7 @@ static inline int __xts_2keys_prep_param(struct 
>> s390_pxts_ctx *ctx,
>>  		memcpy(param->init, pcc_param.xts, 16);
>>  	}
>> 
>> -	memzero_explicit(pcc_param.key, sizeof(pcc_param.key));
>> +	memzero_explicit(&pcc_param, sizeof(pcc_param));
>>  	return rc;
>>  }

already addressed and fixed in the aes_s390 patch series.