Re: [PATCH net-next 2/5] net/rxrpc: Use local FCrypt-PCBC implementation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Stack-allocated `struct fcrypt_key` is not zeroized before returning in `rxkad_decrypt_response()`.
--

commit cab871c1c0eecff467a076e259b88fd1f8a50b0a
Author: Eric Biggers <ebiggers@kernel.org>

net/rxrpc: Use local FCrypt-PCBC implementation

This patch replaces the crypto API implementation of FCrypt-PCBC with a
local implementation. This improves performance, allows removing the crypto
API dependency, and simplifies the decryption code by linearizing the skb.

> diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
> index cba7935977f0..3c9e7f636b42 100644
> --- a/net/rxrpc/rxkad.c
> +++ b/net/rxrpc/rxkad.c

[ ... ]

> @@ -1095,39 +916,22 @@ static int rxkad_decrypt_ticket(struct rxrpc_connection *conn,
>  /*
>   * decrypt the response packet
>   */
> -static int rxkad_decrypt_response(struct rxrpc_connection *conn,
> -				  struct rxkad_response *resp,
> -				  const struct rxrpc_crypt *session_key)
> +static void rxkad_decrypt_response(struct rxrpc_connection *conn,
> +				   struct rxkad_response *resp,
> +				   const struct rxrpc_crypt *session_key)
>  {
> -	struct skcipher_request *req = rxkad_ci_req;
> -	struct scatterlist sg[1];
> -	struct rxrpc_crypt iv;
> -	int ret;
> +	struct fcrypt_key cipher;
>  
>  	_enter(",,%08x%08x",
>  	       ntohl(session_key->n[0]), ntohl(session_key->n[1]));
>  
> -	mutex_lock(&rxkad_ci_mutex);
> -	ret = crypto_sync_skcipher_setkey(rxkad_ci, session_key->x,
> -					  sizeof(*session_key));
> -	if (ret < 0)
> -		goto unlock;
> -
> -	memcpy(&iv, session_key, sizeof(iv));
> -
> -	sg_init_table(sg, 1);
> -	sg_set_buf(sg, &resp->encrypted, sizeof(resp->encrypted));
> -	skcipher_request_set_sync_tfm(req, rxkad_ci);
> -	skcipher_request_set_callback(req, 0, NULL, NULL);
> -	skcipher_request_set_crypt(req, sg, sg, sizeof(resp->encrypted), iv.x);
> -	ret = crypto_skcipher_decrypt(req);
> -	skcipher_request_zero(req);
> -
> -unlock:
> -	mutex_unlock(&rxkad_ci_mutex);
> +	fcrypt_preparekey(&cipher, session_key->x);
>  
> +	static_assert(sizeof(resp->encrypted) % FCRYPT_BSIZE == 0);
> +	fcrypt_pcbc_decrypt(&cipher, session_key->x, &resp->encrypted,
> +			    &resp->encrypted,
> +			    sizeof(resp->encrypted) / FCRYPT_BSIZE);
>  	_leave("");
> -	return ret;
>  }

Does this code leave the cryptographic key material on the stack?

The local struct fcrypt_key cipher is allocated on the stack and initialized
with the session key schedule via fcrypt_preparekey(). After the decryption is
complete, the function returns without securely erasing this key schedule from
the stack.

Could we call memzero_explicit(&cipher, sizeof(cipher)) before returning to
ensure the key material is cleared?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260428024400.123337-1-ebiggers@kernel.org?part=2