Re: [PATCH] ima: correctly recover number of violations after kexec

On Fri, 2026-06-19 at 21:14 +0200, Enrico Bravi wrote:
> When recovering the measurement list after kexec(), the number of
> violations is not recovered as well, causing a mismatch between the
> number reported by the <securityfs>/ima/violations user interface and
> the actual value. In addition, currently it is assumed that when
> recovering an entry, this is a violation if the template data hash
> read from the kexec buffer is an all-zero hash, which can actually be a
> valid hash.

This sentence is a bit convoluted. Please rework it.

> Verify that an all-zero hash corresponds to a violation and consequently
> correctly recover the number of violations.

We need to clarify that this patch only fixes false positives (a record
is declared as a violation even if it isn't, and we fix it by cross-
checking the SHA1 of the template data).

However, we are not fixing the false negatives (if the SHA1 of the
template data is zeros, we don't declare it as a violation, but it can
potentially be).

> Reported-by: Roberto Sassu <[email protected]>
> Closes: https://github.com/linux-integrity/linux/issues/13
> Signed-off-by: Enrico Bravi <[email protected]>
> 
> ---
>  security/integrity/ima/ima_template.c | 28 ++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index 7034573fb41e..147f228ed246 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -430,6 +430,7 @@ int ima_restore_measurement_list(loff_t size, void *buf)
>  	DECLARE_BITMAP(hdr_mask, HDR__LAST);
>  	unsigned long count = 0;
>  	int ret = 0;
> +	int i;
>  
>  	if (!buf || size < sizeof(*khdr))
>  		return 0;
> @@ -515,15 +516,28 @@ int ima_restore_measurement_list(loff_t size, void *buf)
>  		if (ret < 0)
>  			break;
>  
> -		if (memcmp(hdr[HDR_DIGEST].data, zero, sizeof(zero))) {
> -			ret = ima_calc_field_array_hash(
> -						&entry->template_data[0],
> +		ret = ima_calc_field_array_hash(&entry->template_data[0],
>  						entry);
> -			if (ret < 0) {
> -				pr_err("cannot calculate template digest\n");
> -				ret = -EINVAL;
> -				break;
> +		if (ret < 0) {
> +			pr_err("cannot calculate template digest\n");
> +			ret = -EINVAL;
> +			break;
> +		}
> +
> +		if (!memcmp(hdr[HDR_DIGEST].data, zero, sizeof(zero)) &&
> +		    memcmp(entry->digests[ima_sha1_idx].digest, zero, sizeof(zero))) {

Here it would be helpful to have a comment saying that we are reverting
the effect of ima_calc_field_array_hash(): we tried to verify if the
entry is a violation, it is, but then we have to set the template
digest back to zero after the calculation.

> +			for (i = 0; i < NR_BANKS(ima_tpm_chip) + ima_extra_slots; i++) {
> +				/* Unmapped TPM algorithms */
> +				if (!ima_algo_array[i].tfm) {
> +					memset(entry->digests[i].digest, 0,
> +					       TPM_DIGEST_SIZE);
> +					continue;
> +				}
> +
> +				memset(entry->digests[i].digest, 0,
> +				       ima_algo_array[i].digest_size);

Maybe we can just use the instruction above instead of differentiating
for unmapped algorithms? digest_size is taken from the TPM in that
case.

Thanks

Roberto

>  			}
> +			atomic_long_inc(&ima_htable.violations);
>  		}
>  
>  		entry->pcr = !ima_canonical_fmt ? *(u32 *)(hdr[HDR_PCR].data) :
> 
> base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6