Re: [PATCH v2 3/4] ima: limit secure boot feedback scope for appraise

Mimi Zohar
2020-09-04T21:07:20Z

Hi Bruno,

> + bool sb_state = arch_ima_get_secureboot();
> + int appraisal_state = ima_appraise;
>
> if (strncmp(str, "off", 3) == 0)
> - ima_appraise = 0;
> + appraisal_state = 0;
> else if (strncmp(str, "log", 3) == 0)
> - ima_appraise = IMA_APPRAISE_LOG;
> + appraisal_state = IMA_APPRAISE_LOG;
> else if (strncmp(str, "fix", 3) == 0)
> - ima_appraise = IMA_APPRAISE_FIX;
> + appraisal_state = IMA_APPRAISE_FIX;
> else if (strncmp(str, "enforce", 7) == 0)
> - ima_appraise = IMA_APPRAISE_ENFORCE;
> + appraisal_state = IMA_APPRAISE_ENFORCE;
> else
> pr_err("invalid \"%s\" appraise option", str);
> +
> + /* If appraisal state was changed, but secure boot is enabled,
> + * keep its default */
> + if (sb_state) {
> + if (!(appraisal_state & IMA_APPRAISE_ENFORCE))
> + pr_info("Secure boot enabled: ignoring ima_appraise=%s option",
> + str);
> + else
> + ima_appraise = appraisal_state;
> + }

Shouldn't the "else" clause be here. No need to re-post the entire
patch set.

thanks,

Mimi

> #endif
> return 1;
> }