Re: [PATCH v3 1/2] ima: discard modsig when detached data cannot be supplied

[Mimi Zohar]

On Wed, 2026-10-21 at 21:01 +0000, Jérémy Jean wrote:
> ima_collect_modsig() supplies the file contents to the parsed PKCS#7
> message as detached data. If the message already contains embedded data,
> pkcs7_supply_detached_data() returns -EINVAL, but IMA ignores the error.
> ima_modsig_verify() then verifies the embedded data instead of the file
> being appraised.
> 
> Make ima_collect_modsig() report whether the file data was supplied,
> and discard the modsig if it was not. 
> 
> Fixes: 15588227e086 ("ima: Collect modsig")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Jérémy Jean <Jeremy.Jean@xxxxxxxxxxxxxxxxx>
> ---

[...]
>  
> diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
> index 632c746fd81e..f73d6f1b71c0 100644
> --- a/security/integrity/ima/ima_modsig.c
> +++ b/security/integrity/ima/ima_modsig.c
> @@ -96,8 +96,12 @@ int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
>   * Since the modsig is part of the file contents, the hash used in its signature
>   * isn't the same one ordinarily calculated by IMA. Therefore PKCS7 code
>   * calculates a separate one for signature verification.
> + *
> + * Digest export is best-effort; not all signatures expose a single digest.
> + *
> + * Return: true if the file data was supplied, false otherwise.
>   */
> -void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size)
> +bool ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size)
>  {
>  	int rc;
>  
> @@ -109,11 +113,13 @@ void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size)
>  		sizeof(struct module_signature);
>  	rc = pkcs7_supply_detached_data(modsig->pkcs7_msg, buf, size);
>  	if (rc)
> -		return;
> +		return false;
> +
> +	/* Digest export failure does not preclude signature verification. */
> +	pkcs7_get_digest(modsig->pkcs7_msg, &modsig->digest,
> +			 &modsig->digest_size, &modsig->hash_algo);

This change isn't mentioned in the patch description.  Is it necessary to
address the bug and be backported?

Mimi

>  
> -	/* Ask the PKCS7 code to calculate the file hash. */
> -	rc = pkcs7_get_digest(modsig->pkcs7_msg, &modsig->digest,
> -			      &modsig->digest_size, &modsig->hash_algo);
> +	return true;
>  }
>  
>  int ima_modsig_verify(struct key *keyring, const struct modsig *modsig)
> diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> index 8a89236f926c..1be78612394a 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -478,8 +478,8 @@ int ima_eventdigest_modsig_init(struct ima_event_data *event_data,
>  		if (rc)
>  			return rc;
>  		else if (hash_algo == HASH_ALGO__LAST || cur_digestsize == 0)
> -			/* There was some error collecting the digest. */
> -			return -EINVAL;
> +			/* Keep the ordinary measurement without an exported digest. */
> +			return 0;
>  	}
>  
>  	return ima_eventdigest_init_common(cur_digest, cur_digestsize,