From: Dmitry Kasatkin Subject: Re: [PATCH v3 1/3] ima: use ahash API for file hash calculation Date: Tue, 08 Jul 2014 11:07:16 +0300 Message-ID: <53BBA6B4.7030705@samsung.com> References: <1404734207.3029.22.camel@dhcp-9-2-203-236.watson.ibm.com> <53BAA281.7040903@samsung.com> <1404747894.3029.58.camel@dhcp-9-2-203-236.watson.ibm.com> <53BAC6A6.2010509@samsung.com> <1404750875.3029.79.camel@dhcp-9-2-203-236.watson.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: linux-ima-devel@lists.sourceforge.net, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org, dmitry.kasatkin@gmail.com To: Mimi Zohar Return-path: Received: from mailout3.w1.samsung.com ([210.118.77.13]:61364 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750927AbaGHIIc (ORCPT ); Tue, 8 Jul 2014 04:08:32 -0400 In-reply-to: <1404750875.3029.79.camel@dhcp-9-2-203-236.watson.ibm.com> Sender: linux-crypto-owner@vger.kernel.org List-ID: On 07/07/14 19:34, Mimi Zohar wrote: > On Mon, 2014-07-07 at 19:11 +0300, Dmitry Kasatkin wrote: >> On 07/07/14 18:44, Mimi Zohar wrote: >>> On Mon, 2014-07-07 at 16:37 +0300, Dmitry Kasatkin wrote: >>>> On 07/07/14 14:56, Mimi Zohar wrote: >>>>> On Fri, 2014-07-04 at 15:05 +0300, Dmitry Kasatkin wrote: >>>>>> +/** >>>>> This is the kernel-doc delimiter. >>>>> >>>>>> + * ima_calc_file_hash - calculae file hash >>>>>> + * >>>>> Missing kernel-doc argument descriptions. Refer to >>>>> Documentation/kernel-doc-nano-HOWTO.txt. >>> Not defining the arguments results in a kernel-doc warning. Providing >>> kernel-doc is nice, but is unnecessary in this case, as it isn't an >>> exported loadable module, nor an externally visible function to other >>> kernel files. Either remove the extra asterisk, making it a regular >>> comment, or add the arguments. >>> >>>> There is no need to explain arguments as they self-evident. >>>> >>>>>> + * if ima_ahash_minsize parameter is non-zero, this function uses >>>>>> + * ahash for hash caclulation. ahash performance varies for different >>>>>> + * data sizes on different crypto accelerators. shash performance might >>>>>> + * be better for small file. 'ima.ahash_minsize' module parameter allows >>>>>> + * to specify the best value for the system. >>>>>> + * >>>>>> + * If ahash fails, it fallbacks to shash. >>>>>> + */ >>>>>> +int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash) >>>>>> +{ >>>>>> + loff_t i_size; >>>>>> + int rc; >>>>>> + >>>>>> + i_size = i_size_read(file_inode(file)); >>>>>> + >>>>>> + if (ima_ahash_minsize && i_size >= ima_ahash_minsize) { >>>>>> + rc = ima_calc_file_ahash(file, hash); >>>>>> + if (!rc) >>>>>> + return 0; >>>>>> + } >>>>>> + >>>>>> + return ima_calc_file_shash(file, hash); >>>>>> +} >>>>> If the crypto accelerator fails, it falls back to using shash. Is their >>>>> any indication that the HW error is intermittent or persistent? Should >>>>> ima_ahash_minsize be reset? >>>> If hw constantly does not work then it is simply broken. >>> True >>> >>>> You want to be protected from "random" failures. >>>> For me it is not the case either... If it works then it works... >>> This discussion isn't about your particular HW environment, but a >>> general question. For example, suppose we were discussing a laptop with >>> a HW crypto accelerator. If the HW crypto broke, I would at least want >>> to be able to quiesce the system properly. I'd most likely want to be >>> able to continue using my laptop with software crypto. >> Driver probing code will detect that HW is not responding and driver >> will not be enabled... >> >> IMA will not be able to use it... >> >> It is the same story as with any other HW and driver in the system. > Right, but my concern is not about unloading the kernel module, but > about the IMA module parameters left initialized. The existing code > will continue using ahash (software version), even though the kernel > module was unloaded, not shash. My question is about the software > implementations of ahash vs. shash performance. > > Mimi If HW driver will not be available, ahash loads generic driver which is using shash. Performance of that will be the same as for using shash directly. - Dmitry >