Re: [PATCH] crypto: prefix module autoloading with crypto-

2014-11-17 Thread Herbert Xu
On Fri, Nov 14, 2014 at 01:34:59PM -0800, Kees Cook wrote:
 This prefixes all crypto module loading with crypto- so we never run
 the risk of exposing module auto-loading to userspace via a crypto API,
 as demonstrated by Mathias Krause:
 
 https://lkml.org/lkml/2013/3/4/70
 
 Signed-off-by: Kees Cook keesc...@chromium.org

Sorry but this doesn't build for me:

  CC [M]  drivers/crypto/qat/qat_common/adf_ctl_drv.o
drivers/crypto/qat/qat_common/adf_ctl_drv.c:490:21: error: expected declaration 
specifiers or ‘...’ before string constant
make[3]: *** [drivers/crypto/qat/qat_common/adf_ctl_drv.o] Error 1
make[2]: *** [drivers/crypto/qat/qat_common] Error 2
make[1]: *** [drivers/crypto/qat] Error 2
make[1]: *** Waiting for unfinished jobs

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: Documentation - document uncovered member variables

2014-11-17 Thread Herbert Xu
On Fri, Nov 14, 2014 at 05:26:21AM +0100, Stephan Mueller wrote:
 Fix documentation typo for shash_alg-descsize.
 
 Add documentation for initially uncovered member variables.
 
 Signed-off-by: Stephan Mueller smuel...@chronox.de

Patch applied.
-- 
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: qat - Use memzero_explicit

2014-11-17 Thread Herbert Xu
On Fri, Nov 14, 2014 at 11:23:52AM -0800, Tadeusz Struk wrote:
 Use the new memzero_explicit function to cleanup sensitive data.
 
 Signed-off-by: Tadeusz Struk tadeusz.st...@intel.com

Patch applied.
-- 
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


[PATCH 2/2] kernel/kexec: free crypto_shash using crypto_free_shash

2014-11-17 Thread Konstantin Khlebnikov
These objects have special freeing functions which cares about
proper destruction and reference counting.

Signed-off-by: Konstantin Khlebnikov k.khlebni...@samsung.com
---
 kernel/kexec.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/kexec.c b/kernel/kexec.c
index 2abf9f6..5a62311 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -2286,7 +2286,7 @@ out_free_sha_regions:
 out_free_desc:
kfree(desc);
 out_free_tfm:
-   kfree(tfm);
+   crypto_free_shash(tfm);
 out:
return ret;
 }

--
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 1/2] scripts/coccinelle: catch freeing cryptographic structures via kfree

2014-11-17 Thread Konstantin Khlebnikov
Structures allocated by crypto_alloc_* must be freed using crypto_free_*.

Signed-off-by: Konstantin Khlebnikov k.khlebni...@samsung.com
---
 scripts/coccinelle/free/crypto_free.cocci |   45 +
 1 file changed, 45 insertions(+)
 create mode 100644 scripts/coccinelle/free/crypto_free.cocci

diff --git a/scripts/coccinelle/free/crypto_free.cocci 
b/scripts/coccinelle/free/crypto_free.cocci
new file mode 100644
index 000..0799b70
--- /dev/null
+++ b/scripts/coccinelle/free/crypto_free.cocci
@@ -0,0 +1,45 @@
+///
+/// Structures allocated by crypto_alloc_* must be freed using crypto_free_*.
+/// This finds freeing them by kfree.
+///
+// Confidence: Moderate
+// Copyright: (C) 2014 Konstantin Khlebnikov,  GPLv2.
+// Comments: There are false positives in crypto/ where they are actually 
freed.
+// Keywords: crypto, kfree
+// Options: --no-includes --include-headers
+
+virtual org
+virtual report
+virtual context
+
+@r depends on context || org || report@
+expression x;
+identifier crypto_alloc =~ ^crypto_alloc_;
+@@
+
+(
+ x = crypto_alloc(...)
+)
+
+@pb@
+expression r.x;
+position p;
+@@
+
+(
+* kfree@p(x)
+)
+
+@script:python depends on org@
+p  pb.p;
+@@
+
+msg=WARNING: invalid free of crypto_alloc_* allocated data
+coccilib.org.print_todo(p[0], msg)
+
+@script:python depends on report@
+p  pb.p;
+@@
+
+msg=WARNING: invalid free of crypto_alloc_* allocated data
+coccilib.report.print_report(p[0], msg)

--
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 1/2] scripts/coccinelle: catch freeing cryptographic structures via kfree

