[PATCH v2 0/3] ima: use asynchronous hash API for hash calculation

[PATCH v2 0/3] ima: use asynchronous hash API for hash calculation

[Dmitry Kasatkin]

Depending on the IMA policy, it might require to measure huge amount of files.
It may be very important to speedup hash calculation or to reduce (bettery)
energy required to do it. Currently IMA uses synchronous hash API (shash)
which is CPU based. CPU based hash calculation is very CPU intensive and on the
battery powered device will be also high energy consuming.

Many platforms provide cryptographic acceleration modules which allow speedup
and/or reduce energy consumption, and provide asynchronous way to calculate
hashes. Defacto way to implement drivers for such accelerators is using
asynchronous hash API (ahash).

The first patch adds use of ahash API to IMA. Performance of using HW
acceleration depends very much on amount of data to hash and it depends
on particular HW. It is usually inefficient for small data due to HW
initialization overhead. In order to make it possible to optimize performance
for particular system, the patch provides kernel parameter
'ima_ahash=<min_file_size>', which allows to specify optimal file size
when start using ahash. By default ahash is disabled until non-zero value
with 'ima_ahash=' is provided.

Second patch introduces multi-page buffers which makes HW acceleration more
efficient. It extends 'ima_ahash' kernel parameter to specify buffer size:
'ima_ahash=<min_file_size>[,<bufsize>]'

Third patch introduces double-buffering which allows to readahead next portion
of data for hashing while calculating the hash.

Changes to v1:
- ima_ahash_size and ima_ahash_bufsize were combined as ima_ahash
- ahash pre-allocation moved out from __init code to be able to use
  ahash crypto modules. Ahash allocated once on the first use.
- hash calculation falls back to sahsh if ahash allocation/calculation fails
- complex initialization separated from variable declaration
- improved comments

- Dmitry


Dmitry Kasatkin (3):
  ima: use ahash API for file hash calculation
  ima: introduce multi-page collect buffers
  ima: provide double buffering for hash calculation

 Documentation/kernel-parameters.txt |   6 +
 security/integrity/ima/ima_crypto.c | 287 +++++++++++++++++++++++++++++++++++-
 2 files changed, 290 insertions(+), 3 deletions(-)

-- 
1.9.1

[PATCH v2 1/3] ima: use ahash API for file hash calculation

[Dmitry Kasatkin]

Async hash API allows to use HW acceleration for hash calculation.
It may give significant performance gain or/and reduce power consumption,
which might be very beneficial for battery powered devices.

This patch introduces hash calculation using ahash API.

ahash performance depends on data size and particular HW. Under certain
limit, depending on the system, shash performance may be better.

This patch defines 'ima_ahash' kernel parameter which can be used to
define minimal file size to use with ahash. When file size is not set
or smaller than defined by the parameter, shash will be used.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 Documentation/kernel-parameters.txt |   5 +
 security/integrity/ima/ima_crypto.c | 185 +++++++++++++++++++++++++++++++++++-
 2 files changed, 186 insertions(+), 4 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 5a214a3..b406f5c 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1291,6 +1291,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	ihash_entries=	[KNL]
 			Set number of hash buckets for inode cache.
 
+	ima_ahash=	[IMA] Asynchronous hash usage parameters
+			Format: <min_file_size>
+			Set the minimal file size when use asynchronous hash.
+			If ima_ahash is not provided, ahash usage is disabled.
+
 	ima_appraise=	[IMA] appraise integrity measurements
 			Format: { "off" | "enforce" | "fix" }
 			default: "enforce"
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index f82d1d7..5eb19b4 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -25,7 +25,27 @@
 #include <crypto/hash_info.h>
 #include "ima.h"
 
+
+struct ahash_completion {
+	struct completion completion;
+	int err;
+};
+
 static struct crypto_shash *ima_shash_tfm;
+static struct crypto_ahash *ima_ahash_tfm;
+
+/* minimal file size for ahash use */
+static loff_t ima_ahash_size;
+
+static int __init ima_ahash_setup(char *str)
+{
+	int rc;
+
+	rc = kstrtoll(str, 10, &ima_ahash_size);
+
+	return !rc;
+}
+__setup("ima_ahash=", ima_ahash_setup);
 
 /**
  * ima_kernel_read - read file content
@@ -93,9 +113,146 @@ static void ima_free_tfm(struct crypto_shash *tfm)
 		crypto_free_shash(tfm);
 }
 
-/*
- * Calculate the MD5/SHA1 file digest
- */
+static struct crypto_ahash *ima_alloc_atfm(enum hash_algo algo)
+{
+	struct crypto_ahash *tfm = ima_ahash_tfm;
+	int rc;
+
+	if ((algo != ima_hash_algo && algo < HASH_ALGO__LAST) || !tfm) {
+		tfm = crypto_alloc_ahash(hash_algo_name[algo], 0, 0);
+		if (!IS_ERR(tfm)) {
+			if (algo == ima_hash_algo)
+				ima_ahash_tfm = tfm;
+		} else {
+			rc = PTR_ERR(tfm);
+			pr_err("Can not allocate %s (reason: %d)\n",
+			       hash_algo_name[algo], rc);
+		}
+	}
+	return tfm;
+}
+
+static void ima_free_atfm(struct crypto_ahash *tfm)
+{
+	if (tfm != ima_ahash_tfm)
+		crypto_free_ahash(tfm);
+}
+
+static void ahash_complete(struct crypto_async_request *req, int err)
+{
+	struct ahash_completion *res = req->data;
+
+	if (err == -EINPROGRESS)
+		return;
+	res->err = err;
+	complete(&res->completion);
+}
+
+static int ahash_wait(int err, struct ahash_completion *res)
+{
+	switch (err) {
+	case 0:
+		break;
+	case -EINPROGRESS:
+	case -EBUSY:
+		wait_for_completion(&res->completion);
+		reinit_completion(&res->completion);
+		err = res->err;
+		/* fall through */
+	default:
+		pr_crit("ahash calculation failed: err: %d\n", err);
+	}
+
+	return err;
+}
+
+static int ima_calc_file_hash_atfm(struct file *file,
+				   struct ima_digest_data *hash,
+				   struct crypto_ahash *tfm)
+{
+	loff_t i_size, offset;
+	char *rbuf;
+	int rc, read = 0, rbuf_len;
+	struct ahash_request *req;
+	struct scatterlist sg[1];
+	struct ahash_completion res;
+
+	hash->length = crypto_ahash_digestsize(tfm);
+
+	req = ahash_request_alloc(tfm, GFP_KERNEL);
+	if (!req)
+		return -ENOMEM;
+
+	init_completion(&res.completion);
+	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
+				   CRYPTO_TFM_REQ_MAY_SLEEP,
+				   ahash_complete, &res);
+
+	rc = ahash_wait(crypto_ahash_init(req), &res);
+	if (rc)
+		goto out1;
+
+	i_size = i_size_read(file_inode(file));
+
+	if (i_size == 0)
+		goto out2;
+
+	rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!rbuf) {
+		rc = -ENOMEM;
+		goto out1;
+	}
+
+	if (!(file->f_mode & FMODE_READ)) {
+		file->f_mode |= FMODE_READ;
+		read = 1;
+	}
+
+	for (offset = 0; offset < i_size; offset += rbuf_len) {
+		rbuf_len = ima_kernel_read(file, offset, rbuf, PAGE_SIZE);
+		if (rbuf_len < 0) {
+			rc = rbuf_len;
+			break;
+		}
+		if (rbuf_len == 0)
+			break;
+
+		sg_init_one(&sg[0], rbuf, rbuf_len);
+		ahash_request_set_crypt(req, sg, NULL, rbuf_len);
+
+		rc = ahash_wait(crypto_ahash_update(req), &res);
+		if (rc)
+			break;
+	}
+	if (read)
+		file->f_mode &= ~FMODE_READ;
+	kfree(rbuf);
+out2:
+	if (!rc) {
+		ahash_request_set_crypt(req, NULL, hash->digest, 0);
+		rc = ahash_wait(crypto_ahash_final(req), &res);
+	}
+out1:
+	ahash_request_free(req);
+	return rc;
+}
+
+static int ima_calc_file_ahash(struct file *file, struct ima_digest_data *hash)
+{
+	struct crypto_ahash *tfm;
+	int rc;
+
+	tfm = ima_alloc_atfm(hash->algo);
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
+
+	rc = ima_calc_file_hash_atfm(file, hash, tfm);
+
+	ima_free_atfm(tfm);
+
+	return rc;
+}
+
 static int ima_calc_file_hash_tfm(struct file *file,
 				  struct ima_digest_data *hash,
 				  struct crypto_shash *tfm)
@@ -156,7 +313,7 @@ out:
 	return rc;
 }
 
