Re: [PATCH v8 1/5] s390/zcrypt: Fix CPRB memory allocation in zcrypt misc code

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete memory scrubbing in `free_cprbmem()` due to missing 4-byte alignment rounding.
--

--- Patch [1]: [PATCH v8 1/5] s390/zcrypt: Fix CPRB memory allocation in zcrypt misc code ---
commit 8719946bc97657e1ca3c996d1d00474456aa0ef0
Author: Harald Freudenberger <freude@linux.ibm.com>

s390/zcrypt: Fix CPRB memory allocation in zcrypt misc code
    
Both CPRB alloc functions in zcrypt_ccamisc.c and zcrypt_ep11misc.c
did not round up the memory allocation to a multiple of 4 bytes as it
is needed by the zcrypt layer to process the CPRBs.

Now the alloc_and_prep_cprbmem() and alloc_cprbmem() functions
guarantee that the base CPRB struct and a possible parameter block are
aligned to a 4-byte boundary and the backing memory allocation is
rounded up to the next multiple of 4 byte. Also the free_cprbmem() is
updated and scrubs the rounded up amount of memory.

> diff --git a/drivers/s390/crypto/zcrypt_ep11misc.c b/drivers/s390/crypto/zcrypt_ep11misc.c
> index 3dda9589f2b95..e604c100ccce2 100644
> --- a/drivers/s390/crypto/zcrypt_ep11misc.c
> +++ b/drivers/s390/crypto/zcrypt_ep11misc.c
> @@ -355,21 +356,24 @@ EXPORT_SYMBOL(ep11_check_aes_key);
>  
>  /*
>   * Allocate and prepare ep11 cprb plus additional payload.
> + * It is guaranteed that the memory is aligned to a 4 byte boundary.
> + * Furthermore the memory allocation is rounded up to the next
> + * multiple of 4 bytes (with taking the payload_len into account).
>   */
>  static void *alloc_cprbmem(size_t payload_len, u32 xflags)
>  {
> -	size_t len = sizeof(struct ep11_cprb) + payload_len;
> +	size_t memlen = ALIGN(sizeof(struct ep11_cprb) + payload_len, 4);

[Severity: Medium]
The commit message states that free_cprbmem() is updated to scrub the rounded
up amount of memory. While this change is present for zcrypt_ccamisc.c, was it
missed for zcrypt_ep11misc.c?

Since alloc_cprbmem() now allocates the rounded up memory size, does
free_cprbmem() in zcrypt_ep11misc.c also need to be updated to scrub the
matching size? 

Currently, it still uses the exact unrounded size:

    static void free_cprbmem(void *mem, size_t payload_len, bool scrub, u32 xflags)
    {
        if (mem && scrub)
            memzero_explicit(mem, sizeof(struct ep11_cprb) + payload_len);

Could this leave up to 3 bytes of the newly added padding unscrubbed when an
EP11 CPRB is freed? If the cryptographic hardware writes sensitive data into
the padding bytes of a 4-byte aligned response, this data would not be zeroed
before the memory is returned to the allocator.

[ ... ]

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