2014-11-17 Thread Julia Lawall


On Mon, 17 Nov 2014, Konstantin Khlebnikov wrote:

 Structures allocated by crypto_alloc_* must be freed using crypto_free_*.

 Signed-off-by: Konstantin Khlebnikov k.khlebni...@samsung.com
 ---
  scripts/coccinelle/free/crypto_free.cocci |   45 
 +
  1 file changed, 45 insertions(+)
  create mode 100644 scripts/coccinelle/free/crypto_free.cocci

 diff --git a/scripts/coccinelle/free/crypto_free.cocci 
 b/scripts/coccinelle/free/crypto_free.cocci
 new file mode 100644
 index 000..0799b70
 --- /dev/null
 +++ b/scripts/coccinelle/free/crypto_free.cocci
 @@ -0,0 +1,45 @@
 +///
 +/// Structures allocated by crypto_alloc_* must be freed using crypto_free_*.
 +/// This finds freeing them by kfree.
 +///
 +// Confidence: Moderate
 +// Copyright: (C) 2014 Konstantin Khlebnikov,  GPLv2.
 +// Comments: There are false positives in crypto/ where they are actually 
 freed.
 +// Keywords: crypto, kfree
 +// Options: --no-includes --include-headers
 +
 +virtual org
 +virtual report
 +virtual context
 +
 +@r depends on context || org || report@
 +expression x;
 +identifier crypto_alloc =~ ^crypto_alloc_;
 +@@
 +
 +(
 + x = crypto_alloc(...)
 +)

You can drop the outer parentheses, in this case and in the kfree case.

Are there many of these crypto_alloc_ functions?  It would be nicer to
avoid the regular expression.  For one thing, you don't have much control
over what it matches, and for another thing Coccinelle will not be able to
optimize the selection of files.  With the regular expression it will have
to parse every file and analyze every function, which will be slow.

julia

 +
 +@pb@
 +expression r.x;
 +position p;
 +@@
 +
 +(
 +* kfree@p(x)
 +)
 +
 +@script:python depends on org@
 +p  pb.p;
 +@@
 +
 +msg=WARNING: invalid free of crypto_alloc_* allocated data
 +coccilib.org.print_todo(p[0], msg)
 +
 +@script:python depends on report@
 +p  pb.p;
 +@@
 +
 +msg=WARNING: invalid free of crypto_alloc_* allocated data
 +coccilib.report.print_report(p[0], msg)


--
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 1/2] scripts/coccinelle: catch freeing cryptographic structures via kfree

2014-11-17 Thread Konstantin Khlebnikov


On 2014-11-17 18:30, Julia Lawall wrote:


On Mon, 17 Nov 2014, Konstantin Khlebnikov wrote:


Structures allocated by crypto_alloc_* must be freed using crypto_free_*.

Signed-off-by: Konstantin Khlebnikov k.khlebni...@samsung.com
---
  scripts/coccinelle/free/crypto_free.cocci |   45 +
  1 file changed, 45 insertions(+)
  create mode 100644 scripts/coccinelle/free/crypto_free.cocci

diff --git a/scripts/coccinelle/free/crypto_free.cocci 
b/scripts/coccinelle/free/crypto_free.cocci
new file mode 100644
index 000..0799b70
--- /dev/null
+++ b/scripts/coccinelle/free/crypto_free.cocci
@@ -0,0 +1,45 @@
+///
+/// Structures allocated by crypto_alloc_* must be freed using crypto_free_*.
+/// This finds freeing them by kfree.
+///
+// Confidence: Moderate
+// Copyright: (C) 2014 Konstantin Khlebnikov,  GPLv2.
+// Comments: There are false positives in crypto/ where they are actually 
freed.
+// Keywords: crypto, kfree
+// Options: --no-includes --include-headers
+
+virtual org
+virtual report
+virtual context
+
+@r depends on context || org || report@
+expression x;
+identifier crypto_alloc =~ ^crypto_alloc_;
+@@
+
+(
+ x = crypto_alloc(...)
+)

You can drop the outer parentheses, in this case and in the kfree case.

Are there many of these crypto_alloc_ functions?  It would be nicer to
avoid the regular expression.  For one thing, you don't have much control
over what it matches, and for another thing Coccinelle will not be able to
optimize the selection of files.  With the regular expression it will have
to parse every file and analyze every function, which will be slow.


As I see here is eight .. ten candidates, maybe some of them are internal.
Ok, I'll resend patch without regex.



julia