-int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
+static int ima_calc_file_shash(struct file *file, struct ima_digest_data *hash)
 {
 	struct crypto_shash *tfm;
 	int rc;
@@ -172,6 +329,26 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
 	return rc;
 }
 
+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));
+
+	/* shash is more efficient for small data
+	 * ahash performance depends on data size and particular HW
+	 * ima_ahash_size allows to specify the best value for the system
+	 */
+	if (ima_ahash_size && i_size >= ima_ahash_size) {
+		rc = ima_calc_file_ahash(file, hash);
+		if (!rc)
+			return 0;
+	}
+	/* fallback to shash if ahash failed */
+	return ima_calc_file_shash(file, hash);
+}
+
 /*
  * Calculate the hash of template data
  */
-- 
1.9.1

Re: [PATCH v2 1/3] ima: use ahash API for file hash calculation

[Mimi Zohar]

On Tue, 2014-07-01 at 23:12 +0300, Dmitry Kasatkin wrote: 
> Async hash API allows to use HW acceleration for hash calculation.
> It may give significant performance gain or/and reduce power consumption,
> which might be very beneficial for battery powered devices.
> 
> This patch introduces hash calculation using ahash API.
> 
> ahash performance depends on data size and particular HW. Under certain
> limit, depending on the system, shash performance may be better.
> 
> This patch defines 'ima_ahash' kernel parameter which can be used to
> define minimal file size to use with ahash. When file size is not set
> or smaller than defined by the parameter, shash will be used.

Thank you for the updated patches. The changes should be listed here in
the patch description.

I keep going back and forth as to whether the ahash routines should be
totally separate, as posted, from the shash routines.  Having separate
functions is very clear/clean, but there is quite a bit of code
duplication (eg. ima_calc_file_hash()/ima_calc_file_ahash(),
ima_free_tfm()/ima_free_atfm(), ima_alloc_tfm()/ima_alloc_atfm()).

Soliciting comments ...

> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>

> ---
>  Documentation/kernel-parameters.txt |   5 +
>  security/integrity/ima/ima_crypto.c | 185 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 186 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 5a214a3..b406f5c 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1291,6 +1291,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  	ihash_entries=	[KNL]
>  			Set number of hash buckets for inode cache.
> 
> +	ima_ahash=	[IMA] Asynchronous hash usage parameters
> +			Format: <min_file_size>
> +			Set the minimal file size when use asynchronous hash.
> +			If ima_ahash is not provided, ahash usage is disabled.
> +
>  	ima_appraise=	[IMA] appraise integrity measurements
>  			Format: { "off" | "enforce" | "fix" }
>  			default: "enforce"
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index f82d1d7..5eb19b4 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -25,7 +25,27 @@
>  #include <crypto/hash_info.h>
>  #include "ima.h"
> 
> +

Extra blank line not needed.

> +struct ahash_completion {
> +	struct completion completion;
> +	int err;
> +};
> +
>  static struct crypto_shash *ima_shash_tfm;
> +static struct crypto_ahash *ima_ahash_tfm;
> +
> +/* minimal file size for ahash use */
> +static loff_t ima_ahash_size;
> +
> +static int __init ima_ahash_setup(char *str)
> +{
> +	int rc;
> +
> +	rc = kstrtoll(str, 10, &ima_ahash_size);
> +
> +	return !rc;
> +}
> +__setup("ima_ahash=", ima_ahash_setup);
> 
>  /**
>   * ima_kernel_read - read file content
> @@ -93,9 +113,146 @@ static void ima_free_tfm(struct crypto_shash *tfm)
>  		crypto_free_shash(tfm);
>  }
> 
> -/*
> - * Calculate the MD5/SHA1 file digest
> - */
> +static struct crypto_ahash *ima_alloc_atfm(enum hash_algo algo)
> +{
> +	struct crypto_ahash *tfm = ima_ahash_tfm;
> +	int rc;
> +
> +	if ((algo != ima_hash_algo && algo < HASH_ALGO__LAST) || !tfm) {
> +		tfm = crypto_alloc_ahash(hash_algo_name[algo], 0, 0);
> +		if (!IS_ERR(tfm)) {
> +			if (algo == ima_hash_algo)
> +				ima_ahash_tfm = tfm;
> +		} else {
> +			rc = PTR_ERR(tfm);
> +			pr_err("Can not allocate %s (reason: %d)\n",
> +			       hash_algo_name[algo], rc);
> +		}
> +	}
> +	return tfm;
> +}
> +
> +static void ima_free_atfm(struct crypto_ahash *tfm)
> +{
> +	if (tfm != ima_ahash_tfm)
> +		crypto_free_ahash(tfm);
> +}
> +
> +static void ahash_complete(struct crypto_async_request *req, int err)
> +{
> +	struct ahash_completion *res = req->data;
> +
> +	if (err == -EINPROGRESS)
> +		return;
> +	res->err = err;
> +	complete(&res->completion);
> +}
> +
> +static int ahash_wait(int err, struct ahash_completion *res)
> +{
> +	switch (err) {
> +	case 0:
> +		break;
> +	case -EINPROGRESS:
> +	case -EBUSY:
> +		wait_for_completion(&res->completion);
> +		reinit_completion(&res->completion);
> +		err = res->err;
> +		/* fall through */
> +	default:
> +		pr_crit("ahash calculation failed: err: %d\n", err);
> +	}
> +
> +	return err;
> +}
> +
> +static int ima_calc_file_hash_atfm(struct file *file,
> +				   struct ima_digest_data *hash,
> +				   struct crypto_ahash *tfm)
> +{
> +	loff_t i_size, offset;
> +	char *rbuf;
> +	int rc, read = 0, rbuf_len;
> +	struct ahash_request *req;
> +	struct scatterlist sg[1];
> +	struct ahash_completion res;
> +
> +	hash->length = crypto_ahash_digestsize(tfm);
> +
> +	req = ahash_request_alloc(tfm, GFP_KERNEL);
> +	if (!req)
> +		return -ENOMEM;
> +
> +	init_completion(&res.completion);
> +	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
> +				   CRYPTO_TFM_REQ_MAY_SLEEP,
> +				   ahash_complete, &res);
> +
> +	rc = ahash_wait(crypto_ahash_init(req), &res);
> +	if (rc)
> +		goto out1;
> +
> +	i_size = i_size_read(file_inode(file));
> +
> +	if (i_size == 0)
> +		goto out2;
> +
> +	rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> +	if (!rbuf) {
> +		rc = -ENOMEM;
> +		goto out1;
> +	}
> +
> +	if (!(file->f_mode & FMODE_READ)) {
> +		file->f_mode |= FMODE_READ;
> +		read = 1;
> +	}
> +
> +	for (offset = 0; offset < i_size; offset += rbuf_len) {
> +		rbuf_len = ima_kernel_read(file, offset, rbuf, PAGE_SIZE);
> +		if (rbuf_len < 0) {
> +			rc = rbuf_len;
> +			vi break;
> +		}
> +		if (rbuf_len == 0)
> +			break;
> +
> +		sg_init_one(&sg[0], rbuf, rbuf_len);
> +		ahash_request_set_crypt(req, sg, NULL, rbuf_len);
> +
> +		rc = ahash_wait(crypto_ahash_update(req), &res);
> +		if (rc)
> +			break;
> +	}
> +	if (read)
> +		file->f_mode &= ~FMODE_READ;
> +	kfree(rbuf);
> +out2:
> +	if (!rc) {
> +		ahash_request_set_crypt(req, NULL, hash->digest, 0);
> +		rc = ahash_wait(crypto_ahash_final(req), &res);
> +	}
> +out1:
> +	ahash_request_free(req);
> +	return rc;
> +}
> +
> +static int ima_calc_file_ahash(struct file *file, struct ima_digest_data *hash)
> +{
> +	struct crypto_ahash *tfm;
> +	int rc;
> +
> +	tfm = ima_alloc_atfm(hash->algo);
> +	if (IS_ERR(tfm))
> +		return PTR_ERR(tfm);
> +
> +	rc = ima_calc_file_hash_atfm(file, hash, tfm);
> +
> +	ima_free_atfm(tfm);
> +
> +	return rc;
> +}
> +
>  static int ima_calc_file_hash_tfm(struct file *file,
>  				  struct ima_digest_data *hash,
>  				  struct crypto_shash *tfm)
> @@ -156,7 +313,7 @@ out:
>  	return rc;
>  }
> 
> -int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
> +static int ima_calc_file_shash(struct file *file, struct ima_digest_data *hash)
>  {
>  	struct crypto_shash *tfm;
>  	int rc;
> @@ -172,6 +329,26 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
>  	return rc;
>  }
> 
> +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));

Depending on the config options, i_size_read() does more than just
access the inode's i_size.   Instead of calling it again, perhaps it
should be passed to ima_calc_file_hash()/ahash().

