On Mon, 2026-06-29 at 12:11 +0000, Enrico Bravi wrote: > Hi Roberto, > > On Thu, 2026-06-25 at 15:10 +0200, Roberto Sassu wrote: > > 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). > > thank you very much for your feedback. I'll rework the patch description to > make > it more clear and describe what you highlighted. > > > > 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. > > Sure, will add it in the next version. > > > > + 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. > > Yes you're right. It should not be necessary to treat unmapped algorithms > differently. I was thinking that it could be directly used > TPM2_MAX_DIGEST_SIZE > instead. What do you think?
Or sizeof(), looks more robust. Thanks Roberto > Enrico > > > Thanks > > > > Roberto > > > > > } > > > + atomic_long_inc(&ima_htable.violations); > > > } > > > > > > entry->pcr = !ima_canonical_fmt ? *(u32 > > > *)(hdr[HDR_PCR].data) : > > > > > > base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