+
+@pb@
+expression r.x;
+position p;
+@@
+
+(
+* kfree@p(x)
+)
+
+@script:python depends on org@
+p  pb.p;
+@@
+
+msg=WARNING: invalid free of crypto_alloc_* allocated data
+coccilib.org.print_todo(p[0], msg)
+
+@script:python depends on report@
+p  pb.p;
+@@
+
+msg=WARNING: invalid free of crypto_alloc_* allocated data
+coccilib.report.print_report(p[0], msg)




--
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 0/2] crypto: qat - Fix for invalid dma mapping and numa

2014-11-17 Thread Tadeusz Struk
On 10/24/2014 07:45 AM, Herbert Xu wrote:
 On Wed, Oct 15, 2014 at 07:25:45AM -0400, Prarit Bhargava wrote:


 On 10/15/2014 06:35 AM, Nikolay Aleksandrov wrote:
 On 14/10/14 03:24, Tadeusz Struk wrote:
 Hi,
 These two patches fix invalid (zero length) dma mapping and
 enforce numa configuration for maximum performance.

 Change log:
 v2 - Removed numa node calculation based bus number and use predefined
 functions instead.

 Signed-off-by: Tadeusz Struk tadeusz.st...@intel.com
 ---

 Tadeusz Struk (2):
crypto: qat - Prevent dma mapping zero length assoc data
crypto: qat - Enforce valid numa configuration


   drivers/crypto/qat/qat_common/adf_accel_devices.h |3 +-
   drivers/crypto/qat/qat_common/adf_transport.c |   12 +---
   drivers/crypto/qat/qat_common/qat_algs.c  |7 +++--
   drivers/crypto/qat/qat_common/qat_crypto.c|8 +++--
   drivers/crypto/qat/qat_dh895xcc/adf_admin.c   |2 +
   drivers/crypto/qat/qat_dh895xcc/adf_drv.c |   32 
 -
   drivers/crypto/qat/qat_dh895xcc/adf_isr.c |2 +
   7 files changed, 32 insertions(+), 34 deletions(-)


 I just gave a quick run of these patches and they seem to fix the NUMA 
 issue and
 the 0 length warnings.

 Tested-by: Nikolay Aleksandrov niko...@redhat.com

 Thanks Nik :)

 Reviewed-by: Prarit Bhargava pra...@redhat.com
 
 Patch applied to crypto and I will push this to stable.

Hi Greg,
Any idea why this didn't make it to 3.17.3?
Thanks,
Tadeusz

--
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 0/2] crypto: qat - Fix for invalid dma mapping and numa

2014-11-17 Thread Greg KH
On Mon, Nov 17, 2014 at 07:43:08AM -0800, Tadeusz Struk wrote:
 On 10/24/2014 07:45 AM, Herbert Xu wrote:
  On Wed, Oct 15, 2014 at 07:25:45AM -0400, Prarit Bhargava wrote:
 
 
  On 10/15/2014 06:35 AM, Nikolay Aleksandrov wrote:
  On 14/10/14 03:24, Tadeusz Struk wrote:
  Hi,
  These two patches fix invalid (zero length) dma mapping and
  enforce numa configuration for maximum performance.
 
  Change log:
  v2 - Removed numa node calculation based bus number and use predefined
  functions instead.
 
  Signed-off-by: Tadeusz Struk tadeusz.st...@intel.com
  ---
 
  Tadeusz Struk (2):
 crypto: qat - Prevent dma mapping zero length assoc data
 crypto: qat - Enforce valid numa configuration
 
 
drivers/crypto/qat/qat_common/adf_accel_devices.h |3 +-
drivers/crypto/qat/qat_common/adf_transport.c |   12 +---
drivers/crypto/qat/qat_common/qat_algs.c  |7 +++--
drivers/crypto/qat/qat_common/qat_crypto.c|8 +++--
drivers/crypto/qat/qat_dh895xcc/adf_admin.c   |2 +
drivers/crypto/qat/qat_dh895xcc/adf_drv.c |   32 
  -
drivers/crypto/qat/qat_dh895xcc/adf_isr.c |2 +
7 files changed, 32 insertions(+), 34 deletions(-)
 
 
  I just gave a quick run of these patches and they seem to fix the NUMA 
  issue and
  the 0 length warnings.
 
  Tested-by: Nikolay Aleksandrov niko...@redhat.com
 
  Thanks Nik :)
 
  Reviewed-by: Prarit Bhargava pra...@redhat.com
  
  Patch applied to crypto and I will push this to stable.
 
 Hi Greg,
 Any idea why this didn't make it to 3.17.3?