> +
> +	/* shash is more efficient for small data
> +	 * ahash performance depends on data size and particular HW
> +	 * ima_ahash_size allows to specify the best value for the system
> +	 */

Function descriptions belong above the function, not inside.

> +	if (ima_ahash_size && i_size >= ima_ahash_size) {
> +		rc = ima_calc_file_ahash(file, hash);
> +		if (!rc)
> +			return 0;
> +	}
> +	/* fallback to shash if ahash failed */

Same here.

thanks,

Mimi

> +	return ima_calc_file_shash(file, hash);
> +}
> +
>  /*
>   * Calculate the hash of template data
>   */

Re: [PATCH v2 1/3] ima: use ahash API for file hash calculation

[Dmitry Kasatkin]

On 2 July 2014 19:40, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Tue, 2014-07-01 at 23:12 +0300, Dmitry Kasatkin wrote:
>> Async hash API allows to use HW acceleration for hash calculation.
>> It may give significant performance gain or/and reduce power consumption,
>> which might be very beneficial for battery powered devices.
>>
>> This patch introduces hash calculation using ahash API.
>>
>> ahash performance depends on data size and particular HW. Under certain
>> limit, depending on the system, shash performance may be better.
>>
>> This patch defines 'ima_ahash' kernel parameter which can be used to
>> define minimal file size to use with ahash. When file size is not set
>> or smaller than defined by the parameter, shash will be used.
>
> Thank you for the updated patches. The changes should be listed here in
> the patch description.
>
> I keep going back and forth as to whether the ahash routines should be
> totally separate, as posted, from the shash routines.  Having separate
> functions is very clear/clean, but there is quite a bit of code
> duplication (eg. ima_calc_file_hash()/ima_calc_file_ahash(),
> ima_free_tfm()/ima_free_atfm(), ima_alloc_tfm()/ima_alloc_atfm()).
>
> Soliciting comments ...
>

I think what you say is a "pattern": alloc/calc/free.
But the code uses different API mostly and there very small duplication...

In fact with this 'clean' separation we can have CONFIG_ option to
ifdef the code ot have it in separate file for those who really want
to make smallest kernel possible...


>> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
>
>> ---
>>  Documentation/kernel-parameters.txt |   5 +
>>  security/integrity/ima/ima_crypto.c | 185 +++++++++++++++++++++++++++++++++++-
>>  2 files changed, 186 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index 5a214a3..b406f5c 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -1291,6 +1291,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>       ihash_entries=  [KNL]
>>                       Set number of hash buckets for inode cache.
>>
>> +     ima_ahash=      [IMA] Asynchronous hash usage parameters
>> +                     Format: <min_file_size>
>> +                     Set the minimal file size when use asynchronous hash.
>> +                     If ima_ahash is not provided, ahash usage is disabled.
>> +
>>       ima_appraise=   [IMA] appraise integrity measurements
>>                       Format: { "off" | "enforce" | "fix" }
>>                       default: "enforce"
>> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
>> index f82d1d7..5eb19b4 100644
>> --- a/security/integrity/ima/ima_crypto.c
>> +++ b/security/integrity/ima/ima_crypto.c
>> @@ -25,7 +25,27 @@
>>  #include <crypto/hash_info.h>
>>  #include "ima.h"
>>
>> +
>
> Extra blank line not needed.
>
>> +struct ahash_completion {
>> +     struct completion completion;
>> +     int err;
>> +};
>> +
>>  static struct crypto_shash *ima_shash_tfm;
>> +static struct crypto_ahash *ima_ahash_tfm;
>> +
>> +/* minimal file size for ahash use */
>> +static loff_t ima_ahash_size;
>> +
>> +static int __init ima_ahash_setup(char *str)
>> +{
>> +     int rc;
>> +
>> +     rc = kstrtoll(str, 10, &ima_ahash_size);
>> +
>> +     return !rc;
>> +}
>> +__setup("ima_ahash=", ima_ahash_setup);
>>
>>  /**
>>   * ima_kernel_read - read file content
>> @@ -93,9 +113,146 @@ static void ima_free_tfm(struct crypto_shash *tfm)
>>               crypto_free_shash(tfm);
>>  }
>>
>> -/*
>> - * Calculate the MD5/SHA1 file digest
>> - */
>> +static struct crypto_ahash *ima_alloc_atfm(enum hash_algo algo)
>> +{
>> +     struct crypto_ahash *tfm = ima_ahash_tfm;
>> +     int rc;
>> +
>> +     if ((algo != ima_hash_algo && algo < HASH_ALGO__LAST) || !tfm) {
>> +             tfm = crypto_alloc_ahash(hash_algo_name[algo], 0, 0);
>> +             if (!IS_ERR(tfm)) {
>> +                     if (algo == ima_hash_algo)
>> +                             ima_ahash_tfm = tfm;
>> +             } else {
>> +                     rc = PTR_ERR(tfm);
>> +                     pr_err("Can not allocate %s (reason: %d)\n",
>> +                            hash_algo_name[algo], rc);
>> +             }
>> +     }
>> +     return tfm;
>> +}
>> +
>> +static void ima_free_atfm(struct crypto_ahash *tfm)
>> +{
>> +     if (tfm != ima_ahash_tfm)
>> +             crypto_free_ahash(tfm);
>> +}
>> +
>> +static void ahash_complete(struct crypto_async_request *req, int err)
>> +{
>> +     struct ahash_completion *res = req->data;
>> +
>> +     if (err == -EINPROGRESS)
>> +             return;
>> +     res->err = err;
>> +     complete(&res->completion);
>> +}
>> +
>> +static int ahash_wait(int err, struct ahash_completion *res)
>> +{
>> +     switch (err) {
>> +     case 0:
>> +             break;
>> +     case -EINPROGRESS:
>> +     case -EBUSY:
>> +             wait_for_completion(&res->completion);
>> +             reinit_completion(&res->completion);
>> +             err = res->err;
>> +             /* fall through */
>> +     default:
>> +             pr_crit("ahash calculation failed: err: %d\n", err);
>> +     }
>> +
>> +     return err;
>> +}
>> +
>> +static int ima_calc_file_hash_atfm(struct file *file,
>> +                                struct ima_digest_data *hash,
>> +                                struct crypto_ahash *tfm)
>> +{
>> +     loff_t i_size, offset;
>> +     char *rbuf;
>> +     int rc, read = 0, rbuf_len;
>> +     struct ahash_request *req;
>> +     struct scatterlist sg[1];
>> +     struct ahash_completion res;
>> +
>> +     hash->length = crypto_ahash_digestsize(tfm);
>> +
>> +     req = ahash_request_alloc(tfm, GFP_KERNEL);
>> +     if (!req)
>> +             return -ENOMEM;
>> +
>> +     init_completion(&res.completion);
>> +     ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
>> +                                CRYPTO_TFM_REQ_MAY_SLEEP,
>> +                                ahash_complete, &res);
>> +
>> +     rc = ahash_wait(crypto_ahash_init(req), &res);
>> +     if (rc)
>> +             goto out1;
>> +
>> +     i_size = i_size_read(file_inode(file));
>> +
>> +     if (i_size == 0)
>> +             goto out2;
>> +
>> +     rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL);
>> +     if (!rbuf) {
>> +             rc = -ENOMEM;
>> +             goto out1;
>> +     }
>> +
>> +     if (!(file->f_mode & FMODE_READ)) {
>> +             file->f_mode |= FMODE_READ;
>> +             read = 1;
>> +     }
>> +
>> +     for (offset = 0; offset < i_size; offset += rbuf_len) {
>> +             rbuf_len = ima_kernel_read(file, offset, rbuf, PAGE_SIZE);
>> +             if (rbuf_len < 0) {
>> +                     rc = rbuf_len;
>> +                     vi break;
>> +             }
>> +             if (rbuf_len == 0)
>> +                     break;
>> +
>> +             sg_init_one(&sg[0], rbuf, rbuf_len);
>> +             ahash_request_set_crypt(req, sg, NULL, rbuf_len);
>> +
>> +             rc = ahash_wait(crypto_ahash_update(req), &res);
>> +             if (rc)
>> +                     break;
>> +     }
>> +     if (read)
>> +             file->f_mode &= ~FMODE_READ;
>> +     kfree(rbuf);
>> +out2:
>> +     if (!rc) {
>> +             ahash_request_set_crypt(req, NULL, hash->digest, 0);
>> +             rc = ahash_wait(crypto_ahash_final(req), &res);
>> +     }
>> +out1:
>> +     ahash_request_free(req);
>> +     return rc;
>> +}
>> +
>> +static int ima_calc_file_ahash(struct file *file, struct ima_digest_data *hash)
>> +{
>> +     struct crypto_ahash *tfm;
>> +     int rc;
>> +
>> +     tfm = ima_alloc_atfm(hash->algo);
>> +     if (IS_ERR(tfm))
>> +             return PTR_ERR(tfm);
>> +
>> +     rc = ima_calc_file_hash_atfm(file, hash, tfm);
>> +
>> +     ima_free_atfm(tfm);
>> +
>> +     return rc;
>> +}
>> +
>>  static int ima_calc_file_hash_tfm(struct file *file,
>>                                 struct ima_digest_data *hash,
>>                                 struct crypto_shash *tfm)
>> @@ -156,7 +313,7 @@ out:
>>       return rc;
>>  }
>>
>> -int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
>> +static int ima_calc_file_shash(struct file *file, struct ima_digest_data *hash)
>>  {
>>       struct crypto_shash *tfm;
>>       int rc;
>> @@ -172,6 +329,26 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
>>       return rc;
>>  }
>>
>> +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));
>
> Depending on the config options, i_size_read() does more than just
> access the inode's i_size.   Instead of calling it again, perhaps it
> should be passed to ima_calc_file_hash()/ahash().
>

