Linux cryptographic layer development
 help / color / mirror / Atom feed
* Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot
From: joeyli @ 2013-09-26 13:20 UTC (permalink / raw)
  To: Vojtech Pavlik
  Cc: Pavel Machek, Alan Stern, David Howells, linux-kernel,
	linux-security-module, linux-efi, linux-pm, linux-crypto,
	opensuse-kernel, Rafael J. Wysocki, Matthew Garrett, Len Brown,
	Josh Boyer, Matt Fleming, James Bottomley, Greg KH, JKosina,
	Rusty Russell, Herbert Xu, David S. Miller, H. Peter Anvin,
	Michal Marek, Gary Lin, Vivek Goyal
In-Reply-To: <20130926122210.GA30225@suse.cz>

於 四,2013-09-26 於 14:22 +0200,Vojtech Pavlik 提到:
> On Thu, Sep 26, 2013 at 02:06:21PM +0200, Pavel Machek wrote:
> 
> > > For the symmetric key solution, I will try HMAC (Hash Message
> > > Authentication Code). It's already used in networking, hope the
> > > performance is not too bad to a big image.
> > 
> > Kernel already supports crc32 of the hibernation image, you may want
> > to take a look how that is done.
> > 
> > Maybe you want to replace crc32 with cryptographics hash (sha1?) and
> > then use only hash for more crypto? That way speed of whatever
> crypto
> > you do should not be an issue.
> 
> Well, yes, one could skip the CRC when the signing is enabled to gain
> a
> little speedup. 

In current kernel, CRC is for check the integrity of LZO compressed
image, the purpose is different to check the integrity of snapshot
image.
So, CRC will not in non-compress hibernate or userspace hibernate code
path

On the other hand, attacker can easily change the CRC code in the header
of LZO hibernate image.

Thanks a lot!
Joey Lee

^ permalink raw reply

* Re: [PATCH 2/3] ARM: mxs: crypto: Add Freescale MXS DCP driver
From: Marek Vasut @ 2013-09-26 14:02 UTC (permalink / raw)
  To: Veli-Pekka Peltola
  Cc: linux-crypto, Herbert Xu, linux-arm-kernel, David S. Miller
In-Reply-To: <CAO+CcW=h1AZPYDR1q6LUcxbHWFbjMn7DwQnDnfXMS6MykApfdw@mail.gmail.com>

Dear Veli-Pekka Peltola,

> Hi Marek,
> 
> > +         To compile this driver as a module, choose M here: the module
> > +         will be called atmel-sha.
> 
> This is probably wrong?

Certainly. Nice to have you back btw. ;-)

Best regards,
Marek Vasut

^ permalink raw reply

* Re: [PATCH 2/3] ARM: mxs: crypto: Add Freescale MXS DCP driver
From: Marek Vasut @ 2013-09-26 14:04 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: linux-crypto, Herbert Xu, linux-arm-kernel, David S. Miller
In-Reply-To: <21060.9453.908125.71858@ipc1.ka-ro>

Dear Lothar Waßmann,