Because it showed up in Linus's tree _after_ 3.17.3-rc1 was released?
How can I go back in time?

confused,

greg k-h
--
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] its == something belong to it. it's == it is, it has, it was, etc. Sorry - just bugged me as I was reading the code.

2014-11-17 Thread Terence Eden
From: edent github@shkspr.mobi

Signed-off-by: edent github@shkspr.mobi
---
 crypto/ahash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/ahash.c b/crypto/ahash.c
index f6a36a5..ffbcda3 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -271,7 +271,7 @@ static int ahash_save_req(struct ahash_request *req, 
crypto_completion_t cplt)
/*
 * WARNING: We do not backup req-priv here! The req-priv
 *  is for internal use of the Crypto API and the
-*  user must _NOT_ _EVER_ depend on it's content!
+*  user must _NOT_ _EVER_ depend on its content!
 */
 
req-result = PTR_ALIGN((u8 *)priv-ubuf, alignmask + 1);
-- 
1.9.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 1/2] crypto: Add Imagination Technologies hw hash accelerator

2014-11-17 Thread Andrew Bresticker
On Fri, Nov 14, 2014 at 11:55 PM, Corentin LABBE
clabbe.montj...@gmail.com wrote:
 Le 15/11/2014 00:59, Andrew Bresticker a écrit :
 Hi James,

 +
 +struct img_hash_drv {
 +   struct list_head dev_list;
 +   spinlock_t lock;
 +};
 +
 +static struct img_hash_drv img_hash = {
 +   .dev_list = LIST_HEAD_INIT(img_hash.dev_list),
 +   .lock = __SPIN_LOCK_UNLOCKED(img_hash.lock),
 +};

 It looks like the only purpose of this list is to get the
 corresponding struct img_hash_dev in img_hash_init().  If there's
 never going to be multiple instances within an SoC, perhaps you could
 just use a global?  Otherwise, you could do something like the
 qualcomm driver, see drivers/crypto/qce/sha.c.  It looks like there is
 some precedent for this device list though...


 I don't understand, you propose to use a global, something that lots of 
 people want to be removed in my driver.
 It is not better than this global list.

 I have the fealing that there no good way to get a pointer to a driver 
 structure inside the cryptoAPI.
 What to you think about adding a void *data in struct crypto_alg

 Before registering an alg you could do:
 mv_aes_alg_ecb.data = myprivatedriverdata;
 ret = crypto_register_alg(mv_aes_alg_ecb);

 and then get it via
 struct crypto_priv *cp = req-base.tfm-__crt_alg-data;
 (a function will be better than that)

That sounds good to me.
--
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 0/2] crypto: qat - Fix for invalid dma mapping and numa

2014-11-17 Thread Tadeusz Struk
On 11/17/2014 08:59 AM, Greg KH wrote:
 Because it showed up in Linus's tree _after_ 3.17.3-rc1 was released?
 How can I go back in time?

I thought it was already possible, no? :)
So will it be in 3.17.4?
Thanks,
Tadeusz


--
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] its == something belong to it. it's == it is, it has, it was, etc. Sorry - just bugged me as I was reading the code.

2014-11-17 Thread Corentin LABBE
Le 17/11/2014 18:06, Terence Eden a écrit :
 From: edent github@shkspr.mobi
 
 Signed-off-by: edent github@shkspr.mobi
 ---
  crypto/ahash.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/crypto/ahash.c b/crypto/ahash.c
 index f6a36a5..ffbcda3 100644
 --- a/crypto/ahash.c
 +++ b/crypto/ahash.c
 @@ -271,7 +271,7 @@ static int ahash_save_req(struct ahash_request *req, 
 crypto_completion_t cplt)
   /*
* WARNING: We do not backup req-priv here! The req-priv
*  is for internal use of the Crypto API and the
 -  *  user must _NOT_ _EVER_ depend on it's content!
 +  *  user must _NOT_ _EVER_ depend on its content!
*/
  
   req-result = PTR_ALIGN((u8 *)priv-ubuf, alignmask + 1);
 

Hello

Please sign off with your real name.
Your subject line is wrong, you need to add the subsystem crypto: and the 
subject must be simplier; but you could keep the actual subject as a commit 
message

example
[PATCH] crypto: Fix a typo in ahash.c

its == something belong to it. it's == it is, it has, it was, etc. 
Sorry - just bugged me as I was reading the code.
/example

Regards

LABBE Corentin

--
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 0/2] crypto: qat - Fix for invalid dma mapping and numa

