Re: [PATCH v2 0/2] ima: Fallback to the builtin hash algorithm

[Mimi Zohar <zohar@linux.vnet.ibm.com>]

On Thu, 2018-03-22 at 15:43 +0100, Petr Vorel wrote:
> Hello Mimi,
> 
> Changes v1->v2:
> * Move loading buildin hash algorithm to ima_init_crypto() (as requested)
> * Add crypto_alloc_shash() (DRY)
> 
> [1]:
> > The first call to ima_init() emits an error, but we continue without
> > any further messages.  If the change was in ima_init_crypto(), the
> > error message could indicate the resolution.
> I cannot say I like v2 as change in ima_init_crypto() requires more
> changes due updating ima_hash_algo. IMHO v1 with added simple error
> message would be better.  I probably didn't understand what exactly you
> wanted to log in ima_init_crypto(). Am I missing something?

Agreed, the previous version was simpler/better.  My complaint with
the previous version was that there was an error message, but
continued without any indication of how it was resolved.  Changing the
message in ima_init_crypto() is one solution, but adding an additional
message works too. Would this work for you?

@@ -73,6 +73,8 @@ int __init ima_init_crypto(void)
                       hash_algo_name[ima_hash_algo], rc);
                return rc;
        }
+       pr_info("Allocated default hash algorithm: %s\n",
+                hash_algo_name[ima_hash_algo]);
        return 0;
 }

Mimi

> If you like this version, you may want to squash it into single commit.
> 
> Kind regards,
> Petr
> 
> [1]: https://marc.info/?l=linux-integrity&m=152155002608419&w=2
> 
> Petr Vorel (2):
>   ima: Introduce ima_alloc_alg() to reduce duplicity
>   ima: Fallback to the builtin hash algorithm
> 
>  security/integrity/ima/ima.h        |  1 +
>  security/integrity/ima/ima_crypto.c | 58 +++++++++++++++++++++++++++----------
>  security/integrity/ima/ima_main.c   | 26 ++---------------
>  3 files changed, 47 insertions(+), 38 deletions(-)
>