Re: [PATCH net-next] net: tls: Pass union tls_crypto_context pointer to memzero_explicit

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

On 7/5/24 15:41, Simon Horman wrote:
> Pass union tls_crypto_context pointer, rather than struct
> tls_crypto_info pointer, to memzero_explicit().
> 
> The address of the pointer is the same before and after.
> But the new construct means that the size of the dereferenced pointer type
> matches the size being zeroed. Which aids static analysis.
> 
> As reported by Smatch:
> 
>    .../tls_main.c:842 do_tls_setsockopt_conf() error: memzero_explicit() 'crypto_info' too small (4 vs 56)
> 
> No functional change intended.
> Compile tested only.
> 
> Signed-off-by: Simon Horman <horms@kernel.org>

Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
one small nitpick only

> ---
>   net/tls/tls_main.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> index 90b7f253d363..e712b2faeb81 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -616,6 +616,7 @@ static int do_tls_setsockopt_conf(struct sock *sk, sockptr_t optval,
>   	struct tls_crypto_info *alt_crypto_info;
>   	struct tls_context *ctx = tls_get_ctx(sk);
>   	const struct tls_cipher_desc *cipher_desc;
> +	union tls_crypto_context *crypto_ctx;
>   	int rc = 0;
>   	int conf;
>   
> @@ -623,13 +624,15 @@ static int do_tls_setsockopt_conf(struct sock *sk, sockptr_t optval,
>   		return -EINVAL;
>   
>   	if (tx) {
> -		crypto_info = &ctx->crypto_send.info;
> +		crypto_ctx = &ctx->crypto_send;
>   		alt_crypto_info = &ctx->crypto_recv.info;
>   	} else {
> -		crypto_info = &ctx->crypto_recv.info;
> +		crypto_ctx = &ctx->crypto_recv;
>   		alt_crypto_info = &ctx->crypto_send.info;
>   	}
>   
> +	crypto_info = &crypto_ctx->info;
> +
>   	/* Currently we don't support set crypto info more than one time */
>   	if (TLS_CRYPTO_INFO_READY(crypto_info))
>   		return -EBUSY;
> @@ -710,7 +713,7 @@ static int do_tls_setsockopt_conf(struct sock *sk, sockptr_t optval,
>   	return 0;
>   
>   err_crypto_info:
> -	memzero_explicit(crypto_info, sizeof(union tls_crypto_context));
> +	memzero_explicit(crypto_ctx, sizeof(union tls_crypto_context));

nit: That's a good fix to aid static analyzers, and reviewers.
Now it's also easy to follow the standard style and pass
sizeof(*crypto_ctx) instead of the type.

>   	return rc;
>   }