2014-11-17 Thread Greg KH
On Mon, Nov 17, 2014 at 09:20:34AM -0800, Tadeusz Struk wrote:
 On 11/17/2014 08:59 AM, Greg KH wrote:
  Because it showed up in Linus's tree _after_ 3.17.3-rc1 was released?
  How can I go back in time?
 
 I thought it was already possible, no? :)
 So will it be in 3.17.4?

Possibly, if I get around to it, relax please...

greg burried in patches k-h
--
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: prefix module autoloading with crypto-

2014-11-17 Thread Mathias Krause
On 17 November 2014 16:09, Herbert Xu herb...@gondor.apana.org.au wrote:
 On Fri, Nov 14, 2014 at 01:34:59PM -0800, Kees Cook wrote:
 This prefixes all crypto module loading with crypto- so we never run
 the risk of exposing module auto-loading to userspace via a crypto API,
 as demonstrated by Mathias Krause:

 https://lkml.org/lkml/2013/3/4/70

 Signed-off-by: Kees Cook keesc...@chromium.org

 Sorry but this doesn't build for me:

   CC [M]  drivers/crypto/qat/qat_common/adf_ctl_drv.o
 drivers/crypto/qat/qat_common/adf_ctl_drv.c:490:21: error: expected 
 declaration specifiers or ‘...’ before string constant
 make[3]: *** [drivers/crypto/qat/qat_common/adf_ctl_drv.o] Error 1
 make[2]: *** [drivers/crypto/qat/qat_common] Error 2
 make[1]: *** [drivers/crypto/qat] Error 2
 make[1]: *** Waiting for unfinished jobs

Looks like drivers/crypto/qat/qat_common/adf_ctl_drv.c is missing
'#include linux/crypto.h'.

Mathias
--
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] crypto: Fix a typo in ahash.c

2014-11-17 Thread Terence Eden
From: Terence Eden terence.e...@gmail.com

its == something belong to it. it's == it is, it has, it
was, etc. Sorry - just bugged me as I was reading the code.
(Resubmitting - hopefully correctly!)

Signed-off-by: Terence Eden terence.e...@gmail.com
---
 crypto/ahash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/ahash.c b/crypto/ahash.c
index f6a36a5..ffbcda3 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -271,7 +271,7 @@ static int ahash_save_req(struct ahash_request
*req, crypto_completion_t cplt)
 /*
  * WARNING: We do not backup req-priv here! The req-priv
  *  is for internal use of the Crypto API and the
- *  user must _NOT_ _EVER_ depend on it's content!
+ *  user must _NOT_ _EVER_ depend on its content!
  */

 req-result = PTR_ALIGN((u8 *)priv-ubuf, alignmask + 1);
-- 
1.9.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: prefix module autoloading with crypto-

2014-11-17 Thread Kees Cook
On Mon, Nov 17, 2014 at 10:38 AM, Mathias Krause mini...@googlemail.com wrote:
 On 17 November 2014 16:09, Herbert Xu herb...@gondor.apana.org.au wrote:
 On Fri, Nov 14, 2014 at 01:34:59PM -0800, Kees Cook wrote:
 This prefixes all crypto module loading with crypto- so we never run
 the risk of exposing module auto-loading to userspace via a crypto API,
 as demonstrated by Mathias Krause:

 https://lkml.org/lkml/2013/3/4/70

 Signed-off-by: Kees Cook keesc...@chromium.org

 Sorry but this doesn't build for me:

   CC [M]  drivers/crypto/qat/qat_common/adf_ctl_drv.o
 drivers/crypto/qat/qat_common/adf_ctl_drv.c:490:21: error: expected 
 declaration specifiers or ‘...’ before string constant
 make[3]: *** [drivers/crypto/qat/qat_common/adf_ctl_drv.o] Error 1
 make[2]: *** [drivers/crypto/qat/qat_common] Error 2
 make[1]: *** [drivers/crypto/qat] Error 2
 make[1]: *** Waiting for unfinished jobs

 Looks like drivers/crypto/qat/qat_common/adf_ctl_drv.c is missing
 '#include linux/crypto.h'.

Ah, looks like I missed a few in my config. I'll attempt allmodconfig
and send a v2.

Thanks!

-Kees

-- 
Kees Cook
Chrome OS Security
--
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: prefix module autoloading with crypto-

2014-11-17 Thread Kees Cook
This prefixes all crypto module loading with crypto- so we never run
the risk of exposing module auto-loading to userspace via a crypto API,
as demonstrated by Mathias Krause:

