[PATCH] crypto: caam - fix memleak in caam_jr module
This patch fixes a memory leak that appears when caam_jr module is unloaded. Cc: sta...@vger.kernel.org # 3.13+ Signed-off-by: Cristian Stoica cristian.sto...@freescale.com --- drivers/crypto/caam/jr.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c index 1d80bd3..f127f86 100644 --- a/drivers/crypto/caam/jr.c +++ b/drivers/crypto/caam/jr.c @@ -117,6 +117,7 @@ static int caam_jr_remove(struct platform_device *pdev) if (ret) dev_err(jrdev, Failed to shut down job ring\n); irq_dispose_mapping(jrpriv-irq); + kfree(jrpriv); return ret; } -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto/fips: only panic on bad/missing crypto mod signatures
On Fri, Jun 27, 2014 at 03:12:54PM -0400, Jarod Wilson wrote:
Per further discussion with NIST, the requirements for FIPS state that
we only need to panic the system on failed kernel module signature checks
for crypto subsystem modules. This moves the fips-mode-only module
signature check out of the generic module loading code, into the crypto
subsystem, at points where we can catch both algorithm module loads and
mode module loads. At the same time, make CONFIG_CRYPTO_FIPS dependent on
CONFIG_MODULE_SIG, as this is entirely necessary for FIPS mode.
CC: Herbert Xu herb...@gondor.apana.org.au
CC: David S. Miller da...@davemloft.net
CC: Rusty Russell ru...@rustcorp.com.au
CC: Stephan Mueller stephan.muel...@atsec.com
CC: linux-crypto@vger.kernel.org
Signed-off-by: Jarod Wilson ja...@redhat.com
---
crypto/Kconfig | 1 +
crypto/algapi.c | 13 +
kernel/module.c | 3 ---
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/crypto/Kconfig b/crypto/Kconfig
index ce4012a..36402e5 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -24,6 +24,7 @@ comment Crypto core or helper
config CRYPTO_FIPS
bool FIPS 200 compliance
depends on CRYPTO_ANSI_CPRNG !CRYPTO_MANAGER_DISABLE_TESTS
+ depends on MODULE_SIG
help
This options enables the fips boot option which is
required if you want to system to operate in a FIPS 200
diff --git a/crypto/algapi.c b/crypto/algapi.c
index 7a1ae87..7d228ff 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -43,6 +43,12 @@ static inline int crypto_set_driver_name(struct crypto_alg
*alg)
static int crypto_check_alg(struct crypto_alg *alg)
{
+#ifdef CONFIG_CRYPTO_FIPS
+ if (fips_enabled alg-cra_module !alg-cra_module-sig_ok)
+ panic(Module %s signature verification failed in FIPS mode\n,
+ alg-cra_module-name);
+#endif
Please hide the ugly ifdef in a helper inline function.
@@ -437,6 +449,7 @@ int crypto_register_template(struct crypto_template *tmpl)
list_add(tmpl-list, crypto_template_list);
crypto_notify(CRYPTO_MSG_TMPL_REGISTER, tmpl);
+
err = 0;
out:
up_write(crypto_alg_sem);
While I have no problems with you adding a blank line please don't
mix such additions with changes of substance.
Thanks,
--
Email: Herbert Xu herb...@gondor.apana.org.au
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto/fips: only panic on bad/missing crypto mod signatures
On Wed, Jul 02, 2014 at 08:38:50PM +0800, Herbert Xu wrote:
On Fri, Jun 27, 2014 at 03:12:54PM -0400, Jarod Wilson wrote:
Per further discussion with NIST, the requirements for FIPS state that
we only need to panic the system on failed kernel module signature checks
for crypto subsystem modules. This moves the fips-mode-only module
signature check out of the generic module loading code, into the crypto
subsystem, at points where we can catch both algorithm module loads and
mode module loads. At the same time, make CONFIG_CRYPTO_FIPS dependent on
CONFIG_MODULE_SIG, as this is entirely necessary for FIPS mode.
CC: Herbert Xu herb...@gondor.apana.org.au
CC: David S. Miller da...@davemloft.net
CC: Rusty Russell ru...@rustcorp.com.au
CC: Stephan Mueller stephan.muel...@atsec.com
CC: linux-crypto@vger.kernel.org
Signed-off-by: Jarod Wilson ja...@redhat.com
---
crypto/Kconfig | 1 +
crypto/algapi.c | 13 +
kernel/module.c | 3 ---
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/crypto/Kconfig b/crypto/Kconfig
index ce4012a..36402e5 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -24,6 +24,7 @@ comment Crypto core or helper
config CRYPTO_FIPS
bool FIPS 200 compliance
depends on CRYPTO_ANSI_CPRNG !CRYPTO_MANAGER_DISABLE_TESTS
+ depends on MODULE_SIG
help
This options enables the fips boot option which is
required if you want to system to operate in a FIPS 200
diff --git a/crypto/algapi.c b/crypto/algapi.c
index 7a1ae87..7d228ff 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -43,6 +43,12 @@ static inline int crypto_set_driver_name(struct
crypto_alg *alg)
static int crypto_check_alg(struct crypto_alg *alg)
{
+#ifdef CONFIG_CRYPTO_FIPS
+ if (fips_enabled alg-cra_module !alg-cra_module-sig_ok)
+ panic(Module %s signature verification failed in FIPS mode\n,
+ alg-cra_module-name);
+#endif
Please hide the ugly ifdef in a helper inline function.
Will do.
@@ -437,6 +449,7 @@ int crypto_register_template(struct crypto_template
*tmpl)
list_add(tmpl-list, crypto_template_list);
crypto_notify(CRYPTO_MSG_TMPL_REGISTER, tmpl);
+
err = 0;
out:
up_write(crypto_alg_sem);
While I have no problems with you adding a blank line please don't
mix such additions with changes of substance.
Accidental, sorry, was sloppy when moving the check from one place in the
function to another. Will remedy that too. v2 coming after some quick
testing.
--
Jarod Wilson
ja...@redhat.com
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@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
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.kasat...@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];
+
Re: [PATCH v2 1/3] ima: use ahash API for file hash calculation
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);
+}
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@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
On 2 July 2014 19:40, Mimi Zohar zo...@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.kasat...@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);
+ }
+
Re: [PATCH v2 1/3] ima: use ahash API for file hash calculation
On 2 July 2014 20:44, Mimi Zohar zo...@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
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@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
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. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@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
On 2 July 2014 21:33, Dave Hansen dave.han...@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 -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@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
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. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@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
On Wed, 2014-07-02 at 21:20 +0300, Dmitry Kasatkin wrote:
On 2 July 2014 19:40, Mimi Zohar zo...@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.kasat...@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:
+
Re: [PATCH v2 1/3] ima: use ahash API for file hash calculation
On Wed, 2014-07-02 at 21:21 +0300, Dmitry Kasatkin wrote:
On 2 July 2014 20:44, Mimi Zohar zo...@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
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] crypto/fips: only panic on bad/missing crypto mod signatures
Per further discussion with NIST, the requirements for FIPS state that
we only need to panic the system on failed kernel module signature checks
for crypto subsystem modules. This moves the fips-mode-only module
signature check out of the generic module loading code, into the crypto
subsystem, at points where we can catch both algorithm module loads and
mode module loads. At the same time, make CONFIG_CRYPTO_FIPS dependent on
CONFIG_MODULE_SIG, as this is entirely necessary for FIPS mode.
v2: remove extraneous blank line, perform checks in static inline
function, drop no longer necessary fips.h include.
CC: Herbert Xu herb...@gondor.apana.org.au
CC: David S. Miller da...@davemloft.net
CC: Rusty Russell ru...@rustcorp.com.au
CC: Stephan Mueller stephan.muel...@atsec.com
CC: linux-crypto@vger.kernel.org
Signed-off-by: Jarod Wilson ja...@redhat.com
---
crypto/Kconfig | 1 +
crypto/algapi.c | 14 ++
kernel/module.c | 4
3 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/crypto/Kconfig b/crypto/Kconfig
index ce4012a..36402e5 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -24,6 +24,7 @@ comment Crypto core or helper
config CRYPTO_FIPS
bool FIPS 200 compliance
depends on CRYPTO_ANSI_CPRNG !CRYPTO_MANAGER_DISABLE_TESTS
+ depends on MODULE_SIG
help
This options enables the fips boot option which is
required if you want to system to operate in a FIPS 200
diff --git a/crypto/algapi.c b/crypto/algapi.c
index 7a1ae87..e8d3a7d 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -41,8 +41,20 @@ static inline int crypto_set_driver_name(struct crypto_alg
*alg)
return 0;
}
+static inline void crypto_check_module_sig(struct module *mod)
+{
+#ifdef CONFIG_CRYPTO_FIPS
+ if (fips_enabled mod !mod-sig_ok)
+ panic(Module %s signature verification failed in FIPS mode\n,
+ mod-name);
+#endif
+ return;
+}
+
static int crypto_check_alg(struct crypto_alg *alg)
{
+ crypto_check_module_sig(alg-cra_module);
+
if (alg-cra_alignmask (alg-cra_alignmask + 1))
return -EINVAL;
@@ -430,6 +442,8 @@ int crypto_register_template(struct crypto_template *tmpl)
down_write(crypto_alg_sem);
+ crypto_check_module_sig(tmpl-module);
+
list_for_each_entry(q, crypto_template_list, list) {
if (q == tmpl)
goto out;
diff --git a/kernel/module.c b/kernel/module.c
index 81e727c..ae79ce6 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -60,7 +60,6 @@
#include linux/jump_label.h
#include linux/pfn.h
#include linux/bsearch.h
-#include linux/fips.h
#include uapi/linux/module.h
#include module-internal.h
@@ -2448,9 +2447,6 @@ static int module_sig_check(struct load_info *info)
}
/* Not having a signature is only an error if we're strict. */
- if (err 0 fips_enabled)
- panic(Module verification failed with error %d in FIPS mode\n,
- err);
if (err == -ENOKEY !sig_enforce)
err = 0;
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] ima: introduce multi-page collect buffers
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.kasat...@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
Re: [PATCH v2 2/3] ima: introduce multi-page collect buffers
On 2 July 2014 23:21, Mimi Zohar zo...@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.kasat...@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 =
Re: Testing the PRNG driver of the Allwinner Security System A20
On Tue, Jul 1, 2014 at 7:14 AM, Corentin LABBE clabbe.montj...@gmail.com wrote: I am writing the PRNG driver for the Allwinner Security System SoC A20. The datasheet my search turned up (v1, Feb. 2013) just says: 160-bit hardware PRNG with 192-bit seed and gives no other details. Do you have more info, perhaps from a more recent version or talking to the company? I didn't know how to test it, so ... Unless you have much more info, I see no point in enabling it or writing a driver. You need a true hardware RNG to seed it, so you need random(4) /dev/random anyway and can just use /dev/urandom for PRNG requirements. Using this device might have an advantage if it is much faster or less resource-hungry than urandom, but I see nothing in its documentation that indicates it is. Anyway, do your applications need that? And, if so, would an application-specific PRNG be better yet? Then there is the crucial question of trusting the device. Kerckhoff's Principle (http://en.citizendium.org/wiki/Kerckhoffs%27_Principle) has been a maxim for cryptographers since the 19th century; no-one should even consider trusting it until full design details are made public and reviewed. Even then, there might be serious doubts, since hardware can be very subtly sabotaged and an RNG is a tempting target for an intelligence agency. (http://arstechnica.com/security/2013/09/researchers-can-slip-an-undetectable-trojan-into-intels-ivy-bridge-cpus/) That article discusses Intel and the NSA, but similar worries apply elsewhere. Allwinner is a fabless company, so you also need to worry about whatever fab they use. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