I thought about it.. Indeed let's make it like that.


>> +
>> +     /* shash is more efficient for small data
>> +      * ahash performance depends on data size and particular HW
>> +      * ima_ahash_size allows to specify the best value for the system
>> +      */
>
> Function descriptions belong above the function, not inside.
>

This is a comment to the code..

>> +     if (ima_ahash_size && i_size >= ima_ahash_size) {
>> +             rc = ima_calc_file_ahash(file, hash);
>> +             if (!rc)
>> +                     return 0;
>> +     }
>> +     /* fallback to shash if ahash failed */
>
> Same here.
>

Same here...

But sure it can move to function description...

> thanks,
>
> Mimi
>
>> +     return ima_calc_file_shash(file, hash);
>> +}
>> +
>>  /*
>>   * Calculate the hash of template data
>>   */
>
>

Thanks.

Re: [PATCH v2 1/3] ima: use ahash API for file hash calculation

[Mimi Zohar]

On Wed, 2014-07-02 at 21:20 +0300, Dmitry Kasatkin wrote: 
> On 2 July 2014 19:40, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Tue, 2014-07-01 at 23:12 +0300, Dmitry Kasatkin wrote:
> >> Async hash API allows to use HW acceleration for hash calculation.
> >> It may give significant performance gain or/and reduce power consumption,
> >> which might be very beneficial for battery powered devices.
> >>
> >> This patch introduces hash calculation using ahash API.
> >>
> >> ahash performance depends on data size and particular HW. Under certain
> >> limit, depending on the system, shash performance may be better.
> >>
> >> This patch defines 'ima_ahash' kernel parameter which can be used to
> >> define minimal file size to use with ahash. When file size is not set
> >> or smaller than defined by the parameter, shash will be used.
> >
> > Thank you for the updated patches. The changes should be listed here in
> > the patch description.
> >
> > I keep going back and forth as to whether the ahash routines should be
> > totally separate, as posted, from the shash routines.  Having separate
> > functions is very clear/clean, but there is quite a bit of code
> > duplication (eg. ima_calc_file_hash()/ima_calc_file_ahash(),
> > ima_free_tfm()/ima_free_atfm(), ima_alloc_tfm()/ima_alloc_atfm()).
> >
> > Soliciting comments ...
> >
> 
> I think what you say is a "pattern": alloc/calc/free.
> But the code uses different API mostly and there very small duplication...
> 
> In fact with this 'clean' separation we can have CONFIG_ option to
> ifdef the code ot have it in separate file for those who really want
> to make smallest kernel possible...

Ok, this is an acceptable reason.

thanks,

Mimi

> 
> >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> >
> >> ---
> >>  Documentation/kernel-parameters.txt |   5 +
> >>  security/integrity/ima/ima_crypto.c | 185 +++++++++++++++++++++++++++++++++++-
> >>  2 files changed, 186 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> >> index 5a214a3..b406f5c 100644
> >> --- a/Documentation/kernel-parameters.txt
> >> +++ b/Documentation/kernel-parameters.txt
> >> @@ -1291,6 +1291,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> >>       ihash_entries=  [KNL]
> >>                       Set number of hash buckets for inode cache.
> >>
> >> +     ima_ahash=      [IMA] Asynchronous hash usage parameters
> >> +                     Format: <min_file_size>
> >> +                     Set the minimal file size when use asynchronous hash.
> >> +                     If ima_ahash is not provided, ahash usage is disabled.
> >> +
> >>       ima_appraise=   [IMA] appraise integrity measurements
> >>                       Format: { "off" | "enforce" | "fix" }
> >>                       default: "enforce"
> >> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> >> index f82d1d7..5eb19b4 100644
> >> --- a/security/integrity/ima/ima_crypto.c
> >> +++ b/security/integrity/ima/ima_crypto.c
> >> @@ -25,7 +25,27 @@
> >>  #include <crypto/hash_info.h>
> >>  #include "ima.h"
> >>
> >> +
> >
> > Extra blank line not needed.
> >
> >> +struct ahash_completion {
> >> +     struct completion completion;
> >> +     int err;
> >> +};
> >> +
> >>  static struct crypto_shash *ima_shash_tfm;
> >> +static struct crypto_ahash *ima_ahash_tfm;
> >> +
> >> +/* minimal file size for ahash use */
> >> +static loff_t ima_ahash_size;
> >> +
> >> +static int __init ima_ahash_setup(char *str)
> >> +{
> >> +     int rc;
> >> +
> >> +     rc = kstrtoll(str, 10, &ima_ahash_size);
> >> +
> >> +     return !rc;
> >> +}
> >> +__setup("ima_ahash=", ima_ahash_setup);
> >>
> >>  /**
> >>   * ima_kernel_read - read file content
> >> @@ -93,9 +113,146 @@ static void ima_free_tfm(struct crypto_shash *tfm)
> >>               crypto_free_shash(tfm);
> >>  }
> >>
> >> -/*
> >> - * Calculate the MD5/SHA1 file digest
> >> - */
> >> +static struct crypto_ahash *ima_alloc_atfm(enum hash_algo algo)
> >> +{
> >> +     struct crypto_ahash *tfm = ima_ahash_tfm;
> >> +     int rc;
> >> +
> >> +     if ((algo != ima_hash_algo && algo < HASH_ALGO__LAST) || !tfm) {
> >> +             tfm = crypto_alloc_ahash(hash_algo_name[algo], 0, 0);
> >> +             if (!IS_ERR(tfm)) {
> >> +                     if (algo == ima_hash_algo)
> >> +                             ima_ahash_tfm = tfm;
> >> +             } else {
> >> +                     rc = PTR_ERR(tfm);
> >> +                     pr_err("Can not allocate %s (reason: %d)\n",
> >> +                            hash_algo_name[algo], rc);
> >> +             }
> >> +     }
> >> +     return tfm;
> >> +}
> >> +
> >> +static void ima_free_atfm(struct crypto_ahash *tfm)
> >> +{
> >> +     if (tfm != ima_ahash_tfm)
> >> +             crypto_free_ahash(tfm);
> >> +}
> >> +
> >> +static void ahash_complete(struct crypto_async_request *req, int err)
> >> +{
> >> +     struct ahash_completion *res = req->data;
> >> +
> >> +     if (err == -EINPROGRESS)
> >> +             return;
> >> +     res->err = err;
> >> +     complete(&res->completion);
> >> +}
> >> +
> >> +static int ahash_wait(int err, struct ahash_completion *res)
> >> +{
> >> +     switch (err) {
> >> +     case 0:
> >> +             break;
> >> +     case -EINPROGRESS:
> >> +     case -EBUSY:
> >> +             wait_for_completion(&res->completion);
> >> +             reinit_completion(&res->completion);
> >> +             err = res->err;
> >> +             /* fall through */
> >> +     default:
> >> +             pr_crit("ahash calculation failed: err: %d\n", err);
> >> +     }
> >> +
> >> +     return err;
> >> +}
> >> +
> >> +static int ima_calc_file_hash_atfm(struct file *file,
> >> +                                struct ima_digest_data *hash,
> >> +                                struct crypto_ahash *tfm)
> >> +{
> >> +     loff_t i_size, offset;
> >> +     char *rbuf;
> >> +     int rc, read = 0, rbuf_len;
> >> +     struct ahash_request *req;
> >> +     struct scatterlist sg[1];
> >> +     struct ahash_completion res;
> >> +
> >> +     hash->length = crypto_ahash_digestsize(tfm);
> >> +
> >> +     req = ahash_request_alloc(tfm, GFP_KERNEL);
> >> +     if (!req)
> >> +             return -ENOMEM;
> >> +
> >> +     init_completion(&res.completion);
> >> +     ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
> >> +                                CRYPTO_TFM_REQ_MAY_SLEEP,
> >> +                                ahash_complete, &res);
> >> +
> >> +     rc = ahash_wait(crypto_ahash_init(req), &res);
> >> +     if (rc)
> >> +             goto out1;
> >> +
> >> +     i_size = i_size_read(file_inode(file));
> >> +
> >> +     if (i_size == 0)
> >> +             goto out2;
> >> +
> >> +     rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> >> +     if (!rbuf) {
> >> +             rc = -ENOMEM;
> >> +             goto out1;
> >> +     }
> >> +
> >> +     if (!(file->f_mode & FMODE_READ)) {
> >> +             file->f_mode |= FMODE_READ;
> >> +             read = 1;
> >> +     }
> >> +
> >> +     for (offset = 0; offset < i_size; offset += rbuf_len) {
> >> +             rbuf_len = ima_kernel_read(file, offset, rbuf, PAGE_SIZE);
> >> +             if (rbuf_len < 0) {
> >> +                     rc = rbuf_len;
> >> +                     vi break;
> >> +             }
> >> +             if (rbuf_len == 0)
> >> +                     break;
> >> +
> >> +             sg_init_one(&sg[0], rbuf, rbuf_len);
> >> +             ahash_request_set_crypt(req, sg, NULL, rbuf_len);
> >> +
> >> +             rc = ahash_wait(crypto_ahash_update(req), &res);
> >> +             if (rc)
> >> +                     break;
> >> +     }
> >> +     if (read)
> >> +             file->f_mode &= ~FMODE_READ;
> >> +     kfree(rbuf);
> >> +out2:
> >> +     if (!rc) {
> >> +             ahash_request_set_crypt(req, NULL, hash->digest, 0);
> >> +             rc = ahash_wait(crypto_ahash_final(req), &res);
> >> +     }
> >> +out1:
> >> +     ahash_request_free(req);
> >> +     return rc;
> >> +}
> >> +
> >> +static int ima_calc_file_ahash(struct file *file, struct ima_digest_data *hash)
> >> +{
> >> +     struct crypto_ahash *tfm;
> >> +     int rc;
> >> +
> >> +     tfm = ima_alloc_atfm(hash->algo);
> >> +     if (IS_ERR(tfm))
> >> +             return PTR_ERR(tfm);
> >> +
> >> +     rc = ima_calc_file_hash_atfm(file, hash, tfm);
> >> +
> >> +     ima_free_atfm(tfm);
> >> +
> >> +     return rc;
> >> +}
> >> +
> >>  static int ima_calc_file_hash_tfm(struct file *file,
> >>                                 struct ima_digest_data *hash,
> >>                                 struct crypto_shash *tfm)
> >> @@ -156,7 +313,7 @@ out:
> >>       return rc;
> >>  }
> >>
> >> -int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
> >> +static int ima_calc_file_shash(struct file *file, struct ima_digest_data *hash)
> >>  {
> >>       struct crypto_shash *tfm;
> >>       int rc;
> >> @@ -172,6 +329,26 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
> >>       return rc;
> >>  }
> >>
> >> +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));
> >
> > Depending on the config options, i_size_read() does more than just
> > access the inode's i_size.   Instead of calling it again, perhaps it
> > should be passed to ima_calc_file_hash()/ahash().
> >
> 
> I thought about it.. Indeed let's make it like that.
> 
> 
> >> +
> >> +     /* shash is more efficient for small data
> >> +      * ahash performance depends on data size and particular HW
> >> +      * ima_ahash_size allows to specify the best value for the system
> >> +      */
> >
> > Function descriptions belong above the function, not inside.
> >
> 
> This is a comment to the code..
> 
> >> +     if (ima_ahash_size && i_size >= ima_ahash_size) {
> >> +             rc = ima_calc_file_ahash(file, hash);
> >> +             if (!rc)
> >> +                     return 0;
> >> +     }
> >> +     /* fallback to shash if ahash failed */
> >
> > Same here.
> >
> 
> Same here...
> 
> But sure it can move to function description...
> 
> > thanks,
> >
> > Mimi
> >
> >> +     return ima_calc_file_shash(file, hash);
> >> +}
> >> +
> >>  /*
> >>   * Calculate the hash of template data
> >>   */
> >
> >
> 
> Thanks.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v2 1/3] ima: use ahash API for file hash calculation