https://lkml.org/lkml/2013/3/4/70

Signed-off-by: Kees Cook keesc...@chromium.org
---
v2:
 - added missing #include, thanks to minipli
 - built with allmodconfig
---
 arch/arm/crypto/aes_glue.c  | 4 ++--
 arch/arm/crypto/sha1_glue.c | 2 +-
 arch/arm/crypto/sha1_neon_glue.c| 2 +-
 arch/arm/crypto/sha512_neon_glue.c  | 4 ++--
 arch/arm64/crypto/aes-ce-ccm-glue.c | 2 +-
 arch/arm64/crypto/aes-glue.c| 8 
 arch/powerpc/crypto/sha1.c  | 2 +-
 arch/s390/crypto/aes_s390.c | 2 +-
 arch/s390/crypto/des_s390.c | 4 ++--
 arch/s390/crypto/ghash_s390.c   | 2 +-
 arch/s390/crypto/sha1_s390.c| 2 +-
 arch/s390/crypto/sha256_s390.c  | 4 ++--
 arch/s390/crypto/sha512_s390.c  | 4 ++--
 arch/sparc/crypto/aes_glue.c| 2 +-
 arch/sparc/crypto/camellia_glue.c   | 2 +-
 arch/sparc/crypto/crc32c_glue.c | 2 +-
 arch/sparc/crypto/des_glue.c| 2 +-
 arch/sparc/crypto/md5_glue.c| 2 +-
 arch/sparc/crypto/sha1_glue.c   | 2 +-
 arch/sparc/crypto/sha256_glue.c | 4 ++--
 arch/sparc/crypto/sha512_glue.c | 4 ++--
 arch/x86/crypto/aes_glue.c  | 4 ++--
 arch/x86/crypto/aesni-intel_glue.c  | 2 +-
 arch/x86/crypto/blowfish_glue.c | 4 ++--
 arch/x86/crypto/camellia_aesni_avx2_glue.c  | 4 ++--
 arch/x86/crypto/camellia_aesni_avx_glue.c   | 4 ++--
 arch/x86/crypto/camellia_glue.c | 4 ++--
 arch/x86/crypto/cast5_avx_glue.c| 2 +-
 arch/x86/crypto/cast6_avx_glue.c| 2 +-
 arch/x86/crypto/crc32-pclmul_glue.c | 4 ++--
 arch/x86/crypto/crc32c-intel_glue.c | 4 ++--
 arch/x86/crypto/crct10dif-pclmul_glue.c | 4 ++--
 arch/x86/crypto/des3_ede_glue.c | 8 
 arch/x86/crypto/ghash-clmulni-intel_glue.c  | 2 +-
 arch/x86/crypto/salsa20_glue.c  | 4 ++--
 arch/x86/crypto/serpent_avx2_glue.c | 4 ++--
 arch/x86/crypto/serpent_avx_glue.c  | 2 +-
 arch/x86/crypto/serpent_sse2_glue.c | 2 +-
 arch/x86/crypto/sha1_ssse3_glue.c   | 2 +-
 arch/x86/crypto/sha256_ssse3_glue.c | 4 ++--
 arch/x86/crypto/sha512_ssse3_glue.c | 4 ++--
 arch/x86/crypto/twofish_avx_glue.c  | 2 +-
 arch/x86/crypto/twofish_glue.c  | 4 ++--
 arch/x86/crypto/twofish_glue_3way.c | 4 ++--
 crypto/842.c| 1 +
 crypto/aes_generic.c| 2 +-
 crypto/ansi_cprng.c | 2 +-
 crypto/anubis.c | 1 +
 crypto/api.c| 4 ++--
 crypto/arc4.c   | 1 +
 crypto/blowfish_generic.c   | 2 +-
 crypto/camellia_generic.c   | 2 +-
 crypto/cast5_generic.c  | 2 +-
 crypto/cast6_generic.c  | 2 +-
 crypto/ccm.c| 4 ++--
 crypto/crc32.c  | 1 +
 crypto/crc32c_generic.c | 2 +-
 crypto/crct10dif_generic.c  | 2 +-
 crypto/crypto_null.c| 6 +++---
 crypto/ctr.c| 2 +-
 crypto/deflate.c| 2 +-
 crypto/des_generic.c| 2 +-
 crypto/fcrypt.c | 1 +
 crypto/gcm.c| 6 +++---
 crypto/ghash-generic.c  | 2 +-
 crypto/khazad.c | 1 +
 crypto/krng.c   | 2 +-
 crypto/lz4.c| 1 +
 crypto/lz4hc.c  | 1 +
 crypto/lzo.c| 1 +
 crypto/md4.c| 2 +-
 crypto/md5.c| 1 +
 crypto/michael_mic.c| 1 +
 crypto/rmd128.c | 1 +
 crypto/rmd160.c | 1 +
 crypto/rmd256.c | 1 +
 crypto/rmd320.c | 1 +
 crypto/salsa20_generic.c| 2 +-
 crypto/seed.c   | 1 +
 crypto/serpent_generic.c| 4 ++--
 crypto/sha1_generic.c   | 2 +-
 crypto/sha256_generic.c | 4 ++--
 crypto/sha512_generic.c | 4 ++--
 crypto/tea.c| 4 ++--
 crypto/tgr192.c | 4 ++--
 crypto/twofish_generic.c| 2 +-
 crypto/wp512.c  | 4 ++--
 crypto/zlib.c   | 1 +
 

