Re: [PATCH v2 1/2] integrity: Refactor asymmetric_verify for reusability

On Wed, 2026-04-08 at 13:41 -0400, Stefan Berger wrote:
> Refactor asymmetric_verify for reusability. Have it call
> asymmetric_verify_common with the signature verification key and the
> public_key structure as parameters. sigv3 support for ML-DSA will need to
> check the public key type first to decide how to do the signature
> verification and therefore will have these parameters available for
> calling asymmetric_verify_common.
> 
> Signed-off-by: Stefan Berger <[email protected]>

Thanks, Stefan.

> ---
>  security/integrity/digsig_asymmetric.c | 42 +++++++++++++++++---------
>  1 file changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c
> index 6e68ec3becbd..e29ed73f15cd 100644
> --- a/security/integrity/digsig_asymmetric.c
> +++ b/security/integrity/digsig_asymmetric.c
> @@ -79,18 +79,15 @@ static struct key *request_asymmetric_key(struct key *keyring, uint32_t keyid)
>  	return key;
>  }
>  
> -int asymmetric_verify(struct key *keyring, const char *sig,
> -		      int siglen, const char *data, int datalen)
> +static int asymmetric_verify_common(const struct key *key,
> +				    const struct public_key *pk,
> +				    const char *sig, int siglen,
> +				    const char *data, int datalen)
>  {
> -	struct public_key_signature pks;
>  	struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig;
> -	const struct public_key *pk;
> -	struct key *key;
> +	struct public_key_signature pks;
>  	int ret;
>  
> -	if (siglen <= sizeof(*hdr))
> -		return -EBADMSG;
> -
>  	siglen -= sizeof(*hdr);

Normally kernel-doc is unnecessary for static functions.  Here, however, since 
only the caller verifies the signature length, there should be a kernel-doc
function definition.  It should indicate that all callers must verify the
signature length (siglen) and that the public key (pk) is not NULL, before
calling asymmetric_verify_common().

>  
>  	if (siglen != be16_to_cpu(hdr->sig_size))
> @@ -99,15 +96,10 @@ int asymmetric_verify(struct key *keyring, const char *sig,
>  	if (hdr->hash_algo >= HASH_ALGO__LAST)
>  		return -ENOPKG;
>  
> -	key = request_asymmetric_key(keyring, be32_to_cpu(hdr->keyid));
> -	if (IS_ERR(key))
> -		return PTR_ERR(key);
> -
>  	memset(&pks, 0, sizeof(pks));
>  
>  	pks.hash_algo = hash_algo_name[hdr->hash_algo];
>  
> -	pk = asymmetric_key_public_key(key);
>  	pks.pkey_algo = pk->pkey_algo;
>  	if (!strcmp(pk->pkey_algo, "rsa")) {
>  		pks.encoding = "pkcs1";
> @@ -127,11 +119,33 @@ int asymmetric_verify(struct key *keyring, const char *sig,
>  	pks.s_size = siglen;
>  	ret = verify_signature(key, &pks);
>  out:
> -	key_put(key);

The kernel-doc function definition should also indicate that the caller must
free the key.

>  	pr_debug("%s() = %d\n", __func__, ret);
>  	return ret;
>  }
>  
> +int asymmetric_verify(struct key *keyring, const char *sig,
> +		      int siglen, const char *data, int datalen)
> +{
> +	struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig;
> +	const struct public_key *pk;
> +	struct key *key;
> +	int ret;
> +
> +	if (siglen <= sizeof(*hdr))
> +		return -EBADMSG;
> +
> +	key = request_asymmetric_key(keyring, be32_to_cpu(hdr->keyid));
> +	if (IS_ERR(key))
> +		return PTR_ERR(key);
> +	pk = asymmetric_key_public_key(key);

Please add a test here making sure pk is not null.

thanks,

Mimi

> +
> +	ret = asymmetric_verify_common(key, pk, sig, siglen, data, datalen);
> +
> +	key_put(key);
> +
> +	return ret;
> +}
> +
>  /*
>   * calc_file_id_hash - calculate the hash of the ima_file_id struct data
>   * @type: xattr type [enum evm_ima_xattr_type]