[Mimi Zohar]

On Tue, 2014-07-01 at 23:12 +0300, Dmitry Kasatkin wrote:

> -/*
> - * Calculate the MD5/SHA1 file digest
> - */
> +static struct crypto_ahash *ima_alloc_atfm(enum hash_algo algo)
> +{
> +	struct crypto_ahash *tfm = ima_ahash_tfm;
> +	int rc;
> +
> +	if ((algo != ima_hash_algo && algo < HASH_ALGO__LAST) || !tfm) {
> +		tfm = crypto_alloc_ahash(hash_algo_name[algo], 0, 0);

In the case where algo isn't the same as ima_hash_algo, won't this
replace the existing ima_ahash_tfm without freeing it?

Mimi

> +		if (!IS_ERR(tfm)) {
> +			if (algo == ima_hash_algo)
> +				ima_ahash_tfm = tfm;
> +		} else {
> +			rc = PTR_ERR(tfm);
> +			pr_err("Can not allocate %s (reason: %d)\n",
> +			       hash_algo_name[algo], rc);
> +		}
> +	}
> +	return tfm;
> +}
> +
> +static void ima_free_atfm(struct crypto_ahash *tfm)
> +{
> +	if (tfm != ima_ahash_tfm)
> +		crypto_free_ahash(tfm);
> +}

Re: [PATCH v2 1/3] ima: use ahash API for file hash calculation

[Dmitry Kasatkin]

On 2 July 2014 20:44, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Tue, 2014-07-01 at 23:12 +0300, Dmitry Kasatkin wrote:
>
>> -/*
>> - * Calculate the MD5/SHA1 file digest
>> - */
>> +static struct crypto_ahash *ima_alloc_atfm(enum hash_algo algo)
>> +{
>> +     struct crypto_ahash *tfm = ima_ahash_tfm;
>> +     int rc;
>> +
>> +     if ((algo != ima_hash_algo && algo < HASH_ALGO__LAST) || !tfm) {
>> +             tfm = crypto_alloc_ahash(hash_algo_name[algo], 0, 0);
>
> In the case where algo isn't the same as ima_hash_algo, won't this
> replace the existing ima_ahash_tfm without freeing it?
>

Look to next comment...

> Mimi
>
>> +             if (!IS_ERR(tfm)) {
>> +                     if (algo == ima_hash_algo)
>> +                             ima_ahash_tfm = tfm;

Above will set only new tfm for default ima_hash_algo...

Dmitry

>> +             } else {
>> +                     rc = PTR_ERR(tfm);
>> +                     pr_err("Can not allocate %s (reason: %d)\n",
>> +                            hash_algo_name[algo], rc);
>> +             }
>> +     }
>> +     return tfm;
>> +}
>> +
>> +static void ima_free_atfm(struct crypto_ahash *tfm)
>> +{
>> +     if (tfm != ima_ahash_tfm)
>> +             crypto_free_ahash(tfm);
>> +}
>
>
>



-- 
Thanks,
Dmitry

Re: [PATCH v2 1/3] ima: use ahash API for file hash calculation

[Mimi Zohar]