> Hi Marek,
> 
> some small comments below.
> 
> Marek Vasut writes:
> > diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
> > new file mode 100644
> > index 0000000..c2b35c7
> > --- /dev/null
> > +++ b/drivers/crypto/mxs-dcp.c
> 
> [...]
> 
> > +/* AES 128 ECB and AES 128 CBC */
> > +static struct crypto_alg dcp_aes_algs[] = {
> > +	[0] = {
> > +		.cra_name		= "ecb(aes)",
> > +		.cra_driver_name	= "ecb-aes-dcp",
> > +		.cra_priority		= 400,
> > +		.cra_alignmask		= 15,
> > +		.cra_flags		= CRYPTO_ALG_TYPE_ABLKCIPHER |
> > +					  CRYPTO_ALG_ASYNC |
> > +					  CRYPTO_ALG_NEED_FALLBACK,
> > +		.cra_init		= mxs_dcp_aes_fallback_init,
> > +		.cra_exit		= mxs_dcp_aes_fallback_exit,
> > +		.cra_blocksize		= AES_BLOCK_SIZE,
> > +		.cra_ctxsize		= sizeof(struct dcp_async_ctx),
> > +		.cra_type		= &crypto_ablkcipher_type,
> > +		.cra_module		= THIS_MODULE,
> > +		.cra_u	= {
> > +			.ablkcipher = {
> > +				.min_keysize	= AES_MIN_KEY_SIZE,
> > +				.max_keysize	= AES_MAX_KEY_SIZE,
> > +				.setkey		= mxs_dcp_aes_setkey,
> > +				.encrypt	= mxs_dcp_aes_ecb_encrypt,
> > +				.decrypt	= mxs_dcp_aes_ecb_decrypt
> > +			}
> 
> missing ',' after '}'

Is this a problem? The last ',' is not needed by the C standard.

[...]


> 
> > +static const struct of_device_id mxs_dcp_dt_ids[] = {
> > +	{.compatible = "fsl,mxs-dcp", .data = NULL,},
> 
> missing spaces after '{' and before '}'

Thanks!

Best regards,
Marek Vasut

^ permalink raw reply

* Re: [PATCH 2/3] ARM: mxs: crypto: Add Freescale MXS DCP driver
From: Lothar Waßmann @ 2013-09-26 14:09 UTC (permalink / raw)
  To: Marek Vasut; +Cc: linux-crypto, Herbert Xu, linux-arm-kernel, David S. Miller
In-Reply-To: <201309261604.37168.marex@denx.de>

Hi,

Marek Vasut writes:
> Dear Lothar Waßmann,
> 
> > Hi Marek,
> > 
> > some small comments below.
> > 
> > Marek Vasut writes:
> > > diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
> > > new file mode 100644
> > > index 0000000..c2b35c7
> > > --- /dev/null
> > > +++ b/drivers/crypto/mxs-dcp.c
> > 
> > [...]
> > 
> > > +/* AES 128 ECB and AES 128 CBC */
> > > +static struct crypto_alg dcp_aes_algs[] = {
> > > +	[0] = {
> > > +		.cra_name		= "ecb(aes)",
> > > +		.cra_driver_name	= "ecb-aes-dcp",
> > > +		.cra_priority		= 400,
> > > +		.cra_alignmask		= 15,
> > > +		.cra_flags		= CRYPTO_ALG_TYPE_ABLKCIPHER |
> > > +					  CRYPTO_ALG_ASYNC |
> > > +					  CRYPTO_ALG_NEED_FALLBACK,
> > > +		.cra_init		= mxs_dcp_aes_fallback_init,
> > > +		.cra_exit		= mxs_dcp_aes_fallback_exit,
> > > +		.cra_blocksize		= AES_BLOCK_SIZE,
> > > +		.cra_ctxsize		= sizeof(struct dcp_async_ctx),
> > > +		.cra_type		= &crypto_ablkcipher_type,
> > > +		.cra_module		= THIS_MODULE,
> > > +		.cra_u	= {
> > > +			.ablkcipher = {
> > > +				.min_keysize	= AES_MIN_KEY_SIZE,
> > > +				.max_keysize	= AES_MAX_KEY_SIZE,
> > > +				.setkey		= mxs_dcp_aes_setkey,
> > > +				.encrypt	= mxs_dcp_aes_ecb_encrypt,
> > > +				.decrypt	= mxs_dcp_aes_ecb_decrypt
> > > +			}
> > 
> > missing ',' after '}'
> 
> Is this a problem? The last ',' is not needed by the C standard.
> 
The problem arises when someone wants to append another item to the
list. Without the comma he has to change two lines which may cause
merge conflicts if two people add different items to the same struct.

That's why we usually have (unnecessary) commas on the last element of
a struct initializer (except when they are meant to be the last
element of course).


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

^ permalink raw reply

* Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot
From: James Bottomley @ 2013-09-26 14:44 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Pavel Machek, Alan Stern, David Howells, Lee, Chun-Yi,
	linux-kernel, linux-security-module, linux-efi, linux-pm,
	linux-crypto, opensuse-kernel, Rafael J. Wysocki, Matthew Garrett,
	Len Brown, Josh Boyer, Vojtech Pavlik, Matt Fleming, Greg KH,
	Rusty Russell, Herbert Xu, David S. Miller, H. Peter Anvin,
	Michal Marek, Gary Lin, Vivek Goyal, Lee, Chun-Yi
In-Reply-To: <alpine.LNX.2.00.1309260819490.18703@pobox.suse.cz>

On Thu, 2013-09-26 at 08:24 +0200, Jiri Kosina wrote:
> On Wed, 25 Sep 2013, James Bottomley wrote:
> 
> > > I don't get this. Why is it important that current kernel can't
> > > recreate the signature?
> > 
> > The thread model is an attack on the saved information (i.e. the suspend
> > image) between it being saved by the old kernel and used by the new one.
> > The important point isn't that the new kernel doesn't have access to
> > K_{N-1} it's that no-one does: the key is destroyed as soon as the old
> > kernel terminates however the verification public part P_{N-1} survives.
> 
> James,
> 
> could you please describe the exact scenario you think that the symmetric 
> keys aproach doesn't protect against, while the assymetric key aproach 
> does?
> 
> The crucial points, which I believe make the symmetric key aproach work 
> (and I feel quite embarassed by the fact that I haven't realized this 
> initially when coming up with the assymetric keys aproach) are:
> 
> - the kernel that is performing the actual resumption is trusted in the 
>   secure boot model, i.e. you trust it to perform proper verification
> 
> - potentially malicious userspace (which is what we are protecting against 
>   -- malicious root creating fake hibernation image and issuing reboot) 
>   doesn't have access to the symmetric key

OK, so the scheme is to keep a symmetric key in BS that is passed into
the kernel each time (effectively a secret key) for signing and
validation?

The only two problems I see are

     1. The key isn't generational (any compromise obtains it).  This
        can be fixed by using a set of keys generated on each boot and
        passing in both K_{N-1} and K_N
     2. No external agency other than the next kernel can do the
        validation since the validating key has to be secret

The importance of 2 is just tripwire like detection ... perhaps it
doesn't really matter in a personal computer situation.  It would matter
in an enterprise where images are stored and re-used but until servers
have UEFI secure boot, that's not going to be an issue.

James




^ permalink raw reply

* Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot
From: Jiri Kosina @ 2013-09-26 14:48 UTC (permalink / raw)
  To: James Bottomley
  Cc: Pavel Machek, Alan Stern, David Howells, Lee, Chun-Yi,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	opensuse-kernel-stAJ6ESoqRxg9hUCZPvPmw, Rafael J. Wysocki,
	Matthew Garrett, Len Brown, Josh Boyer, Vojtech Pavlik,
	Matt Fleming, Greg KH, Rusty Russell, Herbert Xu, David S. Miller,
	H. Peter Anvin, Michal Marek, Gary Lin, Vivek Goyal
In-Reply-To: <1380206654.18835.56.camel-sFMDBYUN5F8GjUHQrlYNx2Wm91YjaHnnhRte9Li2A+AAvxtiuMwx3w@public.gmane.org>

On Thu, 26 Sep 2013, James Bottomley wrote:

> > could you please describe the exact scenario you think that the symmetric 
> > keys aproach doesn't protect against, while the assymetric key aproach 
> > does?
> > 
> > The crucial points, which I believe make the symmetric key aproach work 
> > (and I feel quite embarassed by the fact that I haven't realized this 
> > initially when coming up with the assymetric keys aproach) are:
> > 
> > - the kernel that is performing the actual resumption is trusted in the 
> >   secure boot model, i.e. you trust it to perform proper verification
> > 
> > - potentially malicious userspace (which is what we are protecting against 
> >   -- malicious root creating fake hibernation image and issuing reboot) 
> >   doesn't have access to the symmetric key
> 
> OK, so the scheme is to keep a symmetric key in BS that is passed into
> the kernel each time (effectively a secret key) for signing and
> validation?

Exactly.

> The only two problems I see are
> 
>      1. The key isn't generational (any compromise obtains it).  This
>         can be fixed by using a set of keys generated on each boot and
>         passing in both K_{N-1} and K_N

I think this could be easily made optional, leaving the user with choice 
of faster or "safer" boot.

>      2. No external agency other than the next kernel can do the
>         validation since the validating key has to be secret

This is true, but as you said, the relevance of this seems to be rather 
questionable.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot
From: Vojtech Pavlik @ 2013-09-26 14:56 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: James Bottomley, Pavel Machek, Alan Stern, David Howells,
	Lee, Chun-Yi, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	opensuse-kernel-stAJ6ESoqRxg9hUCZPvPmw, Rafael J. Wysocki,
	Matthew Garrett, Len Brown, Josh Boyer, Matt Fleming, Greg KH,
	Rusty Russell, Herbert Xu, David S. Miller, H. Peter Anvin,
	Michal Marek, Gary Lin, Vivek Goyal, Lee, Chun-Yi
In-Reply-To: <alpine.LNX.2.00.1309261646150.18703-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org>

On Thu, Sep 26, 2013 at 04:48:00PM +0200, Jiri Kosina wrote:

> > The only two problems I see are
> > 
> >      1. The key isn't generational (any compromise obtains it).  This
> >         can be fixed by using a set of keys generated on each boot and
> >         passing in both K_{N-1} and K_N
> 
> I think this could be easily made optional, leaving the user with choice 
> of faster or "safer" boot.

Ideally, the key should be regenerated on each true reboot and kept the
same if it is just a resume. Unfortunately, I don't see a way to
distinguish those before we call ExitBootServices().

The reasoning behind that is that in the case of a kernel compromise, a
suspended-and-resumed kernel will still be compromised, so there is no
value in passing it a new key. A freshly booted kernel, though, should
get a new key, exactly because the attacker could have obtained a key
from the previous, compromised one.

This speeds up the ususal suspend-and-resume cycle, but provides full
security once the user performs a full reboot.

The question that remains is how to tell in advance.

> >      2. No external agency other than the next kernel can do the
> >         validation since the validating key has to be secret
> 
> This is true, but as you said, the relevance of this seems to be rather 
> questionable.

Indeed, it's hard to imagine a scenario that is also valid within the
secure boot threat model.

-- 
Vojtech Pavlik
Director SUSE Labs

^ permalink raw reply

* Re: IV generation in geode-aes
From: Hamid Nassiby @ 2013-09-26 16:42 UTC (permalink / raw)
  To: Sohail; +Cc: linux-crypto
In-Reply-To: <loom.20130919T140312-617@post.gmane.org>

Hi,

In geode-aes, there is not any IV generation mechanism. In fact IV is
delivered to geode-aes's
registered algorithms, from upper layers.For example in case of
"cbc-aes-geode" algorithm,
from cbc wrapper ("cbc.c") via walk->iv:

blkcipher_walk_init(&walk, dst, src, nbytes);
err = blkcipher_walk_virt(desc, &walk);
op->iv = walk.iv;
...

Regards.

On Thu, Sep 19, 2013 at 4:34 PM, Sohail <sohailtariq25@gmail.com> wrote:
> Hi all,
> I could'nt understand the mechanism of IV generation in geode-aes. Can
> someone explain it in easy to understand manner?
>
> Thanks a lot.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: crypto: crypto_memneq - add equality testing of memory regions w/o timing leaks
From: James Yonan @ 2013-09-26 17:45 UTC (permalink / raw)
  To: James Yonan; +Cc: herbert, linux-crypto, Daniel Borkmann, Florian Weimer
In-Reply-To: <1380183639-6288-1-git-send-email-james@openvpn.net>

Here is the latest iteration of the constant-time memory 
equality-testing patch:

* This patch includes contributions and a signoff from Daniel Borkmann.

* Moved the optimization flag -Os (used to prevent early return 
optimization) into the Makefile after seeing a report that #pragma gcc 
is considered broken and unmaintained.

* Added #ifndef __HAVE_ARCH_CRYPTO_MEMNEQ to accommodate possible future 
arch-specific asm implementations.

* For clarity, separated the size-independent function 
(__crypto_memneq_generic) from the fast-path optimization for 16 bytes 
(__crypto_memneq_16) using a switch so that future fast-path data widths 
can be easily added.

* Reduce the number of #ifdefs by using sizeof(unsigned long) instead of 
BITS_PER_LONG.

* Shortened the public function name to crypto_memneq.

James

On 26/09/2013 02:20, James Yonan wrote:
> When comparing MAC hashes, AEAD authentication tags, or other hash
> values in the context of authentication or integrity checking, it
> is important not to leak timing information to a potential attacker,
> i.e. when communication happens over a network.
>
> Bytewise memory comparisons (such as memcmp) are usually optimized so
> that they return a nonzero value as soon as a mismatch is found. E.g,
> on x86_64/i5 for 512 bytes this can be ~50 cyc for a full mismatch
> and up to ~850 cyc for a full match (cold). This early-return behavior
> can leak timing information as a side channel, allowing an attacker to
> iteratively guess the correct result.
>
> This patch adds a new method crypto_memneq ("memory not equal to each
> other") to the crypto API that compares memory areas of the same length
> in roughly "constant time" (cache misses could change the timing, but
> since they don't reveal information about the content of the strings
> being compared, they are effectively benign). Iow, best and worst case
> behaviour take the same amount of time to complete (in contrast to
> memcmp).
>
> Note that crypto_memneq (unlike memcmp) can only be used to test for
> equality or inequality, NOT for lexicographical order. This, however,
> is not an issue for its use-cases within the crypto API.
>
> We tried to locate all of the places in the crypto API where memcmp was
> being used for authentication or integrity checking, and convert them
> over to crypto_memneq.
>
> crypto_memneq is declared noinline, placed in its own source file,
> and compiled with optimizations that might increase code size disabled
> ("Os") because a smart compiler (or LTO) might notice that the return
> value is always compared against zero/nonzero, and might then
> reintroduce the same early-return optimization that we are trying to
> avoid.
>
> Using #pragma or __attribute__ optimization annotations of the code
> for disabling optimization was avoided as it seems to be considered
> broken or unmaintained for long time in GCC [1]. Therefore, we work
> around that by specifying the compile flag for memneq.o directly in
> the Makefile. We found that this seems to be most appropriate.
>
> As we use ("Os"), this patch also provides a loop-free "fast-path" for
> frequently used 16 byte digests. Similarly to kernel library string
> functions, leave an option for future even further optimized architecture
> specific assembler implementations.
>
> This was a joint work of James Yonan and Daniel Borkmann. Also thanks
> for feedback from Florian Weimer on this and earlier proposals [2].
>
>    [1] http://gcc.gnu.org/ml/gcc/2012-07/msg00211.html
>    [2] https://lkml.org/lkml/2013/2/10/131
>
> Signed-off-by: James Yonan <james@openvpn.net>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Florian Weimer <fw@deneb.enyo.de>
> ---
>   crypto/Makefile              |   7 ++-
>   crypto/asymmetric_keys/rsa.c |   5 +-
>   crypto/authenc.c             |   6 +-
>   crypto/authencesn.c          |   8 +--
>   crypto/ccm.c                 |   4 +-
>   crypto/gcm.c                 |   2 +-
>   crypto/memneq.c              | 138 +++++++++++++++++++++++++++++++++++++++++++
>   include/crypto/algapi.h      |  18 +++++-
>   8 files changed, 174 insertions(+), 14 deletions(-)
>   create mode 100644 crypto/memneq.c
>
> diff --git a/crypto/Makefile b/crypto/Makefile
> index 2d5ed08..b88cdf0 100644
> --- a/crypto/Makefile
> +++ b/crypto/Makefile
> @@ -2,8 +2,13 @@
>   # Cryptographic API
>   #
>
> +# memneq MUST be built with -Os or -O0 to prevent early-return optimizations
> +# that will defeat memneq's actual purpose to prevent timing attacks.
> +CFLAGS_REMOVE_memneq.o := -O1 -O2 -O3
> +CFLAGS_memneq.o := -Os
> +
>   obj-$(CONFIG_CRYPTO) += crypto.o
> -crypto-y := api.o cipher.o compress.o
> +crypto-y := api.o cipher.o compress.o memneq.o
>
>   obj-$(CONFIG_CRYPTO_WORKQUEUE) += crypto_wq.o
>
> diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
> index 4a6a069..1912b9b 100644
> --- a/crypto/asymmetric_keys/rsa.c
> +++ b/crypto/asymmetric_keys/rsa.c
> @@ -13,6 +13,7 @@
>   #include <linux/module.h>
>   #include <linux/kernel.h>
>   #include <linux/slab.h>
> +#include <crypto/algapi.h>
>   #include "public_key.h"
>
>   MODULE_LICENSE("GPL");
> @@ -189,12 +190,12 @@ static int RSA_verify(const u8 *H, const u8 *EM, size_t k, size_t hash_size,
>   		}
>   	}
>
> -	if (memcmp(asn1_template, EM + T_offset, asn1_size) != 0) {
> +	if (crypto_memneq(asn1_template, EM + T_offset, asn1_size) != 0) {
>   		kleave(" = -EBADMSG [EM[T] ASN.1 mismatch]");
>   		return -EBADMSG;
>   	}
>
> -	if (memcmp(H, EM + T_offset + asn1_size, hash_size) != 0) {
> +	if (crypto_memneq(H, EM + T_offset + asn1_size, hash_size) != 0) {
>   		kleave(" = -EKEYREJECTED [EM[T] hash mismatch]");
>   		return -EKEYREJECTED;
>   	}
> diff --git a/crypto/authenc.c b/crypto/authenc.c
> index ffce19d..2b3f4ab 100644
> --- a/crypto/authenc.c
> +++ b/crypto/authenc.c
> @@ -188,7 +188,7 @@ static void authenc_verify_ahash_update_done(struct crypto_async_request *areq,
>   	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
>   				 authsize, 0);
>
> -	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> +	err = crypto_memneq(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
>   	if (err)
>   		goto out;
>
> @@ -227,7 +227,7 @@ static void authenc_verify_ahash_done(struct crypto_async_request *areq,
>   	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
>   				 authsize, 0);
>
> -	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> +	err = crypto_memneq(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
>   	if (err)
>   		goto out;
>
> @@ -462,7 +462,7 @@ static int crypto_authenc_verify(struct aead_request *req,
>   	ihash = ohash + authsize;
>   	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
>   				 authsize, 0);
> -	return memcmp(ihash, ohash, authsize) ? -EBADMSG : 0;
> +	return crypto_memneq(ihash, ohash, authsize) ? -EBADMSG : 0;
>   }
>
>   static int crypto_authenc_iverify(struct aead_request *req, u8 *iv,
> diff --git a/crypto/authencesn.c b/crypto/authencesn.c
> index ab53762..c569d58 100644
> --- a/crypto/authencesn.c
> +++ b/crypto/authencesn.c
> @@ -247,7 +247,7 @@ static void authenc_esn_verify_ahash_update_done(struct crypto_async_request *ar
>   	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
>   				 authsize, 0);
>
> -	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> +	err = crypto_memneq(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
>   	if (err)
>   		goto out;
>
> @@ -296,7 +296,7 @@ static void authenc_esn_verify_ahash_update_done2(struct crypto_async_request *a
>   	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
>   				 authsize, 0);
>
> -	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> +	err = crypto_memneq(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
>   	if (err)
>   		goto out;
>
> @@ -336,7 +336,7 @@ static void authenc_esn_verify_ahash_done(struct crypto_async_request *areq,
>   	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
>   				 authsize, 0);
>
> -	err = memcmp(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
> +	err = crypto_memneq(ihash, ahreq->result, authsize) ? -EBADMSG : 0;
>   	if (err)
>   		goto out;
>
> @@ -568,7 +568,7 @@ static int crypto_authenc_esn_verify(struct aead_request *req)
>   	ihash = ohash + authsize;
>   	scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
>   				 authsize, 0);
> -	return memcmp(ihash, ohash, authsize) ? -EBADMSG : 0;
> +	return crypto_memneq(ihash, ohash, authsize) ? -EBADMSG : 0;
>   }
>
>   static int crypto_authenc_esn_iverify(struct aead_request *req, u8 *iv,
> diff --git a/crypto/ccm.c b/crypto/ccm.c
> index 499c917..3e05499 100644
> --- a/crypto/ccm.c
> +++ b/crypto/ccm.c
> @@ -363,7 +363,7 @@ static void crypto_ccm_decrypt_done(struct crypto_async_request *areq,
>
>   	if (!err) {
>   		err = crypto_ccm_auth(req, req->dst, cryptlen);
> -		if (!err && memcmp(pctx->auth_tag, pctx->odata, authsize))
> +		if (!err && crypto_memneq(pctx->auth_tag, pctx->odata, authsize))
>   			err = -EBADMSG;
>   	}
>   	aead_request_complete(req, err);
> @@ -422,7 +422,7 @@ static int crypto_ccm_decrypt(struct aead_request *req)
>   		return err;
>
>   	/* verify */
> -	if (memcmp(authtag, odata, authsize))
> +	if (crypto_memneq(authtag, odata, authsize))
>   		return -EBADMSG;
>
>   	return err;
> diff --git a/crypto/gcm.c b/crypto/gcm.c
> index 43e1fb0..b4f0179 100644
> --- a/crypto/gcm.c
> +++ b/crypto/gcm.c
> @@ -582,7 +582,7 @@ static int crypto_gcm_verify(struct aead_request *req,
>
>   	crypto_xor(auth_tag, iauth_tag, 16);
>   	scatterwalk_map_and_copy(iauth_tag, req->src, cryptlen, authsize, 0);
> -	return memcmp(iauth_tag, auth_tag, authsize) ? -EBADMSG : 0;
> +	return crypto_memneq(iauth_tag, auth_tag, authsize) ? -EBADMSG : 0;
>   }
>
>   static void gcm_decrypt_done(struct crypto_async_request *areq, int err)
> diff --git a/crypto/memneq.c b/crypto/memneq.c
> new file mode 100644
> index 0000000..cd01622
> --- /dev/null
> +++ b/crypto/memneq.c
> @@ -0,0 +1,138 @@
> +/*
> + * Constant-time equality testing of memory regions.
> + *
> + * Authors:
> + *
> + *   James Yonan <james@openvpn.net>
> + *   Daniel Borkmann <dborkman@redhat.com>
> + *
> + * This file is provided under a dual BSD/GPLv2 license.  When using or
> + * redistributing this file, you may do so under either license.
> + *
> + * GPL LICENSE SUMMARY
> + *
> + * Copyright(c) 2013 OpenVPN Technologies, Inc. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
> + * The full GNU General Public License is included in this distribution
> + * in the file called LICENSE.GPL.
> + *
> + * BSD LICENSE
> + *
> + * Copyright(c) 2013 OpenVPN Technologies, Inc. All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + *   * Redistributions of source code must retain the above copyright
> + *     notice, this list of conditions and the following disclaimer.
> + *   * Redistributions in binary form must reproduce the above copyright
> + *     notice, this list of conditions and the following disclaimer in
> + *     the documentation and/or other materials provided with the
> + *     distribution.
> + *   * Neither the name of OpenVPN Technologies nor the names of its
> + *     contributors may be used to endorse or promote products derived
> + *     from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include <crypto/algapi.h>
> +
> +#ifndef __HAVE_ARCH_CRYPTO_MEMNEQ
> +
> +/* Generic path for arbitrary size */
> +static inline unsigned long
> +__crypto_memneq_generic(const void *a, const void *b, size_t size)
> +{
> +	unsigned long neq = 0;
> +
> +#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
> +	while (size >= sizeof(unsigned long)) {
> +		neq |= *(unsigned long *)a ^ *(unsigned long *)b;
> +		a += sizeof(unsigned long);
> +		b += sizeof(unsigned long);
> +		size -= sizeof(unsigned long);
> +	}
> +#endif /* CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS */
> +	while (size > 0) {
> +		neq |= *(unsigned char *)a ^ *(unsigned char *)b;
> +		a += 1;
> +		b += 1;
> +		size -= 1;
> +	}
> +	return neq;
> +}
> +
> +/* Loop-free fast-path for frequently used 16-byte size */
> +static inline unsigned long __crypto_memneq_16(const void *a, const void *b)
> +{
> +#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> +	if (sizeof(unsigned long) == 8)
> +		return ((*(unsigned long *)(a)   ^ *(unsigned long *)(b))
> +		      | (*(unsigned long *)(a+8) ^ *(unsigned long *)(b+8)));
> +	else if (sizeof(unsigned int) == 4)
> +		return ((*(unsigned int *)(a)    ^ *(unsigned int *)(b))
> +                      | (*(unsigned int *)(a+4)  ^ *(unsigned int *)(b+4))
> +		      | (*(unsigned int *)(a+8)  ^ *(unsigned int *)(b+8))
> +	              | (*(unsigned int *)(a+12) ^ *(unsigned int *)(b+12)));
> +	else
> +#endif /* CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS */
> +		return ((*(unsigned char *)(a)    ^ *(unsigned char *)(b))
> +		      | (*(unsigned char *)(a+1)  ^ *(unsigned char *)(b+1))
> +		      | (*(unsigned char *)(a+2)  ^ *(unsigned char *)(b+2))
> +		      | (*(unsigned char *)(a+3)  ^ *(unsigned char *)(b+3))
> +		      | (*(unsigned char *)(a+4)  ^ *(unsigned char *)(b+4))
> +		      | (*(unsigned char *)(a+5)  ^ *(unsigned char *)(b+5))
> +		      | (*(unsigned char *)(a+6)  ^ *(unsigned char *)(b+6))
> +		      | (*(unsigned char *)(a+7)  ^ *(unsigned char *)(b+7))
> +		      | (*(unsigned char *)(a+8)  ^ *(unsigned char *)(b+8))
> +		      | (*(unsigned char *)(a+9)  ^ *(unsigned char *)(b+9))
> +		      | (*(unsigned char *)(a+10) ^ *(unsigned char *)(b+10))
> +		      | (*(unsigned char *)(a+11) ^ *(unsigned char *)(b+11))
> +		      | (*(unsigned char *)(a+12) ^ *(unsigned char *)(b+12))
> +		      | (*(unsigned char *)(a+13) ^ *(unsigned char *)(b+13))
> +		      | (*(unsigned char *)(a+14) ^ *(unsigned char *)(b+14))
> +		      | (*(unsigned char *)(a+15) ^ *(unsigned char *)(b+15)));
> +}
> +
> +/* Compare two areas of memory without leaking timing information,
> + * and with special optimizations for common sizes.  Users should
> + * not call this function directly, but should instead use
> + * crypto_memneq defined in crypto/algapi.h.
> + */
> +noinline unsigned long __crypto_memneq(const void *a, const void *b,
> +				       size_t size)
> +{
> +	switch (size) {
> +	case 16:
> +		return __crypto_memneq_16(a, b);
> +	default:
> +		return __crypto_memneq_generic(a, b, size);
> +	}
> +}
> +EXPORT_SYMBOL(__crypto_memneq);
> +
> +#endif /* __HAVE_ARCH_CRYPTO_MEMNEQ */
> diff --git a/include/crypto/algapi.h b/include/crypto/algapi.h
> index 418d270..e73c19e 100644
> --- a/include/crypto/algapi.h
> +++ b/include/crypto/algapi.h
> @@ -386,5 +386,21 @@ static inline int crypto_requires_sync(u32 type, u32 mask)
>   	return (type ^ CRYPTO_ALG_ASYNC) & mask & CRYPTO_ALG_ASYNC;
>   }
>
> -#endif	/* _CRYPTO_ALGAPI_H */
> +noinline unsigned long __crypto_memneq(const void *a, const void *b, size_t size);
> +
> +/**
> + * crypto_memneq - Compare two areas of memory without leaking
> + *		   timing information.
> + *
> + * @a: One area of memory
> + * @b: Another area of memory
> + * @size: The size of the area.
> + *
> + * Returns 0 when data is equal, 1 otherwise.
> + */
> +static inline int crypto_memneq(const void *a, const void *b, size_t size)
> +{
> +	return __crypto_memneq(a, b, size) != 0UL ? 1 : 0;
> +}
>
> +#endif	/* _CRYPTO_ALGAPI_H */
>

^ permalink raw reply

* Re: [PATCH 00/51] DMA mask changes
From: Rafał Miłecki @ 2013-09-26 20:23 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: alsa-devel, b43-dev, devel@driverdev.osuosl.org, devicetree,
	dri-devel, e1000-devel, linux-arm-kernel@lists.infradead.org,
	linux-crypto, linux-doc, linux-fbdev, linux-ide, linux-media,
	linux-mmc, linux-nvme, linux-omap, linux ppc dev,
	linux-samsung-soc, Linux SCSI List, linux-tegra, linux-usb,
	linux-wireless@vger.kernel.org, Network Development,
	Solarflare linux maintainers <linux-ne
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

2013/9/19 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> This email is only being sent to the mailing lists in question, not to
> anyone personally.  The list of individuals is far to great to do that.
> I'm hoping no mailing lists reject the patches based on the number of
> recipients.

Huh, I think it was enough to send only 3 patches to the b43-dev@. Like:
[PATCH 01/51] DMA-API: provide a helper to set both DMA and coherent DMA masks
[PATCH 14/51] DMA-API: net: b43: (...)
[PATCH 15/51] DMA-API: net: b43legacy: (...)
;)

I believe Joe has some nice script for doing it that way. When fixing
some coding style / formatting, he sends only related patches to the
given ML.

-- 
Rafał
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 31/51] DMA-API: media: omap3isp: use dma_coerce_mask_and_coherent()
From: Laurent Pinchart @ 2013-09-27  1:56 UTC (permalink / raw)
  To: Russell King
  Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	e1000-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-ide-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Solarflare linux maintainers,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b,
	Mauro Carvalho Chehab
In-Reply-To: <E1VMmCg-0007j1-Pi-eh5Bv4kxaXIANfyc6IWni62ZND6+EDdj@public.gmane.org>

Hi Russell,

Thank you for the patch.

On Thursday 19 September 2013 22:56:02 Russell King wrote:
> The code sequence:
> 	isp->raw_dmamask = DMA_BIT_MASK(32);
> 	isp->dev->dma_mask = &isp->raw_dmamask;
> 	isp->dev->coherent_dma_mask = DMA_BIT_MASK(32);
> bypasses the architectures check on the DMA mask.  It can be replaced
> with dma_coerce_mask_and_coherent(), avoiding the direct initialization
> of this mask.
> 
> Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>

Acked-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

> ---
>  drivers/media/platform/omap3isp/isp.c |    6 +++---
>  drivers/media/platform/omap3isp/isp.h |    3 ---
>  2 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index df3a0ec..1c36080 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2182,9 +2182,9 @@ static int isp_probe(struct platform_device *pdev)
>  	isp->pdata = pdata;
>  	isp->ref_count = 0;
> 
> -	isp->raw_dmamask = DMA_BIT_MASK(32);
> -	isp->dev->dma_mask = &isp->raw_dmamask;
> -	isp->dev->coherent_dma_mask = DMA_BIT_MASK(32);
> +	ret = dma_coerce_mask_and_coherent(isp->dev, DMA_BIT_MASK(32));
> +	if (ret)
> +		return ret;
> 
>  	platform_set_drvdata(pdev, isp);
> 
> diff --git a/drivers/media/platform/omap3isp/isp.h
> b/drivers/media/platform/omap3isp/isp.h index cd3eff4..ce65d3a 100644
> --- a/drivers/media/platform/omap3isp/isp.h
> +++ b/drivers/media/platform/omap3isp/isp.h
> @@ -152,7 +152,6 @@ struct isp_xclk {
>   * @mmio_base_phys: Array with physical L4 bus addresses for ISP register
>   *                  regions.
>   * @mmio_size: Array with ISP register regions size in bytes.
> - * @raw_dmamask: Raw DMA mask
>   * @stat_lock: Spinlock for handling statistics
>   * @isp_mutex: Mutex for serializing requests to ISP.
>   * @crashed: Bitmask of crashed entities (indexed by entity ID)
> @@ -190,8 +189,6 @@ struct isp_device {
>  	unsigned long mmio_base_phys[OMAP3_ISP_IOMEM_LAST];
>  	resource_size_t mmio_size[OMAP3_ISP_IOMEM_LAST];
> 
> -	u64 raw_dmamask;
> -
>  	/* ISP Obj */
>  	spinlock_t stat_lock;	/* common lock for statistic drivers */
>  	struct mutex isp_mutex;	/* For handling ref_count field */
-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 00/51] DMA mask changes
From: Russell King - ARM Linux @ 2013-09-27  8:27 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: alsa-devel, linux-doc, linux-mmc, linux-fbdev, linux-nvme,
	linux-ide, devel@driverdev.osuosl.org, linux-samsung-soc,
	Linux SCSI List, e1000-devel, b43-dev, linux-media, devicetree,
	dri-devel, linux-tegra, linux-omap,
	linux-arm-kernel@lists.infradead.org,
	Solarflare linux maintainers, Network Development, linux-usb,
	linux-wireless@vger.kernel.org, linux-crypto, uclinux-dist-devel
In-Reply-To: <CACna6rxkpYzdD8_Jfi22vA2suUa3k-JM65_gCySQpp4crVCoPg@mail.gmail.com>

On Thu, Sep 26, 2013 at 10:23:08PM +0200, Rafał Miłecki wrote:
> 2013/9/19 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> > This email is only being sent to the mailing lists in question, not to
> > anyone personally.  The list of individuals is far to great to do that.
> > I'm hoping no mailing lists reject the patches based on the number of
> > recipients.
> 
> Huh, I think it was enough to send only 3 patches to the b43-dev@. Like:
> [PATCH 01/51] DMA-API: provide a helper to set both DMA and coherent DMA masks
> [PATCH 14/51] DMA-API: net: b43: (...)
> [PATCH 15/51] DMA-API: net: b43legacy: (...)
> ;)
> 
> I believe Joe has some nice script for doing it that way. When fixing
> some coding style / formatting, he sends only related patches to the
> given ML.

If I did that, then the mailing lists would not get the first patch,
because almost none of the lists would be referred to by patch 1.

Moreover, people complain when they don't have access to the full
patch series - they assume patches are missing for some reason, and
they ask for the rest of the series.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply

* Re: Asymmetric cryptography HW offloading
From: Horia Geantă @ 2013-09-27 10:58 UTC (permalink / raw)
  To: Nikos Mavrogiannopoulos; +Cc: linux-crypto@vger.kernel.org, Yashpal Dutta
In-Reply-To: <524041EE.5070209@gnutls.org>

On 9/23/2013 4:28 PM, Nikos Mavrogiannopoulos wrote:
> On 09/23/2013 02:31 PM, Horia Geantă wrote:
>> Hi,
>>
>> CAAM crypto engine (drivers/crypto/caam/*) is capable of asymmetric
>> operations, like: modular exponentiation, RSA
>> sign/verify/encrypt/decrypt, (EC)DSA sign etc.
>> I would appreciate some design guidelines on how to harness these
>> capabilities, for crypto engines in general.
>>
>> 1. In-kernel interface for asymmetric crypto
>> Should crypto/asymmetric_keys/* be used, i.e. appended with modular
>> exponentiation, other asymmetric operations?
> The BSD's cryptodev supports the following operations which may help in
> that aspect (no elliptic curve operations present). I don't know if all
> of them worth the context switch.
>
> #define CRK_MOD_EXP		0
> #define CRK_MOD_EXP_CRT		1
> #define CRK_DSA_SIGN		2
> #define CRK_DSA_VERIFY		3
> #define CRK_DH_COMPUTE_KEY	4
> #define CRK_MOD_ADD		5
> #define CRK_MOD_ADDINV		6
> #define CRK_MOD_SUB		7
> #define CRK_MOD_MULT		8
> #define CRK_MOD_MULTINV		9
> #define CRK_MOD			10

Thanks for the tip.
I took a look at BSD - AFAICT there is no SW implementation and crypto 
engine drivers handle only the first two operations (MOD_EXP).

My main concern now is the asymmetric ciphers API, that would eventually 
allow implementing the operations in SW/HW.
I was wondering whether the same logic as for symmetric ciphers 
could/should be used (the API layering mentioned in 
Documentation/crypto/api-intro.txt).
For example, crypto/asymmetric_keys/rsa.c could be registered and then 
used via Crypto API:
rsa.c: crypto_alg->cra_name = "rsa"; 
crypto_alg->cra_driver_name="rsa-generic"; crypto_register_alg(crypto_alg);
user: tfm = crypto_alloc_tfm("rsa",...);
User would get either the "rsa-generic" SW implementation or a HW 
implementation, if available.

>
>> 2. User space interface
>> Should AF_ALG be expanded to provide access to this new asymmetric cypto
>> API? The API would allow user space applications to offload PKC operations in
>> HW.
> I'd be interested into adding this support into cryptodev-linux once
> present in kernel.

Thanks.
We already have a draft implementation of asymmetric crypto + 
cryptodev-linux, but was developed prior to crypto/asymmetric_keys 
addition and thus has to be reworked.

Horia

^ permalink raw reply

* Re: [PATCH 2/3] ARM: mxs: crypto: Add Freescale MXS DCP driver
From: Tobias Rauter @ 2013-09-27 16:03 UTC (permalink / raw)
  To: Marek Vasut, Fabio Estevam
  Cc: linux-crypto, Herbert Xu, linux-arm-kernel@lists.infradead.org,
	David S. Miller, Shawn Guo
In-Reply-To: <201309261407.33923.marex@denx.de>

Hi,

Here are some thoughts of the design decisions I made when I wrote
the dcp.c driver. Maybe it helps.

On 2013-09-26 14:07, Marek Vasut wrote:
> Dear Fabio Estevam,
>
>> Hi Marek,
>>
>> Why do we need to have two drivers for the same IP block? It looks
>> confusing to have both.
>
> Sure, I agree. I reviewed the one in mainline just now and I see some
> deficiencies of the dcp.c driver:
>
> 1) It only supports AES_CBC (mine does support AES_ECB, AES_CBC, SHA1 and SH256)

Right, but for ECP only the interface is missing (and it is no real
mode of operation) and hashes should be generally faster in SW.

> 2) The driver was apparently never ran behind anyone working with MXS.


That is probably right.


> 3) What are those ugly new IOCTLs in the dcp.c driver?

When I firstly posted the driver in the mailinglist, there where one
person who actually used this interface (it was introduced in
Freescale's SDK) to use the OTP keys for crypto. As far as I have
seen, the crypto API does not support such keys (i.e. there seems to
be no way to tell a driver to use some kind of special keys - which
are not delivered by the user - via the API).
Therefore I added this miscdevice and adopted Freescale's interface.

> 4) The VMI IRQ is never used, yet it even calls the IRQ handler, this is bogus

That's absolutely right.

>     -> The DCP always triggers the dcp_irq upon DMA completion

The IRQ is triggered after every packet, to enable simultaneous work
for CPU/DCP: While the DCP is computing, the CPU is able to fill more
packets. I don't know how far this is useful, because the 20 Packets
which are enabled by default can address up to 80kB of
plain-/ciphertext. However, I think it is better to do the work 
simultaneously to safe time (actual real world time, not CPU time).

> 5) The IRQ handler can't use usual completion() in the driver because that'd
> trigger "scheduling while atomic" oops, yes?

I decided to use the tasklets because of performance reasons. I don't
remember numbers but a workqueue was significantly slower.  The
use of a kernel thread may reduce the overhead compared to the wq. I
was not sure if it is appropriate to create an extra thread for a
crypto-driver, without real reason (IMHO).

> Finally, because the dcp.c driver only supports AES128 CBC, it depends on kernel
> _always_ passing the DCP scatterlist such that each of it's elements is 16-bytes
> long. [...]
> So, in the AES128 case, if the hardware is passed two (4 bytes + 12 bytes for
> example) DMA descriptors instead of single 16 bytes descriptor, the DCP will
> simply stall or produce incorrect result. This can happen if the user of the
> async crypto API passes such a scatterlist.

The scatterlist alignment and bounce-buffering to get full 16 Byte
blocks is done by the ablkcipher_walk API (with the error
parameter) when needed. As far as I see, you are copying the whole 
buffer to your coherent block and back. Wouldn't it be better to do that 
just for unaligned blocks?


kind regards,
tr

^ permalink raw reply

* Re: [PATCH 2/3] ARM: mxs: crypto: Add Freescale MXS DCP driver
From: Marek Vasut @ 2013-09-28  3:35 UTC (permalink / raw)
  To: Tobias Rauter
  Cc: Fabio Estevam, linux-crypto, Herbert Xu,
	linux-arm-kernel@lists.infradead.org, David S. Miller, Shawn Guo
In-Reply-To: <5245AC5B.80800@gmail.com>

Hi Tobias,

> Hi,
> 
> Here are some thoughts of the design decisions I made when I wrote
> the dcp.c driver. Maybe it helps.

Was the driver not rather forward-ported from some old FSL BSP kernel?

> On 2013-09-26 14:07, Marek Vasut wrote:
> > Dear Fabio Estevam,
> > 
> >> Hi Marek,
> >> 
> >> Why do we need to have two drivers for the same IP block? It looks
> >> confusing to have both.
> > 
> > Sure, I agree. I reviewed the one in mainline just now and I see some
> > deficiencies of the dcp.c driver:
> > 
> > 1) It only supports AES_CBC (mine does support AES_ECB, AES_CBC, SHA1 and
> > SH256)
> 
> Right, but for ECP only the interface is missing (and it is no real
> mode of operation) and hashes should be generally faster in SW.

The ECB is slightly different from CBC, for example the IV handling differs. As 
for the performance, the SHA* hashing was more performant in hardware starting 
with ~8MB chunks. I checked this via the "openssl -speed" test via the linux 
cryptodev patch.

> > 2) The driver was apparently never ran behind anyone working with MXS.
> 
> That is probably right.
> 
> > 3) What are those ugly new IOCTLs in the dcp.c driver?
> 
> When I firstly posted the driver in the mailinglist, there where one
> person who actually used this interface (it was introduced in
> Freescale's SDK) to use the OTP keys for crypto. As far as I have
> seen, the crypto API does not support such keys (i.e. there seems to
> be no way to tell a driver to use some kind of special keys - which
> are not delivered by the user - via the API).
> Therefore I added this miscdevice and adopted Freescale's interface.

The keys are programmed into the OTP registers, correct? There is OCOTP driver 
for the MX23/MX28 OTP hardware. This is what should have been used then.

NOTE: This IOCTL interface seems like quite an abusive way to allow userland to 
access the crypto API in kernel. I understand this is used by some Freescale 
tool, but won't it be better to fix the Freescale tool instead ?

> > 4) The VMI IRQ is never used, yet it even calls the IRQ handler, this is
> > bogus
> 
> That's absolutely right.
> 
> >     -> The DCP always triggers the dcp_irq upon DMA completion
> 
> The IRQ is triggered after every packet, to enable simultaneous work
> for CPU/DCP: While the DCP is computing, the CPU is able to fill more
> packets. I don't know how far this is useful, because the 20 Packets
> which are enabled by default can address up to 80kB of
> plain-/ciphertext. However, I think it is better to do the work
> simultaneously to safe time (actual real world time, not CPU time).

This really depends on how you program the DCP's own DMA controller, or more 
precisely what you write into the DMA descriptors. Each descriptor has a bit 
that signalizes whether the DCP should generate an IRQ upon it's completion. You 
can thus program it to generate IRQ upon competing each descriptor only for the 
last descriptor in a chain or what fits you.

NOTE that chaining multiple DMA descriptors of small size is less performant 
than filling up a large buffer and then running it through the DCP in one longer 
DMA descriptor. This is actually why I have such a large bounce-buffer and not 
only a small one.

> > 5) The IRQ handler can't use usual completion() in the driver because
> > that'd trigger "scheduling while atomic" oops, yes?
> 
> I decided to use the tasklets because of performance reasons. I don't
> remember numbers but a workqueue was significantly slower.  The
> use of a kernel thread may reduce the overhead compared to the wq. I
> was not sure if it is appropriate to create an extra thread for a
> crypto-driver, without real reason (IMHO).

Certainly, my remark was that the FSL driver had this issue and the code here 
didn't change much from the FSL driver. I was thus curious why you don't use 
completion in the driver as usual and whether this was to work around the issue 
the FSL driver had.

> > Finally, because the dcp.c driver only supports AES128 CBC, it depends on
> > kernel _always_ passing the DCP scatterlist such that each of it's
> > elements is 16-bytes long. [...]
> > So, in the AES128 case, if the hardware is passed two (4 bytes + 12 bytes
> > for example) DMA descriptors instead of single 16 bytes descriptor, the
> > DCP will simply stall or produce incorrect result. This can happen if
> > the user of the async crypto API passes such a scatterlist.
> 
> The scatterlist alignment and bounce-buffering to get full 16 Byte
> blocks is done by the ablkcipher_walk API (with the error
> parameter) when needed. As far as I see, you are copying the whole
> buffer to your coherent block and back. Wouldn't it be better to do that
> just for unaligned blocks?

I tried writing a proper driver for this DCP piece of crap hardware about 7 
times. I tried to squeeze as much power from it as possible, among others tried 
to avoid the copying, but there was always some hidden problem somewhere. The 
DCP is really _horribly_ designed hardware. But to answer your question, the 
copying of stuff back and forth is actually not a problem here.

The real question here is, how do we handle the current situation? I think this 
might be a question for Dave or Herbert to answer though.

Best regards,
Marek Vasut

^ permalink raw reply

* Re: Asymmetric cryptography HW offloading
From: Nikos Mavrogiannopoulos @ 2013-09-29 17:50 UTC (permalink / raw)
  To: Horia Geantă; +Cc: linux-crypto@vger.kernel.org, Yashpal Dutta
In-Reply-To: <524564CF.6030508@freescale.com>

On 09/27/2013 12:58 PM, Horia Geantă wrote:

> Thanks for the tip.
> I took a look at BSD - AFAICT there is no SW implementation and crypto
> engine drivers handle only the first two operations (MOD_EXP).
> 
> My main concern now is the asymmetric ciphers API, that would eventually
> allow implementing the operations in SW/HW.
> I was wondering whether the same logic as for symmetric ciphers
> could/should be used (the API layering mentioned in
> Documentation/crypto/api-intro.txt).
> For example, crypto/asymmetric_keys/rsa.c could be registered and then
> used via Crypto API:
> rsa.c: crypto_alg->cra_name = "rsa";

As RSA fits into the encryption API it may be just ok. I'd say do it and
then we see whether that API causes some bottleneck for asymmetric
encryption. But how would you fit modexp in that case?

Having the expensive big number operations could be useful to non-crypto
number crunching projects as well (e.g. gmp and its users).

regards,
Nikos

^ permalink raw reply

* Getting started with crypto drivers
From: Emilio López @ 2013-09-29 18:10 UTC (permalink / raw)
  To: herbert, David Miller, linux-crypto, linux-kernel@vger.kernel.org

Hello everyone,

I would like to write a driver to support the "Security System" hardware 
block on some Allwinner ARM SoCs. The hardware supports AES, DES, 3DES, 
SHA-1, MD5 and has a PRNG. Data passing is done via two FIFOs. You can 
find some ugly userspace code to calculate a SHA1 hash here, so you can 
get an idea of how it works:

http://git.elopez.com.ar/ss-poc/raw/31c57d893ce8913aa0d87c982f5350433fbe89c7/md5-tool.c

My main issue at the moment is that I do not have any experience working 
with the crypto API, and that I found the documentation on 
Documentation/ lacking for my purposes. I'd be very grateful if you 
could suggest me reading material to understand the "hardware-oriented" 
API better, and if you could point me to good drivers I could use as an 
example when developing my own.

Thanks!

Emilio

^ permalink raw reply

* Re: [PATCH 2/3] ARM: mxs: crypto: Add Freescale MXS DCP driver
From: Marek Vasut @ 2013-09-29 22:05 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: linux-crypto, Herbert Xu, linux-arm-kernel, David S. Miller
In-Reply-To: <21060.16409.341402.552996@ipc1.ka-ro>

Dear Lothar Waßmann,

> Hi,
> 
> Marek Vasut writes:
> > Dear Lothar Waßmann,
> > 
> > > Hi Marek,
> > > 
> > > some small comments below.
> > > 
> > > Marek Vasut writes:
> > > > diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
> > > > new file mode 100644
> > > > index 0000000..c2b35c7
> > > > --- /dev/null
> > > > +++ b/drivers/crypto/mxs-dcp.c
> > > 
> > > [...]
> > > 
> > > > +/* AES 128 ECB and AES 128 CBC */
> > > > +static struct crypto_alg dcp_aes_algs[] = {
> > > > +	[0] = {
> > > > +		.cra_name		= "ecb(aes)",
> > > > +		.cra_driver_name	= "ecb-aes-dcp",
> > > > +		.cra_priority		= 400,
> > > > +		.cra_alignmask		= 15,
> > > > +		.cra_flags		= CRYPTO_ALG_TYPE_ABLKCIPHER |
> > > > +					  CRYPTO_ALG_ASYNC |
> > > > +					  CRYPTO_ALG_NEED_FALLBACK,
> > > > +		.cra_init		= mxs_dcp_aes_fallback_init,
> > > > +		.cra_exit		= mxs_dcp_aes_fallback_exit,
> > > > +		.cra_blocksize		= AES_BLOCK_SIZE,
> > > > +		.cra_ctxsize		= sizeof(struct dcp_async_ctx),
> > > > +		.cra_type		= &crypto_ablkcipher_type,
> > > > +		.cra_module		= THIS_MODULE,
> > > > +		.cra_u	= {
> > > > +			.ablkcipher = {
> > > > +				.min_keysize	= AES_MIN_KEY_SIZE,
> > > > +				.max_keysize	= AES_MAX_KEY_SIZE,
> > > > +				.setkey		= mxs_dcp_aes_setkey,
> > > > +				.encrypt	= 
mxs_dcp_aes_ecb_encrypt,
> > > > +				.decrypt	= 
mxs_dcp_aes_ecb_decrypt
> > > > +			}
> > > 
> > > missing ',' after '}'
> > 
> > Is this a problem? The last ',' is not needed by the C standard.
> 
> The problem arises when someone wants to append another item to the
> list. Without the comma he has to change two lines which may cause
> merge conflicts if two people add different items to the same struct.
> 
> That's why we usually have (unnecessary) commas on the last element of
> a struct initializer (except when they are meant to be the last
> element of course).

Good point.

Best regards,
Marek Vasut

^ permalink raw reply

* Re: [PATCH 2/3] ARM: mxs: crypto: Add Freescale MXS DCP driver
From: Marek Vasut @ 2013-09-29 22:09 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: linux-crypto, Herbert Xu, linux-arm-kernel@lists.infradead.org,
	David S. Miller
In-Reply-To: <CAOMZO5A4rH_id25ROU_CdiOd79MhtU9J3jtFVyqeeD0R+=EuTw@mail.gmail.com>

Dear Fabio Estevam,

[...]

> > + * There can even be only one instance of the MXS DCP due to the
> > + * design of Linux Crypto API.
> 
> Is this true? Usually we don't want to create a global struct.

Yeah, unfortunatelly :-(

> 
> > +
> > +/* AES 128 ECB and AES 128 CBC */
> > +static struct crypto_alg dcp_aes_algs[] = {
> > +       [0] = {
> 
> No need for explicitely add this [0]

I wanted to be explicit here, but OK.

[...]

Rest is fixed, thanks!

Best regards,
Marek Vasut

^ permalink raw reply

* Re: [PATCH 3/3] ARM: mxs: dts: Enable DCP for MXS
From: Marek Vasut @ 2013-09-29 22:11 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: linux-crypto, Fabio Estevam, Herbert Xu, Shawn Guo,
	David S. Miller, linux-arm-kernel
In-Reply-To: <21060.7210.127710.800708@ipc1.ka-ro>

Dear Lothar Waßmann,

> Hi,
> 
> Marek Vasut writes:
> > Enable the DCP by default on both i.MX23 and i.MX28.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > Cc: David S. Miller <davem@davemloft.net>
> > Cc: Fabio Estevam <fabio.estevam@freescale.com>
> > Cc: Shawn Guo <shawn.guo@linaro.org>
> > To: linux-crypto@vger.kernel.org
> > ---
> > 
> >  arch/arm/boot/dts/imx23.dtsi | 4 +++-
> >  arch/arm/boot/dts/imx28.dtsi | 5 +++--
> >  2 files changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/imx23.dtsi b/arch/arm/boot/dts/imx23.dtsi
> > index 87faa6e..0630a9a 100644
> > --- a/arch/arm/boot/dts/imx23.dtsi
> > +++ b/arch/arm/boot/dts/imx23.dtsi
> > @@ -337,8 +337,10 @@
> > 
> >  			};
> >  			
> >  			dcp@80028000 {
> > 
> > +				compatible = "fsl,mxs-dcp";
> > 
> >  				reg = <0x80028000 0x2000>;
> > 
> > -				status = "disabled";
> > +				interrupts = <53 54>;
> > +				status = "okay";
> > 
> >  			};
> 
> AFAICT the policy seems to be that nodes, that are always enabled
> don't get a 'status' property at all.

Is this documented somewhere ?

Best regards,
Marek Vasut

^ permalink raw reply

* [PATCH 1/2] crypto: sahara: Remove redundant of_match_ptr
From: Sachin Kamat @ 2013-09-30  3:19 UTC (permalink / raw)
  To: linux-crypto; +Cc: herbert, sachin.kamat, Javier Martin

The data structure of_match_ptr() protects is always compiled in.
Hence of_match_ptr() is not needed.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Cc: Javier Martin <javier.martin@vista-silicon.com>
---
 drivers/crypto/sahara.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/sahara.c b/drivers/crypto/sahara.c
index d7bb8ba..785a9de 100644
--- a/drivers/crypto/sahara.c
+++ b/drivers/crypto/sahara.c
@@ -1058,7 +1058,7 @@ static struct platform_driver sahara_driver = {
 	.driver		= {
 		.name	= SAHARA_NAME,
 		.owner	= THIS_MODULE,
-		.of_match_table = of_match_ptr(sahara_dt_ids),
+		.of_match_table = sahara_dt_ids,
 	},
 	.id_table = sahara_platform_ids,
 };
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 2/2] crypto: mv_cesa: Remove redundant of_match_ptr
From: Sachin Kamat @ 2013-09-30  3:19 UTC (permalink / raw)
  To: linux-crypto; +Cc: herbert, sachin.kamat, Sebastian Andrzej Siewior
In-Reply-To: <1380511182-18424-1-git-send-email-sachin.kamat@linaro.org>

The data structure of_match_ptr() protects is always compiled in.
Hence of_match_ptr() is not needed.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Cc: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
---
 drivers/crypto/mv_cesa.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/mv_cesa.c b/drivers/crypto/mv_cesa.c
index f9b9560..2662d1b 100644
--- a/drivers/crypto/mv_cesa.c
+++ b/drivers/crypto/mv_cesa.c
@@ -1187,7 +1187,7 @@ static struct platform_driver marvell_crypto = {
 	.driver		= {
 		.owner	= THIS_MODULE,
 		.name	= "mv_crypto",
-		.of_match_table = of_match_ptr(mv_cesa_of_match_table),
+		.of_match_table = mv_cesa_of_match_table,
 	},
 };
 MODULE_ALIAS("platform:mv_crypto");
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH 19/51] DMA-API: media: dt3155v4l: replace dma_set_mask()+dma_set_coherent_mask() with new helper
From: Hans Verkuil @ 2013-09-30 11:57 UTC (permalink / raw)
  To: Russell King
  Cc: alsa-devel, b43-dev, devel, devicetree, dri-devel, e1000-devel,
	linux-arm-kernel, linux-crypto, linux-doc, linux-fbdev, linux-ide,
	linux-media, linux-mmc, linux-nvme, linux-omap, linuxppc-dev,
	linux-samsung-soc, linux-scsi, linux-tegra, linux-usb,
	linux-wireless, netdev, Solarflare linux maintainers,
	uclinux-dist-devel, Mauro Carvalho Chehab, Greg Kroah-Hartman
In-Reply-To: <E1VMm13-0007hO-9l@rmk-PC.arm.linux.org.uk>

On 09/19/2013 11:44 PM, Russell King wrote:
> Replace the following sequence:
> 
> 	dma_set_mask(dev, mask);
> 	dma_set_coherent_mask(dev, mask);
> 
> with a call to the new helper dma_set_mask_and_coherent().
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> ---
>  drivers/staging/media/dt3155v4l/dt3155v4l.c |    5 +----
>  1 files changed, 1 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/dt3155v4l/dt3155v4l.c b/drivers/staging/media/dt3155v4l/dt3155v4l.c
> index 90d6ac4..081407b 100644
> --- a/drivers/staging/media/dt3155v4l/dt3155v4l.c
> +++ b/drivers/staging/media/dt3155v4l/dt3155v4l.c
> @@ -901,10 +901,7 @@ dt3155_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	int err;
>  	struct dt3155_priv *pd;
>  
> -	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
> -	if (err)
> -		return -ENODEV;
> -	err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> +	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>  	if (err)
>  		return -ENODEV;
>  	pd = kzalloc(sizeof(*pd), GFP_KERNEL);
> 

^ permalink raw reply

* [PATCH] sha256_ssse3: also test for BMI2
From: oliver @ 2013-10-01 12:34 UTC (permalink / raw)
  To: linux-crypto, herbert, davem; +Cc: Oliver Neukum

From: Oliver Neukum <oneukum@suse.de>

The AVX2 implementation also uses BMI2 instructions,
but doesn't test for their availability. The assumption
that AVX2 and BMI2 always go together is false. Some
Haswells have AVX2 but not BMI2.

Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
 arch/x86/crypto/sha256_ssse3_glue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/crypto/sha256_ssse3_glue.c b/arch/x86/crypto/sha256_ssse3_glue.c
index 50226c4..e52947f 100644
--- a/arch/x86/crypto/sha256_ssse3_glue.c
+++ b/arch/x86/crypto/sha256_ssse3_glue.c
@@ -281,7 +281,7 @@ static int __init sha256_ssse3_mod_init(void)
 	/* allow AVX to override SSSE3, it's a little faster */
 	if (avx_usable()) {
 #ifdef CONFIG_AS_AVX2
-		if (boot_cpu_has(X86_FEATURE_AVX2))
+		if (boot_cpu_has(X86_FEATURE_AVX2) && boot_cpu_has(X86_FEATURE_BMI2))
 			sha256_transform_asm = sha256_transform_rorx;
 		else
 #endif
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH] padata: make the sequence counter an atomic_t
From: Mathias Krause @ 2013-10-02 13:40 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: linux-crypto, Mathias Krause

Using a spinlock to atomically increase a counter sounds wrong -- we've
atomic_t for this!

Also move 'seq_nr' to a different cache line than 'lock' to reduce cache
line trashing. This has the nice side effect of decreasing the size of
struct parallel_data from 192 to 128 bytes for a x86-64 build, e.g.
occupying only two instead of three cache lines.

Those changes results in a 5% performance increase on an IPsec test run
using pcrypt.

Btw. the seq_lock spinlock was never explicitly initialized -- one more
reason to get rid of it.

Signed-off-by: Mathias Krause <mathias.krause@secunet.com>
---
 include/linux/padata.h |    3 +--
 kernel/padata.c        |    9 ++++-----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/include/linux/padata.h b/include/linux/padata.h
index 86292be..4386946 100644
--- a/include/linux/padata.h
+++ b/include/linux/padata.h
@@ -129,10 +129,9 @@ struct parallel_data {
 	struct padata_serial_queue	__percpu *squeue;
 	atomic_t			reorder_objects;
 	atomic_t			refcnt;
+	atomic_t			seq_nr;
 	struct padata_cpumask		cpumask;
 	spinlock_t                      lock ____cacheline_aligned;
-	spinlock_t                      seq_lock;
-	unsigned int			seq_nr;
 	unsigned int			processed;
 	struct timer_list		timer;
 };
diff --git a/kernel/padata.c b/kernel/padata.c
index 07af2c9..2abd25d 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -46,6 +46,7 @@ static int padata_index_to_cpu(struct parallel_data *pd, int cpu_index)
 
 static int padata_cpu_hash(struct parallel_data *pd)
 {
+	unsigned int seq_nr;
 	int cpu_index;
 
 	/*
@@ -53,10 +54,8 @@ static int padata_cpu_hash(struct parallel_data *pd)
 	 * seq_nr mod. number of cpus in use.
 	 */
 
-	spin_lock(&pd->seq_lock);
-	cpu_index =  pd->seq_nr % cpumask_weight(pd->cpumask.pcpu);
-	pd->seq_nr++;
-	spin_unlock(&pd->seq_lock);
+	seq_nr = atomic_inc_return(&pd->seq_nr);
+	cpu_index = seq_nr % cpumask_weight(pd->cpumask.pcpu);
 
 	return padata_index_to_cpu(pd, cpu_index);
 }
@@ -429,7 +428,7 @@ static struct parallel_data *padata_alloc_pd(struct padata_instance *pinst,
 	padata_init_pqueues(pd);
 	padata_init_squeues(pd);
 	setup_timer(&pd->timer, padata_reorder_timer, (unsigned long)pd);
-	pd->seq_nr = 0;
+	atomic_set(&pd->seq_nr, -1);
 	atomic_set(&pd->reorder_objects, 0);
 	atomic_set(&pd->refcnt, 0);
 	pd->pinst = pinst;
-- 
1.7.2.5

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox