On Mon, 2020-08-17 at 18:52 -0300, Bruno Meneguele wrote:
> Instead of print to kmsg any ima_appraise= option passed by the user in case
> of secure boot being enabled, first check if the state was really changed
> from its original "enforce" state, otherwise don't print anything.
Please reword this patch description, removing "Instead of print to
kmsg".
>
> Signed-off-by: Bruno Meneguele <bmeneg@xxxxxxxxxx>
> ---
> security/integrity/ima/ima_appraise.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 2193b51c2743..000df14f198a 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -19,11 +19,7 @@
> static int __init default_appraise_setup(char *str)
> {
> #ifdef CONFIG_IMA_APPRAISE_BOOTPARAM
> - if (arch_ima_get_secureboot()) {
> - pr_info("Secure boot enabled: ignoring ima_appraise=%s boot parameter option",
> - str);
> - return 1;
> - }
> + bool sb_state = arch_ima_get_secureboot();
>
> if (strncmp(str, "off", 3) == 0)
> ima_appraise = 0;
> @@ -35,6 +31,16 @@ static int __init default_appraise_setup(char *str)
> ima_appraise = IMA_APPRAISE_ENFORCE;
> else
> pr_err("invalid \"%s\" appraise option", str);
> +
> + /* If appraisal state was changed, but secure boot is enabled,
> + * reset it to enforced */
> + if (sb_state) {
> + if (!is_ima_appraise_enabled()) {
> + pr_info("Secure boot enabled: ignoring ima_appraise=%s option",
> + str);
> + ima_appraise = IMA_APPRAISE_ENFORCE;
> + }
> + }
Instead of changing ima_appraise and then resetting it back to
enforcing, how about using a temporary variable instead? Maybe
something like:
if (!is_ima_appraise_enabled())
pr_info( ...)
else
ima_appraise = temporary value
> #endif
> return 1;
> }