On Wed, 2014-07-02 at 21:21 +0300, Dmitry Kasatkin wrote: 
> On 2 July 2014 20:44, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Tue, 2014-07-01 at 23:12 +0300, Dmitry Kasatkin wrote:
> >
> >> -/*
> >> - * Calculate the MD5/SHA1 file digest
> >> - */
> >> +static struct crypto_ahash *ima_alloc_atfm(enum hash_algo algo)
> >> +{
> >> +     struct crypto_ahash *tfm = ima_ahash_tfm;
> >> +     int rc;
> >> +
> >> +     if ((algo != ima_hash_algo && algo < HASH_ALGO__LAST) || !tfm) {
> >> +             tfm = crypto_alloc_ahash(hash_algo_name[algo], 0, 0);
> >
> > In the case where algo isn't the same as ima_hash_algo, won't this
> > replace the existing ima_ahash_tfm without freeing it?
> >
> 
> Look to next comment...

Yep, my mistake in reading the code.

Mimi

Re: [PATCH v2 1/3] ima: use ahash API for file hash calculation

[Dave Hansen]

On 07/01/2014 01:12 PM, Dmitry Kasatkin wrote:
> +	ima_ahash=	[IMA] Asynchronous hash usage parameters
> +			Format: <min_file_size>
> +			Set the minimal file size when use asynchronous hash.
> +			If ima_ahash is not provided, ahash usage is disabled.

<groan> ... another boot option...

Can we just set this to something sane, and then make a sysctl or
something else at runtime to tweak it?  The kernel won't use IMA much
before userspace comes up, and it can surely live with a slightly
suboptimal tuning until the boot scripts have a chance to go bang the
tunable.

We should reserve command-line parameters for things that really need
tweaking in early boot or are _needed_ to boot.

Re: [PATCH v2 1/3] ima: use ahash API for file hash calculation

[Dmitry Kasatkin]

On 2 July 2014 21:33, Dave Hansen <dave.hansen@intel.com> wrote:
> On 07/01/2014 01:12 PM, Dmitry Kasatkin wrote:
>> +     ima_ahash=      [IMA] Asynchronous hash usage parameters
>> +                     Format: <min_file_size>
>> +                     Set the minimal file size when use asynchronous hash.
>> +                     If ima_ahash is not provided, ahash usage is disabled.
>
> <groan> ... another boot option...
>
> Can we just set this to something sane, and then make a sysctl or
> something else at runtime to tweak it?  The kernel won't use IMA much
> before userspace comes up, and it can surely live with a slightly
> suboptimal tuning until the boot scripts have a chance to go bang the
> tunable.
>
> We should reserve command-line parameters for things that really need
> tweaking in early boot or are _needed_ to boot.

Thanks... Good that you commented about it.
I thought to have module_param, but as IMA is not a module, ended up
with __setup..
Quite many always-builtin stuff use module_param... Also in LSM...
Runtime can then tweak it for better performance...

Is module param good enough or it should be sysctl?

- Dmitry

Re: [PATCH v2 1/3] ima: use ahash API for file hash calculation

[Dave Hansen]

On 07/02/2014 11:40 AM, Dmitry Kasatkin wrote:
>> > We should reserve command-line parameters for things that really need
>> > tweaking in early boot or are _needed_ to boot.
...
> Is module param good enough or it should be sysctl?

Doesn't matter to me much.  sysctls seem to be the easiest things to
configure and make persistent.  I'm not sure if the normal
/etc/modprobe.d things work for built-in features, but if they do I'd
say a modparam is fine.

[PATCH v2 2/3] ima: introduce multi-page collect buffers

[Dmitry Kasatkin]

Use of multiple-page collect buffers reduces:
1) the number of block IO requests
2) the number of asynchronous hash update requests

Second is important for HW accelerated hashing, because significant
amount of time is spent for preparation of hash update operation,
which includes configuring acceleration HW, DMA engine, etc...
Thus, HW accelerators are more efficient when working on large
chunks of data.

This patch introduces usage of multi-page collect buffers. Buffer size
can be specified by providing additional option to the 'ima_ahash='
kernel parameter. 'ima_ahash=2048,16384' specifies that minimal file
size to use ahash is 2048 byes and buffer size is 16384 bytes.
Default buffer size is 4096 bytes.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 Documentation/kernel-parameters.txt |  3 +-
 security/integrity/ima/ima_crypto.c | 85 ++++++++++++++++++++++++++++++++++---
 2 files changed, 81 insertions(+), 7 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index b406f5c..529ba58 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1292,9 +1292,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			Set number of hash buckets for inode cache.
 
 	ima_ahash=	[IMA] Asynchronous hash usage parameters
-			Format: <min_file_size>
+			Format: <min_file_size>[,<bufsize>]
 			Set the minimal file size when use asynchronous hash.
 			If ima_ahash is not provided, ahash usage is disabled.
+			bufsize - hashing buffer size. 4k if not specified.
 
 	ima_appraise=	[IMA] appraise integrity measurements
 			Format: { "off" | "enforce" | "fix" }
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 5eb19b4..41f2695 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -25,7 +25,6 @@
 #include <crypto/hash_info.h>
 #include "ima.h"
 
-
 struct ahash_completion {
 	struct completion completion;
 	int err;
@@ -36,14 +35,24 @@ static struct crypto_ahash *ima_ahash_tfm;
 
 /* minimal file size for ahash use */
 static loff_t ima_ahash_size;
+/* default is 0 - 1 page. */
+static int ima_max_order;
 
 static int __init ima_ahash_setup(char *str)
 {
-	int rc;
+	int ints[3] = { 0, };
+
+	str = get_options(str, ARRAY_SIZE(ints), ints);
+	if (!ints[0])
+		return 0;
+
+	ima_ahash_size = ints[1];
+	ima_max_order = get_order(ints[2]);
 
-	rc = kstrtoll(str, 10, &ima_ahash_size);
+	pr_info("ima_ahash: minsize=%lld, bufsize=%lu\n",
+		ima_ahash_size, (PAGE_SIZE << ima_max_order));
 
-	return !rc;
+	return 1;
 }
 __setup("ima_ahash=", ima_ahash_setup);
 
@@ -166,6 +175,65 @@ static int ahash_wait(int err, struct ahash_completion *res)
 	return err;
 }
 
+/**
+ * ima_alloc_pages() - Allocated contiguous pages.
+ * @max_size:       Maximum amount of memory to allocate.
+ * @allocated_size: Returned size of actual allocation.
+ * @last_warn:      Should the min_size allocation warn or not.
+ *
+ * Tries to do opportunistic allocation for memory first trying to allocate
+ * max_size amount of memory and then splitting that until zero order is
+ * reached. Allocation is tried without generating allocation warnings unless
+ * last_warn is set. Last_warn set affects only last allocation of zero order.
+ *
+ * Return pointer to allocated memory, or NULL on failure.
+ */
+static void *ima_alloc_pages(loff_t max_size, size_t *allocated_size,
+			     int last_warn)
+{
+	void *ptr;
+	unsigned int order;
+	gfp_t gfp_mask = __GFP_NOWARN | __GFP_WAIT | __GFP_NORETRY;
+
+	order = min(get_order(max_size), ima_max_order);
+
+	for (; order; order--) {
+		ptr = (void *)__get_free_pages(gfp_mask, order);
+		if (ptr) {
+			*allocated_size = PAGE_SIZE << order;
+			return ptr;
+		}
+	}
+
+	/* order is zero - one page */
+
+	gfp_mask = GFP_KERNEL;
+
+	if (!last_warn)
+		gfp_mask |= __GFP_NOWARN;
+
+	ptr = (void *)__get_free_pages(gfp_mask, 0);
+	if (ptr) {
+		*allocated_size = PAGE_SIZE;
+		return ptr;
+	}
+
+	*allocated_size = 0;
+	return NULL;
+}
+
+/**
+ * ima_free_pages() - Free pages allocated by ima_alloc_pages().
+ * @ptr:  Pointer to allocated pages.
+ * @size: Size of allocated buffer.
+ */
+static void ima_free_pages(void *ptr, size_t size)
+{
+	if (!ptr)
+		return;
+	free_pages((unsigned long)ptr, get_order(size));
+}
+
 static int ima_calc_file_hash_atfm(struct file *file,
 				   struct ima_digest_data *hash,
 				   struct crypto_ahash *tfm)
@@ -176,6 +244,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 	struct ahash_request *req;
 	struct scatterlist sg[1];
 	struct ahash_completion res;
+	size_t rbuf_size;
 
 	hash->length = crypto_ahash_digestsize(tfm);
 