Re: [PATCH v2] crypto: prefix module autoloading with crypto-

2014-11-17 Thread Mathias Krause
On 17 November 2014 21:02, Kees Cook keesc...@chromium.org wrote:
 This prefixes all crypto module loading with crypto- so we never run
 the risk of exposing module auto-loading to userspace via a crypto API,
 as demonstrated by Mathias Krause:

 https://lkml.org/lkml/2013/3/4/70

 Signed-off-by: Kees Cook keesc...@chromium.org
 ---
 v2:
  - added missing #include, thanks to minipli
  - built with allmodconfig
 ---
  arch/arm/crypto/aes_glue.c  | 4 ++--
  arch/arm/crypto/sha1_glue.c | 2 +-
  arch/arm/crypto/sha1_neon_glue.c| 2 +-
  arch/arm/crypto/sha512_neon_glue.c  | 4 ++--
  arch/arm64/crypto/aes-ce-ccm-glue.c | 2 +-
  arch/arm64/crypto/aes-glue.c| 8 
  arch/powerpc/crypto/sha1.c  | 2 +-
  arch/s390/crypto/aes_s390.c | 2 +-
  arch/s390/crypto/des_s390.c | 4 ++--
  arch/s390/crypto/ghash_s390.c   | 2 +-
  arch/s390/crypto/sha1_s390.c| 2 +-
  arch/s390/crypto/sha256_s390.c  | 4 ++--
  arch/s390/crypto/sha512_s390.c  | 4 ++--
  arch/sparc/crypto/aes_glue.c| 2 +-
  arch/sparc/crypto/camellia_glue.c   | 2 +-
  arch/sparc/crypto/crc32c_glue.c | 2 +-
  arch/sparc/crypto/des_glue.c| 2 +-
  arch/sparc/crypto/md5_glue.c| 2 +-
  arch/sparc/crypto/sha1_glue.c   | 2 +-
  arch/sparc/crypto/sha256_glue.c | 4 ++--
  arch/sparc/crypto/sha512_glue.c | 4 ++--
  arch/x86/crypto/aes_glue.c  | 4 ++--
  arch/x86/crypto/aesni-intel_glue.c  | 2 +-
  arch/x86/crypto/blowfish_glue.c | 4 ++--
  arch/x86/crypto/camellia_aesni_avx2_glue.c  | 4 ++--
  arch/x86/crypto/camellia_aesni_avx_glue.c   | 4 ++--
  arch/x86/crypto/camellia_glue.c | 4 ++--
  arch/x86/crypto/cast5_avx_glue.c| 2 +-
  arch/x86/crypto/cast6_avx_glue.c| 2 +-
  arch/x86/crypto/crc32-pclmul_glue.c | 4 ++--
  arch/x86/crypto/crc32c-intel_glue.c | 4 ++--
  arch/x86/crypto/crct10dif-pclmul_glue.c | 4 ++--
  arch/x86/crypto/des3_ede_glue.c | 8 
  arch/x86/crypto/ghash-clmulni-intel_glue.c  | 2 +-
  arch/x86/crypto/salsa20_glue.c  | 4 ++--
  arch/x86/crypto/serpent_avx2_glue.c | 4 ++--
  arch/x86/crypto/serpent_avx_glue.c  | 2 +-
  arch/x86/crypto/serpent_sse2_glue.c | 2 +-
  arch/x86/crypto/sha1_ssse3_glue.c   | 2 +-
  arch/x86/crypto/sha256_ssse3_glue.c | 4 ++--
  arch/x86/crypto/sha512_ssse3_glue.c | 4 ++--
  arch/x86/crypto/twofish_avx_glue.c  | 2 +-
  arch/x86/crypto/twofish_glue.c  | 4 ++--
  arch/x86/crypto/twofish_glue_3way.c | 4 ++--
  crypto/842.c| 1 +
  crypto/aes_generic.c| 2 +-
  crypto/ansi_cprng.c | 2 +-
  crypto/anubis.c | 1 +
  crypto/api.c| 4 ++--
  crypto/arc4.c   | 1 +
  crypto/blowfish_generic.c   | 2 +-
  crypto/camellia_generic.c   | 2 +-
  crypto/cast5_generic.c  | 2 +-
  crypto/cast6_generic.c  | 2 +-
  crypto/ccm.c| 4 ++--
  crypto/crc32.c  | 1 +
  crypto/crc32c_generic.c | 2 +-
  crypto/crct10dif_generic.c  | 2 +-
  crypto/crypto_null.c| 6 +++---
  crypto/ctr.c| 2 +-
  crypto/deflate.c| 2 +-
  crypto/des_generic.c| 2 +-
  crypto/fcrypt.c | 1 +
  crypto/gcm.c| 6 +++---
  crypto/ghash-generic.c  | 2 +-
  crypto/khazad.c | 1 +
  crypto/krng.c   | 2 +-
  crypto/lz4.c| 1 +
  crypto/lz4hc.c  | 1 +
  crypto/lzo.c| 1 +
  crypto/md4.c| 2 +-
  crypto/md5.c| 1 +
  crypto/michael_mic.c| 1 +
  crypto/rmd128.c | 1 +
  crypto/rmd160.c | 1 +
  crypto/rmd256.c | 1 +
  crypto/rmd320.c | 1 +
  crypto/salsa20_generic.c| 2 +-
  crypto/seed.c   | 1 +
  crypto/serpent_generic.c| 4 ++--
  crypto/sha1_generic.c   | 2 +-
  crypto/sha256_generic.c | 4 ++--
  crypto/sha512_generic.c | 4 ++--
  crypto/tea.c| 4 ++--
  crypto/tgr192.c | 4 ++--
  

