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

[Mimi Zohar]

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;
>  }