@@ -197,7 +266,11 @@ static int ima_calc_file_hash_atfm(struct file *file,
 	if (i_size == 0)
 		goto out2;
 
-	rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+	/*
+	 * Try to allocate maximum size of memory, fail if not even single
+	 * page cannot be allocated.
+	 */
+	rbuf = ima_alloc_pages(i_size, &rbuf_size, 1);
 	if (!rbuf) {
 		rc = -ENOMEM;
 		goto out1;
@@ -226,7 +299,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 	}
 	if (read)
 		file->f_mode &= ~FMODE_READ;
-	kfree(rbuf);
+	ima_free_pages(rbuf, rbuf_size);
 out2:
 	if (!rc) {
 		ahash_request_set_crypt(req, NULL, hash->digest, 0);
-- 
1.9.1

Re: [PATCH v2 2/3] ima: introduce multi-page collect buffers

[Mimi Zohar]

On Tue, 2014-07-01 at 23:12 +0300, Dmitry Kasatkin wrote: 
> Use of multiple-page collect buffers reduces:
> 1) the number of block IO requests
> 2) the number of asynchronous hash update requests
> 
> Second is important for HW accelerated hashing, because significant
> amount of time is spent for preparation of hash update operation,
> which includes configuring acceleration HW, DMA engine, etc...
> Thus, HW accelerators are more efficient when working on large
> chunks of data.
> 
> This patch introduces usage of multi-page collect buffers. Buffer size
> can be specified by providing additional option to the 'ima_ahash='
> kernel parameter. 'ima_ahash=2048,16384' specifies that minimal file
> size to use ahash is 2048 byes and buffer size is 16384 bytes.
> Default buffer size is 4096 bytes.
> 
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> ---
>  Documentation/kernel-parameters.txt |  3 +-
>  security/integrity/ima/ima_crypto.c | 85 ++++++++++++++++++++++++++++++++++---
>  2 files changed, 81 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index b406f5c..529ba58 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1292,9 +1292,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  			Set number of hash buckets for inode cache.
> 
>  	ima_ahash=	[IMA] Asynchronous hash usage parameters
> -			Format: <min_file_size>
> +			Format: <min_file_size>[,<bufsize>]
>  			Set the minimal file size when use asynchronous hash.
>  			If ima_ahash is not provided, ahash usage is disabled.
> +			bufsize - hashing buffer size. 4k if not specified.
> 
>  	ima_appraise=	[IMA] appraise integrity measurements
>  			Format: { "off" | "enforce" | "fix" }
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index 5eb19b4..41f2695 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -25,7 +25,6 @@
>  #include <crypto/hash_info.h>
>  #include "ima.h"
> 
> -
>  struct ahash_completion {
>  	struct completion completion;
>  	int err;
> @@ -36,14 +35,24 @@ static struct crypto_ahash *ima_ahash_tfm;
> 
>  /* minimal file size for ahash use */
>  static loff_t ima_ahash_size;
> +/* default is 0 - 1 page. */
> +static int ima_max_order;
> 
>  static int __init ima_ahash_setup(char *str)
>  {
> -	int rc;
> +	int ints[3] = { 0, };
> +
> +	str = get_options(str, ARRAY_SIZE(ints), ints);
> +	if (!ints[0])
> +		return 0;
> +
> +	ima_ahash_size = ints[1];
> +	ima_max_order = get_order(ints[2]);

get_options() returns the number of options processed in ints[0].
Before assigning ima_max_order, we normally check to see if it exists.
I guess in this case it doesn't matter since it would return 0 anyway.

Should there be any value checking here?  Should the values be some
multiple of 1024?

> 
> -	rc = kstrtoll(str, 10, &ima_ahash_size);
> +	pr_info("ima_ahash: minsize=%lld, bufsize=%lu\n",
> +		ima_ahash_size, (PAGE_SIZE << ima_max_order));
> 
> -	return !rc;
> +	return 1;
>  }
>  __setup("ima_ahash=", ima_ahash_setup);
> 
> @@ -166,6 +175,65 @@ static int ahash_wait(int err, struct ahash_completion *res)
>  	return err;
>  }
> 
> +/**
> + * ima_alloc_pages() - Allocated contiguous pages.
> + * @max_size:       Maximum amount of memory to allocate.
> + * @allocated_size: Returned size of actual allocation.
> + * @last_warn:      Should the min_size allocation warn or not.
> + *
> + * Tries to do opportunistic allocation for memory first trying to allocate
> + * max_size amount of memory and then splitting that until zero order is
> + * reached. Allocation is tried without generating allocation warnings unless
> + * last_warn is set. Last_warn set affects only last allocation of zero order.
> + *
> + * Return pointer to allocated memory, or NULL on failure.
> + */
> +static void *ima_alloc_pages(loff_t max_size, size_t *allocated_size,
> +			     int last_warn)
> +{
> +	void *ptr;
> +	unsigned int order;
> +	gfp_t gfp_mask = __GFP_NOWARN | __GFP_WAIT | __GFP_NORETRY;
> +
> +	order = min(get_order(max_size), ima_max_order);
> +
> +	for (; order; order--) {
> +		ptr = (void *)__get_free_pages(gfp_mask, order);
> +		if (ptr) {
> +			*allocated_size = PAGE_SIZE << order;
> +			return ptr;
> +		}
> +	}
> +
> +	/* order is zero - one page */
> +
> +	gfp_mask = GFP_KERNEL;
> +
> +	if (!last_warn)
> +		gfp_mask |= __GFP_NOWARN;
> +
> +	ptr = (void *)__get_free_pages(gfp_mask, 0);
> +	if (ptr) {
> +		*allocated_size = PAGE_SIZE;
> +		return ptr;
> +	}
> +
> +	*allocated_size = 0;
> +	return NULL;
> +}
> +
> +/**
> + * ima_free_pages() - Free pages allocated by ima_alloc_pages().
> + * @ptr:  Pointer to allocated pages.
> + * @size: Size of allocated buffer.
> + */
> +static void ima_free_pages(void *ptr, size_t size)
> +{
> +	if (!ptr)
> +		return;
> +	free_pages((unsigned long)ptr, get_order(size));
> +}
> +
>  static int ima_calc_file_hash_atfm(struct file *file,
>  				   struct ima_digest_data *hash,
>  				   struct crypto_ahash *tfm)
> @@ -176,6 +244,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
>  	struct ahash_request *req;
>  	struct scatterlist sg[1];
>  	struct ahash_completion res;
> +	size_t rbuf_size;
> 
>  	hash->length = crypto_ahash_digestsize(tfm);
> 
> @@ -197,7 +266,11 @@ static int ima_calc_file_hash_atfm(struct file *file,
>  	if (i_size == 0)
>  		goto out2;
> 
> -	rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> +	/*
> +	 * Try to allocate maximum size of memory, fail if not even single
> +	 * page cannot be allocated.
> +	 */

The comment is nice explanation, but might be unnecessary if there was a
function comment.

Mimi

> +	rbuf = ima_alloc_pages(i_size, &rbuf_size, 1);
>  	if (!rbuf) {
>  		rc = -ENOMEM;
>  		goto out1;
> @@ -226,7 +299,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
>  	}
>  	if (read)
>  		file->f_mode &= ~FMODE_READ;
> -	kfree(rbuf);
> +	ima_free_pages(rbuf, rbuf_size);
>  out2:
>  	if (!rc) {
>  		ahash_request_set_crypt(req, NULL, hash->digest, 0);

Re: [PATCH v2 2/3] ima: introduce multi-page collect buffers

[Dmitry Kasatkin]

On 2 July 2014 23:21, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Tue, 2014-07-01 at 23:12 +0300, Dmitry Kasatkin wrote:
>> Use of multiple-page collect buffers reduces:
>> 1) the number of block IO requests
>> 2) the number of asynchronous hash update requests
>>
>> Second is important for HW accelerated hashing, because significant
>> amount of time is spent for preparation of hash update operation,
>> which includes configuring acceleration HW, DMA engine, etc...
>> Thus, HW accelerators are more efficient when working on large
>> chunks of data.
>>
>> This patch introduces usage of multi-page collect buffers. Buffer size
>> can be specified by providing additional option to the 'ima_ahash='
>> kernel parameter. 'ima_ahash=2048,16384' specifies that minimal file
>> size to use ahash is 2048 byes and buffer size is 16384 bytes.
>> Default buffer size is 4096 bytes.
>>
>> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
>> ---
>>  Documentation/kernel-parameters.txt |  3 +-
>>  security/integrity/ima/ima_crypto.c | 85 ++++++++++++++++++++++++++++++++++---
>>  2 files changed, 81 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index b406f5c..529ba58 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -1292,9 +1292,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>                       Set number of hash buckets for inode cache.
>>
>>       ima_ahash=      [IMA] Asynchronous hash usage parameters
>> -                     Format: <min_file_size>
>> +                     Format: <min_file_size>[,<bufsize>]
>>                       Set the minimal file size when use asynchronous hash.
>>                       If ima_ahash is not provided, ahash usage is disabled.
>> +                     bufsize - hashing buffer size. 4k if not specified.
>>
>>       ima_appraise=   [IMA] appraise integrity measurements
>>                       Format: { "off" | "enforce" | "fix" }
>> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
>> index 5eb19b4..41f2695 100644
>> --- a/security/integrity/ima/ima_crypto.c
>> +++ b/security/integrity/ima/ima_crypto.c
>> @@ -25,7 +25,6 @@
>>  #include <crypto/hash_info.h>
>>  #include "ima.h"
>>
>> -
>>  struct ahash_completion {
>>       struct completion completion;
>>       int err;
>> @@ -36,14 +35,24 @@ static struct crypto_ahash *ima_ahash_tfm;
>>
>>  /* minimal file size for ahash use */
>>  static loff_t ima_ahash_size;
>> +/* default is 0 - 1 page. */
>> +static int ima_max_order;
>>
>>  static int __init ima_ahash_setup(char *str)
>>  {
>> -     int rc;
>> +     int ints[3] = { 0, };
>> +
>> +     str = get_options(str, ARRAY_SIZE(ints), ints);
>> +     if (!ints[0])
>> +             return 0;
>> +
>> +     ima_ahash_size = ints[1];
>> +     ima_max_order = get_order(ints[2]);
>
> get_options() returns the number of options processed in ints[0].
> Before assigning ima_max_order, we normally check to see if it exists.
> I guess in this case it doesn't matter since it would return 0 anyway.
>
> Should there be any value checking here?  Should the values be some
> multiple of 1024?
>

It is a goo question. I was looking how get_options is used. Here as
ints initialized to 0, get_order returns the same value for anyhing
which is rounded up to PAGE_SIZE. So it works in all cases.

But if we now go to module param or sysctl, then this can be discarded?

Thanks for reviewing.


>>
>> -     rc = kstrtoll(str, 10, &ima_ahash_size);
>> +     pr_info("ima_ahash: minsize=%lld, bufsize=%lu\n",
>> +             ima_ahash_size, (PAGE_SIZE << ima_max_order));
>>
>> -     return !rc;
>> +     return 1;
>>  }
>>  __setup("ima_ahash=", ima_ahash_setup);
>>
>> @@ -166,6 +175,65 @@ static int ahash_wait(int err, struct ahash_completion *res)
>>       return err;
>>  }
>>
>> +/**
>> + * ima_alloc_pages() - Allocated contiguous pages.
>> + * @max_size:       Maximum amount of memory to allocate.
>> + * @allocated_size: Returned size of actual allocation.
>> + * @last_warn:      Should the min_size allocation warn or not.
>> + *
>> + * Tries to do opportunistic allocation for memory first trying to allocate
>> + * max_size amount of memory and then splitting that until zero order is
>> + * reached. Allocation is tried without generating allocation warnings unless
>> + * last_warn is set. Last_warn set affects only last allocation of zero order.
>> + *
>> + * Return pointer to allocated memory, or NULL on failure.
>> + */
>> +static void *ima_alloc_pages(loff_t max_size, size_t *allocated_size,
>> +                          int last_warn)
>> +{
>> +     void *ptr;
>> +     unsigned int order;
>> +     gfp_t gfp_mask = __GFP_NOWARN | __GFP_WAIT | __GFP_NORETRY;
>> +
>> +     order = min(get_order(max_size), ima_max_order);
>> +
>> +     for (; order; order--) {
>> +             ptr = (void *)__get_free_pages(gfp_mask, order);
>> +             if (ptr) {
>> +                     *allocated_size = PAGE_SIZE << order;
>> +                     return ptr;
>> +             }
>> +     }
>> +
>> +     /* order is zero - one page */
>> +
>> +     gfp_mask = GFP_KERNEL;
>> +
>> +     if (!last_warn)
>> +             gfp_mask |= __GFP_NOWARN;
>> +
>> +     ptr = (void *)__get_free_pages(gfp_mask, 0);
>> +     if (ptr) {
>> +             *allocated_size = PAGE_SIZE;
>> +             return ptr;
>> +     }
>> +
>> +     *allocated_size = 0;
>> +     return NULL;
>> +}
>> +
>> +/**
>> + * ima_free_pages() - Free pages allocated by ima_alloc_pages().
>> + * @ptr:  Pointer to allocated pages.
>> + * @size: Size of allocated buffer.
>> + */
>> +static void ima_free_pages(void *ptr, size_t size)
>> +{
>> +     if (!ptr)
>> +             return;
>> +     free_pages((unsigned long)ptr, get_order(size));
>> +}
>> +
>>  static int ima_calc_file_hash_atfm(struct file *file,
>>                                  struct ima_digest_data *hash,
>>                                  struct crypto_ahash *tfm)
>> @@ -176,6 +244,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
>>       struct ahash_request *req;
>>       struct scatterlist sg[1];
>>       struct ahash_completion res;
>> +     size_t rbuf_size;
>>
>>       hash->length = crypto_ahash_digestsize(tfm);
>>
>> @@ -197,7 +266,11 @@ static int ima_calc_file_hash_atfm(struct file *file,
>>       if (i_size == 0)
>>               goto out2;
>>
>> -     rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL);
>> +     /*
>> +      * Try to allocate maximum size of memory, fail if not even single
>> +      * page cannot be allocated.
>> +      */
>
> The comment is nice explanation, but might be unnecessary if there was a
> function comment.
>
> Mimi
>
>> +     rbuf = ima_alloc_pages(i_size, &rbuf_size, 1);
>>       if (!rbuf) {
>>               rc = -ENOMEM;
>>               goto out1;
>> @@ -226,7 +299,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
>>       }
>>       if (read)
>>               file->f_mode &= ~FMODE_READ;
>> -     kfree(rbuf);
>> +     ima_free_pages(rbuf, rbuf_size);
>>  out2:
>>       if (!rc) {
>>               ahash_request_set_crypt(req, NULL, hash->digest, 0);
>
>



-- 
Thanks,
Dmitry

[PATCH v2 3/3] ima: provide double buffering for hash calculation

[Dmitry Kasatkin]

Asynchronous hash API allows initiate hash calculation and perform
other tasks while hash is calculated.

This patch introduces usage of double buffering for simultaneous
hashing and reading of the next chunk of data from the storage.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/ima/ima_crypto.c | 63 +++++++++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 16 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 41f2695..69d55a7 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -239,12 +239,12 @@ static int ima_calc_file_hash_atfm(struct file *file,
 				   struct crypto_ahash *tfm)
 {
 	loff_t i_size, offset;
-	char *rbuf;
-	int rc, read = 0, rbuf_len;
+	char *rbuf[2] = { NULL, };
+	int rc, read = 0, rbuf_len, active = 0, ahash_rc = 0;
 	struct ahash_request *req;
 	struct scatterlist sg[1];
 	struct ahash_completion res;
-	size_t rbuf_size;
+	size_t rbuf_size[2];
 
 	hash->length = crypto_ahash_digestsize(tfm);
 
@@ -270,36 +270,67 @@ static int ima_calc_file_hash_atfm(struct file *file,
 	 * Try to allocate maximum size of memory, fail if not even single
 	 * page cannot be allocated.
 	 */
-	rbuf = ima_alloc_pages(i_size, &rbuf_size, 1);
-	if (!rbuf) {
+	rbuf[0] = ima_alloc_pages(i_size, &rbuf_size[0], 1);
+	if (!rbuf[0]) {
 		rc = -ENOMEM;
 		goto out1;
 	}
 
+	/* Only allocate one buffer if that is enough. */
+	if (i_size > rbuf_size[0]) {
+		/*
+		 * Try to allocate secondary buffer if that fails fallback to
+		 * using single buffering. Use previous memory allocation size
+		 * as baseline for possible allocation size.
+		 */
+		rbuf[1] = ima_alloc_pages(i_size - rbuf_size[0],
+					  &rbuf_size[1], 0);
+	}
+
 	if (!(file->f_mode & FMODE_READ)) {
 		file->f_mode |= FMODE_READ;
 		read = 1;
 	}
 
 	for (offset = 0; offset < i_size; offset += rbuf_len) {
-		rbuf_len = ima_kernel_read(file, offset, rbuf, PAGE_SIZE);
-		if (rbuf_len < 0) {
-			rc = rbuf_len;
-			break;
+		if (!rbuf[1] && offset) {
+			/* if we do not use second buffer and it is not the
+			 * the first read/request, then wait for completion
+			 */
+			rc = ahash_wait(ahash_rc, &res);
+			if (rc)
+				goto out3;
+		}
+		/* read buffer */
+		rbuf_len = min_t(loff_t, i_size - offset, rbuf_size[active]);
+		rc = ima_kernel_read(file, offset, rbuf[active], rbuf_len);
+		if (rc != rbuf_len)
+			goto out3;
+
+		if (rbuf[1] && offset) {
+			/* if we use second buffer and it is not the the first
+			 * read/request, then wait for completion
+			 */
+			rc = ahash_wait(ahash_rc, &res);
+			if (rc)
+				goto out3;
 		}
-		if (rbuf_len == 0)
-			break;
 
-		sg_init_one(&sg[0], rbuf, rbuf_len);
+		sg_init_one(&sg[0], rbuf[active], rbuf_len);
 		ahash_request_set_crypt(req, sg, NULL, rbuf_len);
 
-		rc = ahash_wait(crypto_ahash_update(req), &res);
-		if (rc)
-			break;
+		ahash_rc = crypto_ahash_update(req);
+
+		if (rbuf[1])
+			active = !active; /* swap buffers, if we use two */
 	}
+	/* wait for the last request to complete */
+	rc = ahash_wait(ahash_rc, &res);
+out3:
 	if (read)
 		file->f_mode &= ~FMODE_READ;
-	ima_free_pages(rbuf, rbuf_size);
+	ima_free_pages(rbuf[0], rbuf_size[0]);
+	ima_free_pages(rbuf[1], rbuf_size[1]);
 out2:
 	if (!rc) {
 		ahash_request_set_crypt(req, NULL, hash->digest, 0);
-- 
1.9.1