Re: [PATCH v2] crypto: prefix module autoloading with crypto-

2014-11-17 Thread Kees Cook
On Mon, Nov 17, 2014 at 3:20 PM, Mathias Krause mini...@googlemail.com wrote:
 On 17 November 2014 21:02, Kees Cook keesc...@chromium.org wrote:
 This prefixes all crypto module loading with crypto- so we never run
 the risk of exposing module auto-loading to userspace via a crypto API,
 as demonstrated by Mathias Krause:

 https://lkml.org/lkml/2013/3/4/70

 Signed-off-by: Kees Cook keesc...@chromium.org
 ---
 v2:
  - added missing #include, thanks to minipli
  - built with allmodconfig
 [...snip...]
 diff --git a/include/linux/crypto.h b/include/linux/crypto.h
 index d45e949699ea..d14230f6e977 100644
 --- a/include/linux/crypto.h
 +++ b/include/linux/crypto.h
 @@ -26,6 +26,13 @@
  #include linux/uaccess.h

  /*
 + * Autoloaded crypto modules should only use a prefixed name to avoid 
 allowing
 + * arbitrary modules to be loaded.
 + */
 +#define MODULE_ALIAS_CRYPTO(name)  \
 +   MODULE_ALIAS(crypto- name)

 This would break userland relying on the old aliases, e.g. 'modprobe
 aes' no longer works.

 Why not have both aliases, one with the crypto- prefix for on-demand
 loading within the crypto API and one without for manual loading from
 userland? E.g., something like this:

 #define MODULE_ALIAS_CRYPTO(name)  \
MODULE_ALIAS(name); \
MODULE_ALIAS(crypto- name)

 That would prevent the userland breakage and still achieves the goal
 of restricting the request_module() call offered by the means of the
 AF_ALG API.

That was my intention originally, and I should go back to it. The
trouble is with the use of __UNIQUE_ID in the MODULE_ALIAS macro. It
uses __LINE__ to produce the id, so the suggested macro expansion
(which is what I started with) won't work on non-gcc compilers.

I haven't found any solutions for C89 version of gcc's __COUNTER__,
and I haven't found any C89 ways to force a macro to be expanded as
being multi-line.

I'd like to avoid having to open-code both MODULE_ALIAS and
MODULE_ALIAS_CRYPTO in each module's source.

Anyone see some sneaky way to accomplish this?

-Kees

-- 
Kees Cook
Chrome OS Security
--
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