[Cryptodev-linux-devel] cryptodev-linux 1.9

[Phil Sutter]

Hi,

I just released current master as new version 1.9. It is comprised of
the following commits in addition to the previous release:

* fix benchmarks linking
* fix Makefile to allow parallel make with -j option
* use Linux kernel conventions for Makefile variables
* for consistency, use $(...) instead of ${...} in makefiles
* fix clean-up on error path for crypto_create_session
* remove code duplication in cryptodev_hash_init
* add separate target for building tests
* fix destination for staged installs
* add install target for tests
* fix comment typo
* avoid calls to kmalloc on hotpaths
* avoid redundant checks in cryptodev_hash_deinit
* Fix test compile time warnings
* Support skcipher in addition to ablkcipher API
* Adjust to recent user page API changes
* Adjust to another change in the user page API
* fix issues with install target
* setting KERNEL_DIR is not necessary to build tests
* fix ignored SIGALRM signals on some platforms
* fix incorrect return code in case of error from openssl_cioccrypt
* remove not used local variables
* fix warnings of "implicit declaration of function" in async_speed
* rename header file to clarify purpose
* use buf_align macro to reduce code duplication
* avoid implicit conversion between signed and unsigned char
* do more strict code checking to avoid maintenance issues
* adjust to API changes in kernel >=4.10
* zc: Use the power of #elif
* Fix ablkcipher algorithms usage in v4.8+ kernels

Many thanks to all who contributed in order to keep this project alive
and up to date with recent Linux versions. This would not be possible
without your help!

Sadly, GNA has announced[1] it will go out of service quite soon. Since
we moved the Git repository to Github already, this affects us in only
two aspects, namely the mailing list and the public download area
(which, unlike Github, provides GPG signatures for the release
tarballs).

In order to overcome this, I have created a tarball mirror at my own
server, available here[2]. Since I happen to maintain my own instance of
Mailman as well, I am considering to replicate this mailing list there,
including all subscribers. I hope this won't cause any inconveniences.
If you see a problem with it, feel free to contact me so we can sort
things out (off-list is fine, of course).

Cheers, Phil

[1] https://mail.gna.org/public/project/2016-11/msg1.html
[2] http://nwl.cc/pub/cryptodev-linux/

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

[Cryptodev-linux-devel] tests/cipher-gcm crashes kernel

[Phil Sutter]

Hi,

Testing cryptodev on a recent kernel (4.10.0), I get an oops when
running tests/cipher-gcm. It fails with the last entry in
aes_gcm_vectors (the one with auth data).

I couldn't find out yet where the problem comes from, but looking at
cryptodev_cipher_auth(), it seems like we miss attaching auth_sg to the
request for newer kernels.

Looking at the kernel changes introduced by commit 81c4c35eb61a6
("crypto: ccm - Convert to new AEAD interface"), it seems that auth_sg
has to be appended to src_sg via sg_chain(). Though that might be
unrelated to the actual crash cause.

Could you have a look?

Thanks, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH] Fix ablkcipher algorithms usage in v4.8+ kernels

[Phil Sutter]

On Thu, Feb 09, 2017 at 11:47:16AM +, Horia Geantă wrote:
> On 2/9/2017 12:35 PM, Phil Sutter wrote:
> > Hi,
> > 
> > On Wed, Feb 08, 2017 at 04:41:50PM +, Horia Geantă wrote:
> >> Phil?
> > 
> > I intentionally left this out since (according to my INBOX) there are
> > open questions from Michael. Did you sort this out in private?
> > 
> It seems that somehow you've been removed from Cc.

Oh, sorry. I missed that.

> The discussion is in the archives:
> https://mail.gna.org/public/cryptodev-linux-devel/2016-11/msg3.html
> https://mail.gna.org/public/cryptodev-linux-devel/2016-11/msg4.html
> https://mail.gna.org/public/cryptodev-linux-devel/2016-11/msg5.html
> and AFAICT the conclusion was that the patch is needed.

OK, cool. Patch applied, thanks!

Cheers, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH] adjust to API changes in kernel >=4.10

[Phil Sutter]

Hi,

On Thu, Feb 09, 2017 at 08:28:06AM +, Cristian Stoica wrote:
> I'm sure I can find an explanation of why I've written it that way but as I 
> look at your version I wonder why I didn't recheck my assumptions and 
> simplify it further.
> 
> Anyway, Phil  can apply your suggested version since it's probably more clear.

DONE. Thanks for the heads-up!

Cheers, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH] Fix ablkcipher algorithms usage in v4.8+ kernels

[Phil Sutter]

Hi,

On Wed, Feb 08, 2017 at 04:41:50PM +, Horia Geantă wrote:
> Phil?

I intentionally left this out since (according to my INBOX) there are
open questions from Michael. Did you sort this out in private?

Cheers, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] Question regarding cryptodev using hardware acceleration for FIPS validation

[Phil Sutter]

Hi,

On Thu, Feb 02, 2017 at 06:53:55AM -0800, Chirag Shahani wrote:
> Thanks for your reply.
> 
> Yes. The QAT 1.5 module is loaded. I verified that by:
> 
> cat /proc/icp_c2xxx_dev0/version
> +--+
> | Hardware and Software versions for device 0
> |
> +--+
> Hardware Version: B0 SKU1
> Firmware Version:
> 1.3.0
> MMP Version:
> 1.0.0
> Driver Version:
> 1.3.0
> +--+
> 
> lsmod also shows the o/p, that the driver is loaded.
> lsmod | grep icp
> The following is displayed:
> # icp_qat_netkey 6748 0
> # icp_qa_al 1334748 2 icp_qat_netkey
> 
> Also, when I started IPSec traffic, the counters were increasing.
> 
> How can I confirm that the upstream has support for C2xxx?
> It would be great if you can point me to the piece of code in upstream
> which has support for any other acceleration. I can maybe research from
> there on how can I add support for C2xxx?

To see whether some accelerated algorithm is in use, have a look at
/proc/crypto (ideally compare before loading the module and after your
IPsec test). QAT drivers reside within drivers/crypto/qat within Linux
kernel sources. You might want to have a look if something in there
supports your engine.

Cheers, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] Question regarding cryptodev using hardware acceleration for FIPS validation

[Phil Sutter]

Hi Chirag,

On Tue, Jan 31, 2017 at 10:58:15AM -0800, Chirag Shahani wrote:
[...]
> However, when I running the same user space program on a box which has SoC
> C2000 co-processor (QAT-1.5), I was expecting the code to* NOT *print
> printf("Note: This is not an accelerated cipher\n").
> 
> 
> 
> *if (!(siop.flags & SIOP_FLAG_KERNEL_DRIVER_ONLY)) {
> printf("Note: This is not an accelerated cipher\n");}*
> 
> Could anyone please let me know what I am missing and what is required to
> make sure the accelerated cipher is used iso of the linux kernel to do the
> encrypt/ decrypt functions.

Looks like you're missing a kernel driver for the crypto engine. Is any
of the qat_* modules loaded? Do you see any accelerated ciphers in
/proc/crypto? I didn't have a closer look, but it looks like upstream
doesn't support the C2xxx series at all.

Cheers, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH 6/6] setting KERNEL_DIR is not necessary to build tests

[Phil Sutter]

On Wed, Oct 26, 2016 at 02:09:29PM +0300, Cristian Stoica wrote:
> Signed-off-by: Cristian Stoica 
> ---
>  tests/Makefile | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/tests/Makefile b/tests/Makefile
> index 49959de..2502f32 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -1,5 +1,3 @@
> -KERNEL_DIR ?= /lib/modules/$(shell uname -r)/build
> -KBUILD_CFLAGS += -I.. $(CRYPTODEV_CFLAGS)
>  CFLAGS += -I.. $(CRYPTODEV_CFLAGS) -Wall -Werror
>  
>  comp_progs := cipher_comp hash_comp hmac_comp

Applied (and fixed the conflict due to missing previous patches),
thanks!

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH 3/6] remove not used local variables

[Phil Sutter]

On Wed, Oct 26, 2016 at 02:09:26PM +0300, Cristian Stoica wrote:
> Signed-off-by: Cristian Stoica 
> ---
>  tests/async_hmac.c  | 2 --
>  tests/cipher_comp.c | 2 +-
>  tests/hash_comp.c   | 5 ++---
>  tests/hmac.c| 2 --
>  tests/sha_speed.c   | 1 -
>  tests/speed.c   | 1 -
>  6 files changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/async_hmac.c b/tests/async_hmac.c
> index 94a02c0..85d19c6 100644
> --- a/tests/async_hmac.c
> +++ b/tests/async_hmac.c
> @@ -185,8 +185,6 @@ test_extras(int cfd)
>   struct session_op sess;
>   struct crypt_op cryp;
>   uint8_t mac[AALG_MAX_RESULT_LEN];
> - uint8_t oldmac[AALG_MAX_RESULT_LEN];
> - uint8_t md5_hmac_out[] = 
> "\x75\x0c\x78\x3e\x6a\xb0\xb5\x03\xea\xa8\x6e\x31\x0a\x5d\xb7\x38";
>   uint8_t sha1_out[] = 
> "\x8f\x82\x03\x94\xf9\x53\x35\x18\x20\x45\xda\x24\xf3\x4d\xe5\x2b\xf8\xbc\x34\x32";
>   int i;
>  
> diff --git a/tests/cipher_comp.c b/tests/cipher_comp.c
> index 03f67bf..dbf9977 100644
> --- a/tests/cipher_comp.c
> +++ b/tests/cipher_comp.c
> @@ -33,7 +33,7 @@ test_crypto(int cfd, struct session_op *sess, int datalen)
>  
>   struct crypt_op cryp;
>  
> - int ret = 0, fail = 0;
> + int ret = 0;
>  
>   data = malloc(datalen);
>   encrypted = malloc(datalen);
> diff --git a/tests/hash_comp.c b/tests/hash_comp.c
> index e6a4346..73f85ed 100644
> --- a/tests/hash_comp.c
> +++ b/tests/hash_comp.c
> @@ -36,7 +36,7 @@ test_crypto(int cfd, struct session_op *sess, int datalen)
>  
>   struct crypt_op cryp;
>  
> - int ret = 0, fail = 0;
> + int ret = 0;
>  
>   data = malloc(datalen);
>   memset(data, datalen & 0xff, datalen);
> @@ -66,7 +66,7 @@ test_crypto(int cfd, struct session_op *sess, int datalen)
>  
>   if (memcmp(mac, mac_comp, AALG_MAX_RESULT_LEN)) {
>   printf("fail for datalen %d, MACs do not match!\n", datalen);
> - fail = 1;
> + ret = 1;
>   printf("wrong mac: ");
>   printhex(mac, 20);
>   printf("right mac: ");
> @@ -88,7 +88,6 @@ main(int argc, char **argv)
>   struct session_op sess;
>   int datalen = BLOCK_SIZE;
>   int datalen_end = MAX_DATALEN;
> - int i;
>  
>   if (argc > 1) {
>   datalen = min(max(atoi(argv[1]), BLOCK_SIZE), MAX_DATALEN);
> diff --git a/tests/hmac.c b/tests/hmac.c
> index 80a2c42..3b248f3 100644
> --- a/tests/hmac.c
> +++ b/tests/hmac.c
> @@ -212,8 +212,6 @@ test_extras(int cfd)
>  #endif
>   struct crypt_op cryp;
>   uint8_t mac[AALG_MAX_RESULT_LEN];
> - uint8_t oldmac[AALG_MAX_RESULT_LEN];
> - uint8_t md5_hmac_out[] = 
> "\x75\x0c\x78\x3e\x6a\xb0\xb5\x03\xea\xa8\x6e\x31\x0a\x5d\xb7\x38";
>   uint8_t sha1_out[] = 
> "\x8f\x82\x03\x94\xf9\x53\x35\x18\x20\x45\xda\x24\xf3\x4d\xe5\x2b\xf8\xbc\x34\x32";
>   int i;
>  
> diff --git a/tests/sha_speed.c b/tests/sha_speed.c
> index 5034b58..1e67260 100644
> --- a/tests/sha_speed.c
> +++ b/tests/sha_speed.c
> @@ -131,7 +131,6 @@ int main(void)
>  {
>   int fd, i, fdc = -1, alignmask = 0;
>   struct session_op sess;
> - char keybuf[32];
>  #ifdef CIOCGSESSINFO
>   struct session_info_op siop;
>  #endif
> diff --git a/tests/speed.c b/tests/speed.c
> index 0b40bcf..84a5bf0 100644
> --- a/tests/speed.c
> +++ b/tests/speed.c
> @@ -76,7 +76,6 @@ int encrypt_data(struct session_op *sess, int fdc, int 
> chunksize, int alignmask)
>  {
>   struct crypt_op cop;
>   char *buffer, iv[32];
> - uint8_t mac[HASH_MAX_LEN];
>   static int val = 23;
>   struct timeval start, end;
>   double total = 0;

The last one was actually added by you two patches before. Please get
rid of that nit and respin.

Thanks, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH 2/6] fix warnings of "implicit declaration of function" in async_speed

[Phil Sutter]

On Wed, Oct 26, 2016 at 02:09:25PM +0300, Cristian Stoica wrote:
> Signed-off-by: Cristian Stoica 
> ---
>  tests/async_speed.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/async_speed.c b/tests/async_speed.c
> index 1188599..dad5bc5 100644
> --- a/tests/async_speed.c
> +++ b/tests/async_speed.c
> @@ -26,6 +26,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  
>  #ifdef ENABLE_ASYNC

Hmm. What functions is it complaining about? On my local system with
gcc-5.4 there are no warnings?

Thanks, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH 1/6] avoid implicit conversion between signed and unsigned char

[Phil Sutter]

Hi,

On Wed, Oct 26, 2016 at 02:09:24PM +0300, Cristian Stoica wrote:
> Signed-off-by: Cristian Stoica 
> ---
>  tests/async_cipher.c | 36 ++--
>  tests/async_hmac.c   |  8 +++
>  tests/cipher-aead-srtp.c | 50 +++---
>  tests/cipher-aead.c  | 48 ++---
>  tests/cipher-gcm.c   | 62 
> 
>  tests/cipher.c   | 35 ++-
>  tests/cipher_comp.c  | 12 +-
>  tests/fullspeed.c|  3 ++-
>  tests/hash_comp.c|  8 +++
>  tests/hmac.c |  8 +++
>  tests/speed.c|  2 ++
>  11 files changed, 137 insertions(+), 135 deletions(-)
> 
> diff --git a/tests/async_cipher.c b/tests/async_cipher.c
> index 162a695..dd08403 100644
> --- a/tests/async_cipher.c
> +++ b/tests/async_cipher.c
> @@ -9,7 +9,7 @@
>  #include 
>  #include 
>  #include 
> -
> +#include 

I guess you can omit this by casting to __u8 instead, which is used in
cryptodev.h already. Any reason why uint8_t is preferred instead?

>  #include 
>  #include 
>  
> @@ -26,10 +26,10 @@ static int debug = 0;
>  static int
>  test_crypto(int cfd)
>  {
> - char plaintext_raw[DATA_SIZE + 63], *plaintext;
> - char ciphertext_raw[DATA_SIZE + 63], *ciphertext;
> - char iv[BLOCK_SIZE];
> - char key[KEY_SIZE];
> + uint8_t plaintext_raw[DATA_SIZE + 63], *plaintext;
> + uint8_t ciphertext_raw[DATA_SIZE + 63], *ciphertext;
> + uint8_t iv[BLOCK_SIZE];
> + uint8_t key[KEY_SIZE];
>  
>   struct session_op sess;
>  #ifdef CIOCGSESSINFO
> @@ -62,8 +62,8 @@ test_crypto(int cfd)
>   perror("ioctl(CIOCGSESSINFO)");
>   return 1;
>   }
> - plaintext = (char *)(((unsigned long)plaintext_raw + siop.alignmask) & 
> ~siop.alignmask);
> - ciphertext = (char *)(((unsigned long)ciphertext_raw + siop.alignmask) 
> & ~siop.alignmask);
> + plaintext = (uint8_t *)(((unsigned long)plaintext_raw + siop.alignmask) 
> & ~siop.alignmask);
> + ciphertext = (uint8_t *)(((unsigned long)ciphertext_raw + 
> siop.alignmask) & ~siop.alignmask);

I tried to fix tests/cipher.c after enabling -Wall by leaving the
variables in type char and casting at the point they are assigned to the
ioctl structs, which resulted in a smaller patch and less long lines. ;)
Would you mind giving it a try?

OTOH, the lines above cry for a macro. I think of something like:

#define buf_align(buf, align) \
((unsigned long)(buf) + (align) & ~(align))

What do you think?

[...]
> diff --git a/tests/async_hmac.c b/tests/async_hmac.c
> index 97fd0c5..94a02c0 100644
> --- a/tests/async_hmac.c
> +++ b/tests/async_hmac.c
> @@ -61,7 +61,7 @@ test_crypto(int cfd)
>  
>   cryp.ses = sess.ses;
>   cryp.len = sizeof("what do ya want for nothing?")-1;
> - cryp.src = "what do ya want for nothing?";
> + cryp.src = (uint8_t*)"what do ya want for nothing?";

Missing space between type and asterisk. (There are a few more cases,
but you get the idea.)

Thanks, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] Crypto API changes in 4.8

[Phil Sutter]

On Sat, Aug 06, 2016 at 01:22:01AM +0200, Michael Weiser wrote:
> Hi Phil,
> 
> On Fri, Aug 05, 2016 at 10:09:08PM +0200, Phil Sutter wrote:
> 
> > > I've done some groundwork, mostly research through a well known search
> > > engine and in the kernel code. What I've found is now at
> > > https://github.com/michaelweiser/cryptodev-linux/commit/30e66ed072d34ad0ca93f7457a9772519ca68de0.
> > Looks pretty reasonable. The only thing I would worry about is the
> > dropped checks in lines 210 and 228. Was there a specific reason for
> > that?
> 
> Yeah, they check for NULL pointers themselves. I've seen checks like
> those dropped in Herbert Xu's patches and looked at the code of
> skcipher_request_free() to find kzfree() which in turn contains:
> 
> if (unlikely(ZERO_OR_NULL_PTR(mem)))
> return;
> 
> See https://www.spinics.net/lists/linux-crypto/msg18154.html line 120.

Ah, I see.

> > > It compiles but I haven't had a chance to actually test it for function
> > > yet. 
> > Well, after compiling and loading the module, you could run the tests in
> > test/ subdir (note how I subtly try to push you into testing).
> 
> I expected nothing else and you've done it very smoothly. ;) I was on
> the train when I did that research and could only do some haphazard
> compiling of the module on my Mac. Now that I'm at home I've got a huge
> agenda, containing some vacation, for the next week, so I most likely
> will only get to actual testing by the week after next.

No problem, maybe I'll find time myself over the next few days.

> > > What is your take on supporting old kernel versions: Should I #ifdef for
> > > < 4.3 or can I just drop ablkcipher like I've done now?
> > Yes, sadly we have to support older kernels as well (see existing compat
> > code). That's the curse of out-of-tree modules, I fear.
> 
> Are a ton of #ifdefs alright then or do you prefer another approach?

Well, doing it as beautiful as possible is preferred, but eventually it
will boil down to a ton of #ifdefs anyway. So no need to bust a gut over
it. :)

Thanks, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH] fix the NULL input or output case for get_userbuf

[Phil Sutter]

Hi Cristian,

On Wed, Jan 20, 2016 at 02:06:49PM +, Cristian Stoica wrote:
> > I really have no idea where you're heading at with this. But without further
> > context, it doesn't make any sense to me. If you have a functional
> > enhancement up your sleeve, I suggest submitting that along with this
> > change so I get an idea of what this is all about.
> > 
> 
> Of course, these patches have some context.
> 
> I'm playing with the code to speed up hashing operations.
> For now I want to see if there is a simple design that will make better use 
> of the Linux Crypto API which has both 
> 'init-update-final' and 'digest' operations.
> One objective is to have less ioctl calls and avoid unnecessary init-updates 
> where a single digest will do.

Ah, this explains things a bit.

> Currently I have a CIOCHASH ioctl that is identical to CIOCCRYPT but only for 
> hashing. In the current form it is faster for my test cases but needs some 
> clarifications on API.
> 
> I can send some code to discuss design ideas if you are interested.

Sure, just send them as RFC to the list and we'll figure things out
together. :)

Thanks, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH] fix the NULL input or output case for get_userbuf

[Phil Sutter]

On Tue, Jan 19, 2016 at 01:38:53PM +, Cristian Stoica wrote:
> 
> 
> On 01/18/2016 11:04 PM, Phil Sutter wrote:
> > On Tue, Jan 12, 2016 at 12:08:02PM +0200, Cristian Stoica wrote:
> ...
> >> -  *src_sg = NULL; /* default to no input */
> >> -  *dst_sg = NULL; /* default to ignore output */
> >> -
> 
> >
> > This does not look correct to me. The purpose of those is to ensure
> > *src_sg and *dst_sg are initialized. Otherwise if src or dst are NULL
> > (which matches the situation of "NULL input or output" as you state),
> > they are left uninitialized by this function.
> 
> > Do you have an actual use case where this bites you?
> 
> The use-case I'm thinking is one where get_userbuf is called knowing upfront 
> that
> only one of src or dst are of interest, skipping work completely on the other 
> buffer.
> Of course, I can call get_userbuf with both output scatterlists and ignore 
> one of
> them. But I would add more flexibility to this function if it is possible.
> 
> For example:
> ret = get_userbuf(ses_ptr, src, len, NULL, 0, task, mm, _sg, NULL);
>     ^ 
> The current implementation does not permit output  to be NULL 
> even though
> it allows for input buffer to be NULL.

But it also doesn't need to, because get_userbuf() is never called this
way. In my opinion you're optimizing for a use-case which doesn't exist.
Once there is one, I'm all open for changes in that function, but until
that's the case I don't see sense in changing that function's behaviour.

> If this patch is incorrect for all cases, I'm thinking of some alternative 
> implementations:
> - the callers set the scatterlists  with NULL before calling get_userbuf
> - get_userbuf checks for non-NULL output scatterlists before writing default 
> values to them

It doesn't need to. The caller always just passes a pointer and expects
it to be initialized, either to NULL or a scatterlist which points to
the data src/dst points to. This is the determining information, and it
does the right thing in any case.

I really have no idea where you're heading at with this. But without
further context, it doesn't make any sense to me. If you have a
functional enhancement up your sleeve, I suggest submitting that along
with this change so I get an idea of what this is all about.

Cheers, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH] remove code duplication in cryptodev_hash_init

[Phil Sutter]

On Tue, Jan 12, 2016 at 10:16:18AM +0200, Cristian Stoica wrote:
> cryptodev_hash_init is concerned mostly with allocating data structures
> for hash operations.
> This patch replaces the call it makes to crypto_ahash_init with
> one to cryptodev_hash_reset to avoid code duplication. This call is made
> now outside of the original function to increase modularity.
> 
> Signed-off-by: Cristian Stoica 

This does not apply cleanly due to context changes in cryptlib.c, please
respin.

Thanks, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH] fix the NULL input or output case for get_userbuf

[Phil Sutter]

Hi,

On Tue, Jan 12, 2016 at 12:08:02PM +0200, Cristian Stoica wrote:
> NULL input or output is a valid case and we should not attempt
> to write default values to NULL input or output pointers.
> 
> Signed-off-by: Cristian Stoica 
> ---
>  zc.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/zc.c b/zc.c
> index 29b0501..d6adcc4 100644
> --- a/zc.c
> +++ b/zc.c
> @@ -176,9 +176,6 @@ int get_userbuf(struct csession *ses,
>   return 0;
>   }
>  
> - *src_sg = NULL; /* default to no input */
> - *dst_sg = NULL; /* default to ignore output */
> -

This does not look correct to me. The purpose of those is to ensure
*src_sg and *dst_sg are initialized. Otherwise if src or dst are NULL
(which matches the situation of "NULL input or output" as you state),
they are left uninitialized by this function.

Do you have an actual use case where this bites you?

Thanks, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] cryptodev fails to compile with kernel 4.04 on arch

[Phil Sutter]

Hi,

On Mon, Jun 01, 2015 at 09:55:16PM +0200, Sebastian Anding wrote:
 i tried to compile cryptodev 1.7 on arch linux at a beagle bone black.
 
 I attached a uname -a, gcc --version and make 2 compile.err and
 1 compile.err.add

This looks like a problem in your build system, maybe you are missing
some crucial headers? 1.7 builds fine here on Gentoo with Linux-4.0.4

Regards, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] Problem with OpenSSH/OpenSSL Interaction When Cryptodev is Used

[Phil Sutter]

On Wed, May 27, 2015 at 11:20:28AM +0100, Gordan Bobic wrote:
 On 2015-05-27 11:07, Phil Sutter wrote:
  Hi,
  
  On Wed, May 27, 2015 at 10:25:00AM +0100, Gordan Bobic wrote:
  ./cipher-aead-srtp
  ioctl(CIOCGSESSION): Invalid argument
  
  Not sure if this should succeed at all, at least that's not my code. ;)
  
  # ./hmac_comp
  requested cipher CRYPTO_AES_CBC and mac CRYPTO_SHA1_HMAC, got cipher
  cbc(aes) with driver mv-cbc-aes and hash hmac(sha1) with driver
  mv-hmac-sha1
  fail for datalen 0x10, MACs do not match!
  wrong mac:
  \xd7\xd1\xa6\xef\x0a\x38\xe1\x09\x45\xe1\x8b\x48\x88\xaa\xa9\x23\x4c\xd4\x67\xd1
  right mac:
  \xd7\xd1\xa6\xef\x0a\x38\xe1\x09\x45\xe1\x8b\x48\x88\xaa\xa9\x23\x4c\xd4\x67\xd1
  test_crypto() failed for datalen of 16
  
  That's bad. I'm pretty sure SSH uses HMAC. Unless there is a bug in
  hmac_comp, the fact that identical binary strings are shown for actual
  and expected result makes me suspect a caching issue (the additional
  data access while printing sometimes leads to a cache flush so one does
  not see the complained difference).
 
 But - my OpenSSL is built without -DUSE_CRYPTODEV_DIGESTS, so if OpenSSH
 used OpenSSL for that, there should be no attempt to even use hardware
 digests.

Does this cover HMAC as well?

  You are using MV_CESA, therefore you are probably on a Kirkwood with
  VIVT cache architecture - which is pretty delicate when it comes to the
  zero-copy stuff involved. Please make sure your kernel doesn't miss any
  fixes in mv_cesa regarding cache flushes.
 
 That is easier said than done. I'm using the upstream vendor provided
 sources that are both ancient (3.4.6) and heavily patched (the diff
 between mainline 3.4.6 and their kernel is MBs).

Sounds familiar. For mv_cesa though, it should be possible to just take
over mainline mv_cesa.c with minimal adjustments (drop clock gating e.g.).

  Also, for the sake of testing
  you could turn off zero-copy in cryptodev as well.
 
 How? I don't see any obvious options in the Makefile to do that.
 All I see is the async option, which is disabled by default.

You can set COP_FLAG_NO_ZC in struct crypt_op. So you need to recompile
openssl. Alternatively you could change the code in cryptodev referring
to that flag so it always behaves as if it had been set.

Cheers, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] cryptodev, mv_cesa sha1

[Phil Sutter]

Hello Jan,

On Mon, Apr 27, 2015 at 12:44:50PM +0200, JM wrote:
 I installed cryptodev 1.7 on my Debian box running on Marvell Feroceon
 processor (featuring mv_cesa hardware accelerator). I'm running debian
 wheezy with openssl_1.0.1e (with enabled cryptodev and the two recommended
 patches) and kernel 3.16 from backports.
 
 My problem is that SHA1 acceleration doesn't seem to work, while it should
 be supported by the engine.

Specifically the combination of cryptodev and CESA (although with
heavily modified driver) is what we use at work and does a decent job.

 If I run ./sha_speed in tests I always get the sha1-asm fallback driver,
 instead of mv-sha1.

This per se does not sound like a cryptodev problem. A few things I
would like you to check:

- Run the *_comp tools to see if cryptodev returns correct output
- same as above with mv_cesa.ko unloaded (i.e., forcing software
  implementations in kernel).
- Are the kernel crypto self-tests enabled? Maybe the one for mv-sha1
  fails?
- Enable kernel crypto self-tests if not already happening and check if
  they succeed.
- What's the priority of each sha1 provider in /proc/crypto? Although it
  shouldn't, maybe mv-sha1 is considered slower than sha1-asm?
- Since at the moment an alternative driver is out for review, are you
  using the mainline mv_cesa.c or something else?

Cheers, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] Issue with make check

[Phil Sutter]

Hi,

On Fri, Apr 10, 2015 at 11:08:10AM +0200, Filip Kubicz wrote:
 The version of cc is 
 
  cc (Ubuntu 4.8.2-18ubuntu1) 4.8.2
  Copyright (C) 2013 Free Software Foundation, Inc.

OK, looks like GCC as well. According to the GCC docs at
https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html (description of
'-llibrary'), the library names should even be specified after the
objects referring to them. So I fixed tests/Makefile, please see
upstream commit da73010.

Thanks for pointing that out,
Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] Python bindings for the cryptodev module.

[Phil Sutter]

Hi,

On Wed, Mar 04, 2015 at 05:50:35PM +0200, Tilemachos Charalampous wrote:
 Hello. I'm an undergraduate student studying electrical and computer 
 engineering at the NTUA.
 As part of an Operating Systems course, I worked a bit on the cryptodev 
 module, and needed it to use it in a Pyhton project. After searching a 
 bit on the Internet I couldn't find any cryptodev bindings, so I decided 
 to develop them myself. I was encouraged by my instructor (Vangelis 
 Koukis, Cc:ed) to develop the bindings to something more general, that 
 supports the whole cryprtodev module.

Forgot the Cc or was it blind?

 I have created two types of bindings. Static bindings and semi automated 
 bindings. Static bindings are created based on cryptodev.h and ioctl.h 
 (from my Linux system). Semi automated bindings are created using 
 ctypesgen for python. I have created a simple makefile in which 
 ctypesgen is used to create ioctl.py and cryptodev.py automatically into 
 a Python module. The reason that I could not build fully automated 
 bindings is because of a bug in ctypesgen, in which it cannot parse 
 sizeof if the argument is defined in a different header file. You can 
 see a more detailed explanation in README.md of the ctypesgen branch on 
 my repository under Why not fully automated way? (links provided at 
 the end). Also I have implemented some of your tests into Python that 
 use the bindings.
 
 If you are interested, I would like to get this code included in 
 mainline cryptodev. All my changes currently live at 
 https://github.com/tchar/cryptodev-python/.
 Also the link for README.md, which explains the ctypesgen bug I 
 mentioned is 
 https://github.com/tchar/cryptodev-python/blob/ctypesgen/README.md
 The link for the ctypesgen module is https://code.google.com/p/ctypesgen/

Looks nice. How about submitting a patch? Do we have to keep the python
bindings themselfs or is it possible to have them generated upon 'make
dist'? I would prefer not having to keep them in sync manually. Maybe
this is related to your mention of 'semi automated bindings', but I
don't get it due to lack of experience in python/C interfaces.

Cheers, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] cryptodev-linux 1.7

[Phil Sutter]

I *knew* it.

On Sat, Feb 07, 2015 at 11:01:05PM +0100, Phil Sutter wrote:
 I have eventually managed to release a new version of cryptodev-linux.
 As almost a year has passed since the last release, the list of changes
 is a rather long one:

There is the bug already: It's 2015, so almost two years have passed. Of
course I consistently messed up the release date in NEWS as well.

Cheers, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

[Cryptodev-linux-devel] cryptodev-linux 1.7

[Phil Sutter]

Hi,

I have eventually managed to release a new version of cryptodev-linux.
As almost a year has passed since the last release, the list of changes
is a rather long one:

* Added support for composite AEAD keys by Cristian Stoica.

* Added support for sysctl to modify verbosity by Nikolaos Tsakalakis.

* Several bugfixes by Cristian Stoica.

* When a driver requires aligned data but unaligned are provided, then
  zero copy is disabled to prevent driver failing to encrypt.

* Compatibility to kernel version 3.13 and above by Cosmin Paraschiv.

* Various checkpatch.pl fixes.

* Introduced ddebug, dinfo, dwarning and derr macros wrapping dprintk.

* Improved support for cross-compiling.

* Hmac_comp test has become more picky when checking results.

* Fixed allocated resource cleanup in error case, patch by Cristian Stoica.

* Buffer size allocation fixup for AEAD modes by Nikos Mavrogiannopoulos.

* Support for composite AEAD keys added by Cristian Stoica.

* Fixed tag and dsl_len calculation for AEAD ciphers, patch by Cristian Stoica.

* Documentation updates by Nikos Mavrogiannopoulos.

As this is my first release as project maintainer, please bear with me
possibly messing things up. For me, the release process unfolded as
follows (not in order of actual appearance, but that detail shall remain
hidden):

- Fixed 'make dist', replacing most of the manual work in it with a
  simple call to 'git archive' while doing so.
- Incremented Makefile's VERSION variable.
- Tagged master with my own signature.
- Used 'make dist' to create a new tarball and signature, which I
  uploaded to gna.org.
- pushed master along with the new tag to Github. (Actually, I pushed
  all earlier tags by accident as well ...)

Nikos, please let me know if I forgot something. Website news entry
comes as soon as this mail is out and accessible from the archive.

Cheers, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH] fix tag and dst_len calculation for aead ciphers

[Phil Sutter]

Hi Christian,

On Mon, Aug 18, 2014 at 04:17:42PM +0300, Cristian Stoica wrote:
 Hi Phil,
 
 On 05.08.2014 01:36, Phil Sutter wrote:
  On Thu, Jul 10, 2014 at 01:32:59PM +0300, Cristian Stoica wrote:
  Calculate tag and destination buffer length in a single place to avoid
  code duplication. The TLS case is fixed by rounding the destination
  length to cipher block size.
  
  Applied, thanks. Again, sorry for the delay.
 
 The patch doesn't show up on the git repository. Let me know if you've
 just missed the push or are still reviewing so I can help you with it.

But it is there. Maybe you pull from Nikos' repository? The 'official'
upstream has moved to:
https://github.com/cryptodev-linux/cryptodev-linux

I just noticed, cryptodev-linux.org points to Nikos' clone, as well
which should be fixed.

Cheers, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH] fix tag and dst_len calculation for aead ciphers

[Phil Sutter]

On Thu, Jul 10, 2014 at 01:32:59PM +0300, Cristian Stoica wrote:
 Calculate tag and destination buffer length in a single place to avoid
 code duplication. The TLS case is fixed by rounding the destination
 length to cipher block size.

Applied, thanks. Again, sorry for the delay.

Cheers, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH 1/3] use function-local storage for cipher and hmac keys

[Phil Sutter]

On Fri, May 30, 2014 at 01:59:02PM +0300, Cristian Stoica wrote:
 Composite ciphers (cipher + hmac) use both keys at the same time. This
 patch is the first in a series that adds support for composite ciphers
 keys.
 
 Signed-off-by: Cristian Stoica cristian.sto...@freescale.com

All applied, thanks.

Cheers, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] ? about cryptosession2

[Phil Sutter]

Hi,

On Wed, Mar 05, 2014 at 10:01:16AM -0700, Brad Walker wrote:
 I noticed that on FreeBSD 7 OpenBSD  cryptodev.h defines an ioctl called:
 CIOCGSESSION2
 
 But, this is not defined in the cryptodev.h file for Linux.
 
 Curious to know why that is?

Cryptodev-linux is meant to be compatible to OCF but not all it's
extensions. Likewise, there are addons to cryptodev-linux which OCF does
not support.

Greetings, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH v2] clean-up allocated resources after a failed open

[Phil Sutter]

Hi Christian,

On Fri, Feb 14, 2014 at 09:00:07AM +, cristian.sto...@freescale.com wrote:
  Nope, that's exactly it. But in this case probably a mistake happened
  when submitting your v2, as it contains the above change.
 [] 
 The v2 patch does not touch the pcr part anymore
 (https://github.com/cryptodev-linux/cryptodev-linux/blob/master/ioctl.c#L447)
 Does the patch apply or is it defective? What change is contained in v2 that 
 shouldn't be?

After taking a *closer* look at v2 again, I realised I must have misread
that change completely, mistaking it with the old one from v1. Sorry for
the confusion!

Patch applied, thanks a lot!

Best wishes, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] cryptodev uCLinux..

[Phil Sutter]

Brad,

On Wed, Feb 12, 2014 at 06:20:31PM -0700, Brad Walker wrote:
 I am a Linux consultant working at a start-up over here in Boulder, CO. We
 are using uCLinux (based on Linux 2.6.39) on a really powerful processor
 (Freescale Kinetis) but it is resource limited.
 
 The chip has h/w acceleration for encryption. The chip support DES, 3DES,
 MD5, SHA1, and SHA256. I've gotten the glue code that talks to the hardware
 working.
 
 I installed the Cryptodev driver onto our platform and it works great!!
 
 Thought you might like to know as a point of reference.

Thanks a lot for your kudos!

Greetings, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH v2] clean-up allocated resources after a failed open

[Phil Sutter]

Hi Christian,

On Thu, Feb 13, 2014 at 07:33:32AM +, cristian.sto...@freescale.com wrote:
 Hi Phil,
 
 for (i = 0; i  DEF_COP_RINGSIZE; i++) {
 tmp = kzalloc(sizeof(struct todo_list_item), GFP_KERNEL);
 if (!tmp)
   - return -ENOMEM;
   + goto err_ringalloc;
  
  This would mean that in case pcr allocation fails, the code would try to
  destroy mutexes which haven't been initialised at all. Probably I was a
  bit unclear here: just leave the code as is, returning -ENOMEM at this
  point is perfectly fine.
 []
 That part of the patch was reverted and doesn't show up anymore in v2. If pcr 
 alloc fails,
 open returns -ENOMEM immediately as in the original (the v2 patch context 
 does not make it clear).
 If ring alloc fails, we clean-up pcr (which didn't fail) and rollback the for 
 loop. But by this
 point, the mutex-es have already been initialized and we should revert that 
 too.
 Did you have something else in mind?

Nope, that's exactly it. But in this case probably a mistake happened
when submitting your v2, as it contains the above change.

Greetings, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH v2] clean-up allocated resources after a failed open

[Phil Sutter]

Hi,

On Wed, Feb 12, 2014 at 09:52:17AM +0200, Cristian Stoica wrote:
 If 'open /dev/crypto' fails, all allocated resources must be freed
 before open returns; close can't be called to clean-up since
 there is no file descriptor after a failed open.
 
 Signed-off-by: Cristian Stoica cristian.sto...@freescale.com
 ---
  ioctl.c | 18 --
  1 file changed, 16 insertions(+), 2 deletions(-)
 
 diff --git a/ioctl.c b/ioctl.c
 index 4838870..d4e83f4 100644
 --- a/ioctl.c
 +++ b/ioctl.c
 @@ -440,7 +440,7 @@ static void cryptask_routine(struct work_struct *work)
  static int
  cryptodev_open(struct inode *inode, struct file *filp)
  {
 - struct todo_list_item *tmp;
 + struct todo_list_item *tmp, *tmp_next;
   struct crypt_priv *pcr;
   int i;
  
 @@ -466,7 +466,7 @@ cryptodev_open(struct inode *inode, struct file *filp)
   for (i = 0; i  DEF_COP_RINGSIZE; i++) {
   tmp = kzalloc(sizeof(struct todo_list_item), GFP_KERNEL);
   if (!tmp)
 - return -ENOMEM;
 + goto err_ringalloc;

This would mean that in case pcr allocation fails, the code would try to
destroy mutexes which haven't been initialised at all. Probably I was a
bit unclear here: just leave the code as is, returning -ENOMEM at this
point is perfectly fine.

   pcr-itemcount++;
   ddebug(2, allocated new item at %p, tmp);
   list_add(tmp-__hook, pcr-free.list);
 @@ -475,6 +475,20 @@ cryptodev_open(struct inode *inode, struct file *filp)
   ddebug(2, Cryptodev handle initialised, %d elements in queue,
   DEF_COP_RINGSIZE);
   return 0;
 +
 +/* In case of errors, free any memory allocated so far */
 +err_ringalloc:
 + list_for_each_entry_safe(tmp, tmp_next, pcr-free.list, __hook) {
 + list_del(tmp-__hook);
 + kfree(tmp);
 + }
 + mutex_destroy(pcr-done.lock);
 + mutex_destroy(pcr-todo.lock);
 + mutex_destroy(pcr-free.lock);
 + mutex_destroy(pcr-fcrypt.sem);
 + kfree(pcr);
 + filp-private_data = NULL;
 + return -ENOMEM;
  }
  
  static int
 -- 
 1.8.3.1
 
 

Best wishes, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH] clean-up allocated resources after a failed open

[Phil Sutter]

Hi,

Thanks for your patch, please see my comments below.

On Tue, Feb 11, 2014 at 12:43:13PM +0200, Cristian Stoica wrote:
 If 'open /dev/crypto' fails, all allocated resources must be freed
 before open returns; close can't be called to clean-up since
 there is no file descriptor after a failed open.
 
 Signed-off-by: Cristian Stoica cristian.sto...@freescale.com
 ---
  ioctl.c | 21 ++---
  1 file changed, 18 insertions(+), 3 deletions(-)
 
 diff --git a/ioctl.c b/ioctl.c
 index 4838870..8172b22 100644
 --- a/ioctl.c
 +++ b/ioctl.c
 @@ -440,13 +440,13 @@ static void cryptask_routine(struct work_struct *work)
  static int
  cryptodev_open(struct inode *inode, struct file *filp)
  {
 - struct todo_list_item *tmp;
 + struct todo_list_item *tmp, *tmp_next;
   struct crypt_priv *pcr;
   int i;
  
   pcr = kzalloc(sizeof(*pcr), GFP_KERNEL);
   if (!pcr)
 - return -ENOMEM;
 + goto open_err1;

This is unnecessary in my point of view. It adds a line of code (the
label below) and disturbs the reading flow.

   filp-private_data = pcr;
  
   mutex_init(pcr-fcrypt.sem);
 @@ -466,7 +466,8 @@ cryptodev_open(struct inode *inode, struct file *filp)
   for (i = 0; i  DEF_COP_RINGSIZE; i++) {
   tmp = kzalloc(sizeof(struct todo_list_item), GFP_KERNEL);
   if (!tmp)
 - return -ENOMEM;
 + goto open_err2;
 +

Please use a more descriptive label, like err_ringalloc or so. In this
case it's not as important, but especially with many labels just
numbering them makes it hard to remember what came from where.

Do you think the added empty line helps readability?

   pcr-itemcount++;
   ddebug(2, allocated new item at %p, tmp);
   list_add(tmp-__hook, pcr-free.list);
 @@ -475,6 +476,20 @@ cryptodev_open(struct inode *inode, struct file *filp)
   ddebug(2, Cryptodev handle initialised, %d elements in queue,
   DEF_COP_RINGSIZE);
   return 0;
 +
 +/* In case of errors, free any memory allocated so far */
 +open_err2:
 + list_for_each_entry_safe(tmp, tmp_next, pcr-free.list, __hook) {
 + list_del(tmp-__hook);
 + kfree(tmp);
 + }
 + mutex_destroy(pcr-done.lock);
 + mutex_destroy(pcr-todo.lock);
 + mutex_destroy(pcr-free.lock);
 + mutex_destroy(pcr-fcrypt.sem);
 + kfree(pcr);

Not sure if it's a must-have, but cryptodev_release zeroes
filp-private_data in the end. Maybe duplicate it's semantics here or
abandon entirely?

 +open_err1:
 + return -ENOMEM;
  }
  
  static int

Best wishes, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [RFC 1/9] fix private data memleak

[Phil Sutter]

Hey Christian,

On Mon, Jan 27, 2014 at 07:53:56AM +, cristian.sto...@freescale.com wrote:
  Why should that be necessary?
 The patch is concerned with 'close' being called with garbage input.
 I'm not sure about the call chain here: is 'close' never called when 'open' 
 fails? What is the assumption of the caller?

Suppose you write some simple program that writes to a file. You use
open() to get the fd, which fails (for whatever reason). Would you then
call close()? :)

  I just did a quick test, simply assigning
  NULL to pcr instead of the kzalloc call - despite filp-private_data
  being garbage at the beginning of cryptodev_open, cryptodev_release is
  not being called when it fails with -ENOMEM.
 [] 
 This looks like the caller assumes that nothing ever happened if 'open' 
 failed. In that case, any cleanup should be done in 'open'.

What else should she assume? I bet you wouldn't either (see above).
Actually, it's quite simple: open() returns a file descriptor, or -1 if
the call fails. In the later case, there simply is no descriptor to
close, no matter what the caller assumes.

  Instead, I see a more prominent potential memleak here: If one of the
  todo list item allocations fails, nothing is being freed. There probably
  should be a bunch of gotos to the end of the function to undo stuff that
  has been done so far. What do you think?
 [] 
 Based on your observations, all clean-up should be done in 'open' to avoid 
 the possibility of memleaks because 'close' is not called.
 However, if 'close' is ever called with garbage, then it will fail - so we 
 should cover that case too. It's one thing less to think about.

I think we can savely assume that 'close' (or 'cryptodev_release' to be
precise) is always called for a file descriptor (struct file in our
case) that was opened successfully. How would you distinguish between
valid data and garbage anyway? Say, filp-private_data is 0xdeadbeef, is
that garbage or the address of allocated data?

But back to the topic: for cryptodev_open(), I suggest using the
demonised 'goto' like so:

| if (!init_a())
|   goto fail_a;
| if (!init_b())
|   goto fail_b;
| return 0;
| fail_b:
|   deinit_a();
| fail_a:
|   return -1;

Feel free to give it a go, I'll happily review your patch. Otherwise let
me know and I'll make something up on my own.

Greetings, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH v2] Replace INIT_COMPLETION with reinit_completion.

[Phil Sutter]

On Thu, Jan 16, 2014 at 12:18:08PM +0200, Horia Geanta wrote:
 From: Cosmin Paraschiv cosmin.parasc...@freescale.com
 
 In the 3.13-rc1 Linux kernel, the INIT_COMPLETION macro has been replaced
 with an inline function, reinit_completion [1][2]. We are currently
 using the 3.13-rc3 Linux kernel, which leads to the following error:
 
 cryptlib.c:279:2: error: implicit declaration of function 'INIT_COMPLETION' 
 [-Werror=implicit-function-declaration]
   INIT_COMPLETION(cdata-async.result-completion);
 
 [1] 
 https://github.com/torvalds/linux/commit/c32f74ab2872994bc8336ed367313da3139350ca
 [2] 
 https://github.com/torvalds/linux/commit/62026aedaacedbe1ffe94a3599ad4acd8ecdf587

Applied, thanks a lot.

Greetings, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH] Replace INIT_COMPLETION with reinit_completion.

[Phil Sutter]

Hi Christian,

On Tue, Jan 21, 2014 at 08:26:35AM +, cristian.sto...@freescale.com wrote:
 Hi Phil,
 
  Wouldn't it be better to conditionally define INIT_COMPLETION to
  reinit_completion for newer kernels? Doesn't this break downwards
  compatibility?
  
  I could think of something like:
  
  #if (LINUX_VERSION_CODE = KERNEL_VERSION(3, 12, 0))
  #  define INIT_COMPLETION(x) reinit_completion(%(x))
  #endif
 
 
 Do you have any comments on the v2 of the patch?
 https://mail.gna.org/public/cryptodev-linux-devel/2014-01/msg4.html

Looking good so far. I just compile-tested against 3.10 and 3.13 without
problems. If you don't mind, I'll leave the patch sitting for a few more
days to allow for more discussion (if any at all).

Greetings, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH] Replace INIT_COMPLETION with reinit_completion.

[Phil Sutter]

Hi,

On Wed, Jan 15, 2014 at 05:22:18PM +0200, Horia Geanta wrote:
 From: Cosmin Paraschiv cosmin.parasc...@freescale.com
 
 In the 3.12 Linux kernel, the INIT_COMPLETION macro has been replaced
 with an inline function, reinit_completion [1][2]. We are currently
 using the 3.13-rc3 Linux kernel, which leads to the following error:
 
 cryptlib.c:220:2: error: implicit declaration of function 'INIT_COMPLETION' 
 [-Werror=implicit-function-declaration]
   INIT_COMPLETION(cdata-async.result-completion);
 
 [1] 
 https://github.com/torvalds/linux/commit/c32f74ab2872994bc8336ed367313da3139350ca
 [2] 
 https://github.com/torvalds/linux/commit/62026aedaacedbe1ffe94a3599ad4acd8ecdf587
 
 Signed-off-by: Cosmin Paraschiv cosmin.parasc...@freescale.com
 Reviewed-by: Cristian Stoica cristian.sto...@freescale.com
 Tested-by: Cristian Stoica cristian.sto...@freescale.com
 Signed-off-by: Horia Geanta horia.gea...@freescale.com
 ---
  cryptlib.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)
 
 diff --git a/cryptlib.c b/cryptlib.c
 index 54d5d41..a923c14 100644
 --- a/cryptlib.c
 +++ b/cryptlib.c
 @@ -217,7 +217,7 @@ ssize_t cryptodev_cipher_encrypt(struct cipher_data 
 *cdata,
  {
   int ret;
  
 - INIT_COMPLETION(cdata-async.result-completion);
 + reinit_completion(cdata-async.result-completion);

Wouldn't it be better to conditionally define INIT_COMPLETION to
reinit_completion for newer kernels? Doesn't this break downwards
compatibility?

I could think of something like:

#if (LINUX_VERSION_CODE = KERNEL_VERSION(3, 12, 0))
#  define INIT_COMPLETION(x) reinit_completion(%(x))
#endif

Greetings, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH] Replace INIT_COMPLETION with reinit_completion.

[Phil Sutter]

On Wed, Jan 15, 2014 at 06:21:30PM +0200, Horia Geantă wrote:
 On 1/15/2014 5:46 PM, Phil Sutter wrote:
  Hi,
 
  On Wed, Jan 15, 2014 at 05:22:18PM +0200, Horia Geanta wrote:
  From: Cosmin Paraschiv cosmin.parasc...@freescale.com
 
  In the 3.12 Linux kernel, the INIT_COMPLETION macro has been replaced
  with an inline function, reinit_completion [1][2]. We are currently
  using the 3.13-rc3 Linux kernel, which leads to the following error:
 
  cryptlib.c:220:2: error: implicit declaration of function 
  'INIT_COMPLETION' [-Werror=implicit-function-declaration]
 INIT_COMPLETION(cdata-async.result-completion);
 
  [1] 
  https://github.com/torvalds/linux/commit/c32f74ab2872994bc8336ed367313da3139350ca
  [2] 
  https://github.com/torvalds/linux/commit/62026aedaacedbe1ffe94a3599ad4acd8ecdf587
 
  Signed-off-by: Cosmin Paraschiv cosmin.parasc...@freescale.com
  Reviewed-by: Cristian Stoica cristian.sto...@freescale.com
  Tested-by: Cristian Stoica cristian.sto...@freescale.com
  Signed-off-by: Horia Geanta horia.gea...@freescale.com
  ---
cryptlib.c | 8 
1 file changed, 4 insertions(+), 4 deletions(-)
 
  diff --git a/cryptlib.c b/cryptlib.c
  index 54d5d41..a923c14 100644
  --- a/cryptlib.c
  +++ b/cryptlib.c
  @@ -217,7 +217,7 @@ ssize_t cryptodev_cipher_encrypt(struct cipher_data 
  *cdata,
{
 int ret;
 
  -  INIT_COMPLETION(cdata-async.result-completion);
  +  reinit_completion(cdata-async.result-completion);
 
  Wouldn't it be better to conditionally define INIT_COMPLETION to
  reinit_completion for newer kernels? Doesn't this break downwards
  compatibility?
 
 That's a good question.
 Before posting v2, I would like to find out what is the goal here - to 
 be compatible with kernel 2.6.x onwards or...?

To not break existing compatibility. I have no idea which is the
earliest version of Linux cryptodev-linux runs on, but the proposed
change smells like breaking cryptodev-linux for everything before 3.12,
no?

 I am sure this is not the first time changes in the kernel affect 
 out-of-tree modules.

Yes, that's why most of them are more and more cluttered with those #IF
checks over time. Probably one of the main reasons why (sane) people try
to get their kernel modules mainline sooner or later.

Greetings, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH] allow user override for kernel and installation directory

[Phil Sutter]

Hi,

On Thu, Jun 27, 2013 at 05:39:59PM -0600, Anthony Foiani wrote:
 Cristian --
 
 On Thu, Jun 27, 2013 at 7:42 AM, Cristian Stoica
 cristian.sto...@freescale.com wrote:
  - this is useful for cross-building for embedded systems
 
 This patch shouldn't be necessary: you can give variable assignments
 as arguments on the command line, and they should override basic
 assignment within the makefile.  See:

But it still makes sense, since overriding them is possible only when
passing the variable assignment as parameter to make, not via
environment variable.

Take this sample Makefile:
| foo1 = bar
| foo2 ?= bar
| 
| all:
|   @echo ${foo1} ${foo2}


Then look at the changing output:
| $ make foo1=foo foo2=foo
| foo foo
| $ foo1=foo foo2=foo make
| bar foo

Although it may be possible to overwrite the variables when calling
make, there certainly are build systems passing make variables via
environment.

Greetings, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] Asynchronous support in Cryptodev is buggy

[Phil Sutter]

Hi,

On Thu, Feb 14, 2013 at 05:20:07PM +, Dutta Yashpal-B05456 wrote:
 Looked at asynchronous support in Cryptodev-1.5 and found that it is getting 
 in following flow
 cryptask_routine calling crypto_run. Now in crypto_run API, all the 
 user-space process provided 
 buffers are either being copied to kernel buffers or they are being mapped in 
 kernel using 
 get_user_pages.
 
 Either way it is wrong because by the time cryptask_routine is executing, it 
 is running in work_queue
 kernel thread context and not in process context. So, these user-space buffer 
 are not accessible in work-queue 
 Context. In my view, asynchronous support needs to be fixed.

Have you tested this? It's working fine! Prove it wrong, then I'll take
care of it. :)

Greetings, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

[Cryptodev-linux-devel] [PATCH 2/4] zc.c: simplify code a bit by exiting early

[Phil Sutter]

Also, hiding expressions in one-liners is not worth the lines saved.

Signed-off-by: Phil Sutter phil.sut...@viprinet.com
---
 zc.c |   98 -
 1 files changed, 48 insertions(+), 50 deletions(-)

diff --git a/zc.c b/zc.c
index 773dfb4..db28b58 100644
--- a/zc.c
+++ b/zc.c
@@ -55,28 +55,28 @@ int __get_userbuf(uint8_t __user *addr, uint32_t len, int 
write,
 
if (unlikely(!pgcount || !len || !addr)) {
sg_mark_end(sg);
+   return 0;
}
-   else {
-   down_write(mm-mmap_sem);
-   ret = get_user_pages(task, mm,
-   (unsigned long)addr, pgcount, write, 0, pg, 
NULL);
-   up_write(mm-mmap_sem);
-   if (ret != pgcount)
-   return -EINVAL;
 
-   sg_init_table(sg, pgcount);
+   down_write(mm-mmap_sem);
+   ret = get_user_pages(task, mm,
+   (unsigned long)addr, pgcount, write, 0, pg, NULL);
+   up_write(mm-mmap_sem);
+   if (ret != pgcount)
+   return -EINVAL;
 
-   pglen = min((ptrdiff_t)(PAGE_SIZE - PAGEOFFSET(addr)), 
(ptrdiff_t)len);
-   sg_set_page(sg, pg[i++], pglen, PAGEOFFSET(addr));
+   sg_init_table(sg, pgcount);
 
+   pglen = min((ptrdiff_t)(PAGE_SIZE - PAGEOFFSET(addr)), (ptrdiff_t)len);
+   sg_set_page(sg, pg[i++], pglen, PAGEOFFSET(addr));
+
+   len -= pglen;
+   for (sgp = sg_next(sg); len; sgp = sg_next(sgp)) {
+   pglen = min((uint32_t)PAGE_SIZE, len);
+   sg_set_page(sgp, pg[i++], pglen, 0);
len -= pglen;
-   for (sgp = sg_next(sg); len; sgp = sg_next(sgp)) {
-   pglen = min((uint32_t)PAGE_SIZE, len);
-   sg_set_page(sgp, pg[i++], pglen, 0);
-   len -= pglen;
-   }
-   sg_mark_end(sg_last(sg, pgcount));
}
+   sg_mark_end(sg_last(sg, pgcount));
return 0;
 }
 
@@ -139,11 +139,13 @@ int get_userbuf(struct csession *ses,
 
/* Empty input is a valid option to many algorithms  is tested by 
NIST/FIPS */
/* Make sure NULL input has 0 length */
-   if (!src  src_len) { src_len = 0; }
+   if (!src  src_len)
+   src_len = 0;
 
/* I don't know that null output is ever useful, but we can handle it 
gracefully */
/* Make sure NULL output has 0 length */
-   if (!dst  dst_len) { dst_len = 0; }
+   if (!dst  dst_len)
+   dst_len = 0;
 
if (ses-alignmask  !IS_ALIGNED((unsigned long)src, ses-alignmask)) {
dprintk(2, KERN_WARNING, careful - source address %lx is not 
%d byte aligned\n,
@@ -169,7 +171,7 @@ int get_userbuf(struct csession *ses,
return rc;
}
 
-   if (src == dst) {
+   if (src == dst) {   /* inplace operation */
rc = __get_userbuf(src, src_len, 1, ses-used_pages,
   ses-pages, ses-sg, task, mm);
if (unlikely(rc)) {
@@ -178,40 +180,36 @@ int get_userbuf(struct csession *ses,
return rc;
}
(*src_sg) = (*dst_sg) = ses-sg;
+   return 0;
}
-   else {
-   const unsigned int readonly_pages = ses-readonly_pages;
-   const unsigned int writable_pages = ses-used_pages - 
readonly_pages;
-
-   if(likely(src)) {
-   rc = __get_userbuf(src, src_len, 0, readonly_pages,
-  ses-pages, ses-sg, task, 
mm);
-   if (unlikely(rc)) {
-   dprintk(1, KERN_ERR,
-   failed to get user pages for data 
input\n);
-   return rc;
-   }
-   *src_sg = ses-sg;
-   }
-   else {
-   *src_sg = NULL; // no input
-   }
 
-   if(likely(dst)) {
-   struct page **dst_pages = ses-pages + readonly_pages;
-   *dst_sg = ses-sg + readonly_pages;
-
-   rc = __get_userbuf(dst, dst_len, 1, writable_pages,
-  dst_pages, *dst_sg, task, 
mm);
-   if (unlikely(rc)) {
-   dprintk(1, KERN_ERR,
-   failed to get user pages for 
data output\n);
-   release_user_pages(ses);  /* FIXME: use 
__release_userbuf(src, ...) */
-   return rc;
-   }
+   *src_sg = NULL; // default to no input
+   *dst_sg = NULL; // default to ignore output
+
+   if(likely(src)) {
+   rc = __get_userbuf(src, src_len, 0, ses

Re: [Cryptodev-linux-devel] (no subject)

[Phil Sutter]

Hi,

On Wed, Feb 29, 2012 at 01:19:43PM +0100, Nikos Mavrogiannopoulos wrote:
 On Tue, Feb 28, 2012 at 11:56 PM, Phil Sutter p...@nwl.cc wrote:
  Another thing I just noticed, these commit-mails are somehow broken.
  E.g. backslashes are missing completely, and worse they're somehow
  interpreted (trailing ones make lines continue and stuff). Just in case
  you haven't noticed yet.
 
 I never noticed. I'll check, and try to report it to guys at repo.or.cz. If
 you have already examples in mind from the archive let me know.

No problem, take e.g. the mail containing my openssl_wrapper (Message-ID
20120228213130.af02a1472...@rover.ms.mff.cuni.cz). The printk-changes
right at the beginning are already interesting:
| -   printf(running %sn, __func__);
| +   if (debug) printf(running %sn, __func__);
here the mailer changed the '\n' at the end of the format string to 'n'.
This happens every time, your test program silencing commit is a fine
demonstration. :)

Further down that mail, at the start of the next commit there is
something like this:
|  hostprogs := cipher cipher-aead hmac speed async_cipher async_hmac 
async_speed sha_speed hashcrypt_speed fullspeed cipher-gcm -cipher-aead-srtp
| +   cipher-aead-srtp ${comp_progs}
the backslashes at EOL are missing, and even the patch syntax got messed up.

Hope that helps, Phil

___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

[Cryptodev-linux-devel] [PATCH 1/2] tests: add cryptodev simulating openssl wrapper

[Phil Sutter]

Signed-off-by: Phil Sutter phil.sut...@viprinet.com
---
 tests/openssl_wrapper.c |  267 +++
 1 files changed, 267 insertions(+), 0 deletions(-)
 create mode 100644 tests/openssl_wrapper.c

diff --git a/tests/openssl_wrapper.c b/tests/openssl_wrapper.c
new file mode 100644
index 000..038c58f
--- /dev/null
+++ b/tests/openssl_wrapper.c
@@ -0,0 +1,267 @@
+#include crypto/cryptodev.h
+#include stdio.h
+#include string.h
+#include openssl/aes.h
+#include openssl/evp.h
+#include openssl/hmac.h
+
+//#define DEBUG
+
+#ifdef DEBUG
+#  define dbgp(...) { \
+   fprintf(stderr, %s:%d: , __FILE__, __LINE__); \
+   fprintf(stderr, __VA_ARGS__); \
+   fprintf(stderr, \n); \
+}
+#else
+#  define dbgp(...) /* nothing */
+#endif
+
+enum ctx_type {
+   ctx_type_none = 0,
+   ctx_type_hmac,
+   ctx_type_md,
+};
+
+union openssl_ctx {
+   HMAC_CTX hmac;
+   EVP_MD_CTX md;
+};
+
+struct ctx_mapping {
+   __u32 ses;
+   enum ctx_type type;
+   union openssl_ctx ctx;
+};
+
+static struct ctx_mapping ctx_map[512];
+
+static struct ctx_mapping *find_mapping(__u32 ses)
+{
+   int i;
+
+   for (i = 0; i  512; i++) {
+   if (ctx_map[i].ses == ses)
+   return ctx_map[i];
+   }
+   return NULL;
+}
+
+static struct ctx_mapping *new_mapping(void)
+{
+   return find_mapping(0);
+}
+
+static void remove_mapping(__u32 ses)
+{
+   struct ctx_mapping *mapping;
+
+   if (!(mapping = find_mapping(ses))) {
+   printf(%s: failed to find mapping for session %d\n, __func__, 
ses);
+   return;
+   }
+   switch (mapping-type) {
+   case ctx_type_none:
+   break;
+   case ctx_type_hmac:
+   dbgp(%s: calling HMAC_CTX_cleanup\n, __func__);
+   HMAC_CTX_cleanup(mapping-ctx.hmac);
+   break;
+   case ctx_type_md:
+   dbgp(%s: calling EVP_MD_CTX_cleanup\n, __func__);
+   EVP_MD_CTX_cleanup(mapping-ctx.md);
+   break;
+   }
+   memset(mapping, 0, sizeof(*mapping));
+}
+
+static union openssl_ctx *__ses_to_ctx(__u32 ses)
+{
+   struct ctx_mapping *mapping;
+
+   if (!(mapping = find_mapping(ses)))
+   return NULL;
+   return mapping-ctx;
+}
+
+static HMAC_CTX *ses_to_hmac(__u32 ses) { return (HMAC_CTX 
*)__ses_to_ctx(ses); }
+static EVP_MD_CTX *ses_to_md(__u32 ses) { return (EVP_MD_CTX 
*)__ses_to_ctx(ses); }
+
+static const EVP_MD *sess_to_evp_md(struct session_op *sess)
+{
+   switch (sess-mac) {
+#ifndef OPENSSL_NO_MD5
+   case CRYPTO_MD5_HMAC: return EVP_md5();
+#endif
+#ifndef OPENSSL_NO_SHA
+   case CRYPTO_SHA1_HMAC:
+   case CRYPTO_SHA1:
+   return EVP_sha1();
+#endif
+#ifndef OPENSSL_NO_RIPEMD
+   case CRYPTO_RIPEMD160_HMAC: return EVP_ripemd160();
+#endif
+#ifndef OPENSSL_NO_SHA256
+   case CRYPTO_SHA2_256_HMAC: return EVP_sha256();
+#endif
+#ifndef OPENSSL_NO_SHA512
+   case CRYPTO_SHA2_384_HMAC: return EVP_sha384();
+   case CRYPTO_SHA2_512_HMAC: return EVP_sha512();
+#endif
+   default:
+   printf(%s: failed to get an EVP, things will be broken!\n, 
__func__);
+   return NULL;
+   }
+}
+
+static int openssl_hmac(struct session_op *sess, struct crypt_op *cop)
+{
+   HMAC_CTX *ctx = ses_to_hmac(sess-ses);
+
+   if (!ctx) {
+   struct ctx_mapping *mapping = new_mapping();
+   if (!mapping) {
+   printf(%s: failed to get new mapping\n, __func__);
+   return 1;
+   }
+
+   mapping-ses = sess-ses;
+   mapping-type = ctx_type_hmac;
+   ctx = mapping-ctx.hmac;
+
+   dbgp(calling HMAC_CTX_init);
+   HMAC_CTX_init(ctx);
+   dbgp(calling HMAC_Init_ex);
+   if (!HMAC_Init_ex(ctx, sess-mackey, sess-mackeylen,
+   sess_to_evp_md(sess), NULL)) {
+   printf(%s: HMAC_Init_ex failed\n, __func__);
+   return 1;
+   }
+   }
+
+   if (cop-len) {
+   dbgp(calling HMAC_Update);
+   if (!HMAC_Update(ctx, cop-src, cop-len)) {
+   printf(%s: HMAC_Update failed\n, __func__);
+   return 1;
+   }
+   }
+   if (cop-flags  COP_FLAG_FINAL ||
+   (cop-len  !(cop-flags  COP_FLAG_UPDATE))) {
+   dbgp(calling HMAC_Final);
+   if (!HMAC_Final(ctx, cop-mac, 0)) {
+   printf(%s: HMAC_Final failed\n, __func__);
+   remove_mapping(sess-ses);
+   return 1;
+   }
+   remove_mapping(sess-ses);
+   }
+   return 0;
+}
+
+static int openssl_md(struct session_op *sess, struct crypt_op *cop)
+{
+   EVP_MD_CTX *ctx = ses_to_md(sess-ses

[Cryptodev-linux-devel] [Patch v2 1/2] tests: add cryptodev simulating openssl wrapper

[Phil Sutter]

Signed-off-by: Phil Sutter phil.sut...@viprinet.com
---
 tests/openssl_wrapper.c |  267 +++
 tests/openssl_wrapper.h |6 +
 2 files changed, 273 insertions(+), 0 deletions(-)
 create mode 100644 tests/openssl_wrapper.c
 create mode 100644 tests/openssl_wrapper.h

diff --git a/tests/openssl_wrapper.c b/tests/openssl_wrapper.c
new file mode 100644
index 000..038c58f
--- /dev/null
+++ b/tests/openssl_wrapper.c
@@ -0,0 +1,267 @@
+#include crypto/cryptodev.h
+#include stdio.h
+#include string.h
+#include openssl/aes.h
+#include openssl/evp.h
+#include openssl/hmac.h
+
+//#define DEBUG
+
+#ifdef DEBUG
+#  define dbgp(...) { \
+   fprintf(stderr, %s:%d: , __FILE__, __LINE__); \
+   fprintf(stderr, __VA_ARGS__); \
+   fprintf(stderr, \n); \
+}
+#else
+#  define dbgp(...) /* nothing */
+#endif
+
+enum ctx_type {
+   ctx_type_none = 0,
+   ctx_type_hmac,
+   ctx_type_md,
+};
+
+union openssl_ctx {
+   HMAC_CTX hmac;
+   EVP_MD_CTX md;
+};
+
+struct ctx_mapping {
+   __u32 ses;
+   enum ctx_type type;
+   union openssl_ctx ctx;
+};
+
+static struct ctx_mapping ctx_map[512];
+
+static struct ctx_mapping *find_mapping(__u32 ses)
+{
+   int i;
+
+   for (i = 0; i  512; i++) {
+   if (ctx_map[i].ses == ses)
+   return ctx_map[i];
+   }
+   return NULL;
+}
+
+static struct ctx_mapping *new_mapping(void)
+{
+   return find_mapping(0);
+}
+
+static void remove_mapping(__u32 ses)
+{
+   struct ctx_mapping *mapping;
+
+   if (!(mapping = find_mapping(ses))) {
+   printf(%s: failed to find mapping for session %d\n, __func__, 
ses);
+   return;
+   }
+   switch (mapping-type) {
+   case ctx_type_none:
+   break;
+   case ctx_type_hmac:
+   dbgp(%s: calling HMAC_CTX_cleanup\n, __func__);
+   HMAC_CTX_cleanup(mapping-ctx.hmac);
+   break;
+   case ctx_type_md:
+   dbgp(%s: calling EVP_MD_CTX_cleanup\n, __func__);
+   EVP_MD_CTX_cleanup(mapping-ctx.md);
+   break;
+   }
+   memset(mapping, 0, sizeof(*mapping));
+}
+
+static union openssl_ctx *__ses_to_ctx(__u32 ses)
+{
+   struct ctx_mapping *mapping;
+
+   if (!(mapping = find_mapping(ses)))
+   return NULL;
+   return mapping-ctx;
+}
+
+static HMAC_CTX *ses_to_hmac(__u32 ses) { return (HMAC_CTX 
*)__ses_to_ctx(ses); }
+static EVP_MD_CTX *ses_to_md(__u32 ses) { return (EVP_MD_CTX 
*)__ses_to_ctx(ses); }
+
+static const EVP_MD *sess_to_evp_md(struct session_op *sess)
+{
+   switch (sess-mac) {
+#ifndef OPENSSL_NO_MD5
+   case CRYPTO_MD5_HMAC: return EVP_md5();
+#endif
+#ifndef OPENSSL_NO_SHA
+   case CRYPTO_SHA1_HMAC:
+   case CRYPTO_SHA1:
+   return EVP_sha1();
+#endif
+#ifndef OPENSSL_NO_RIPEMD
+   case CRYPTO_RIPEMD160_HMAC: return EVP_ripemd160();
+#endif
+#ifndef OPENSSL_NO_SHA256
+   case CRYPTO_SHA2_256_HMAC: return EVP_sha256();
+#endif
+#ifndef OPENSSL_NO_SHA512
+   case CRYPTO_SHA2_384_HMAC: return EVP_sha384();
+   case CRYPTO_SHA2_512_HMAC: return EVP_sha512();
+#endif
+   default:
+   printf(%s: failed to get an EVP, things will be broken!\n, 
__func__);
+   return NULL;
+   }
+}
+
+static int openssl_hmac(struct session_op *sess, struct crypt_op *cop)
+{
+   HMAC_CTX *ctx = ses_to_hmac(sess-ses);
+
+   if (!ctx) {
+   struct ctx_mapping *mapping = new_mapping();
+   if (!mapping) {
+   printf(%s: failed to get new mapping\n, __func__);
+   return 1;
+   }
+
+   mapping-ses = sess-ses;
+   mapping-type = ctx_type_hmac;
+   ctx = mapping-ctx.hmac;
+
+   dbgp(calling HMAC_CTX_init);
+   HMAC_CTX_init(ctx);
+   dbgp(calling HMAC_Init_ex);
+   if (!HMAC_Init_ex(ctx, sess-mackey, sess-mackeylen,
+   sess_to_evp_md(sess), NULL)) {
+   printf(%s: HMAC_Init_ex failed\n, __func__);
+   return 1;
+   }
+   }
+
+   if (cop-len) {
+   dbgp(calling HMAC_Update);
+   if (!HMAC_Update(ctx, cop-src, cop-len)) {
+   printf(%s: HMAC_Update failed\n, __func__);
+   return 1;
+   }
+   }
+   if (cop-flags  COP_FLAG_FINAL ||
+   (cop-len  !(cop-flags  COP_FLAG_UPDATE))) {
+   dbgp(calling HMAC_Final);
+   if (!HMAC_Final(ctx, cop-mac, 0)) {
+   printf(%s: HMAC_Final failed\n, __func__);
+   remove_mapping(sess-ses);
+   return 1;
+   }
+   remove_mapping(sess-ses);
+   }
+   return 0;
+}
+
+static int openssl_md(struct session_op *sess, struct

[Cryptodev-linux-devel] [PATCH] allow updating the IV in userspace

[Phil Sutter]

When the user has specified COP_FLAG_WRITE_IV in crypt_op.flags, the
updated IV will be written back to userspace. This is useful for
encryption of continuous data in several steps, without having to care
for each cipher's inerna.

Protecting this functionality by a flag allows for backwards
compatibility as well as using a read-only (i.e. const) buffer holding
the IV.
---
 cryptodev.h  |1 +
 cryptodev_int.h  |6 +++
 cryptodev_main.c |   99 --
 3 files changed, 66 insertions(+), 40 deletions(-)

diff --git a/cryptodev.h b/cryptodev.h
index c50505b..db4697c 100644
--- a/cryptodev.h
+++ b/cryptodev.h
@@ -120,6 +120,7 @@ struct session_info_op {
 #define COP_FLAG_NONE  (0  0) /* totally no flag */
 #define COP_FLAG_UPDATE(1  0) /* multi-update hash mode */
 #define COP_FLAG_FINAL (1  1) /* multi-update final hash mode */
+#define COP_FLAG_WRITE_IV  (1  2) /* update the IV during operation */
 
 /* Stuff for bignum arithmetic and public key
  * cryptography - not supported yet by linux
diff --git a/cryptodev_int.h b/cryptodev_int.h
index 4be66ec..d20c01e 100644
--- a/cryptodev_int.h
+++ b/cryptodev_int.h
@@ -66,6 +66,12 @@ inline static void cryptodev_cipher_set_iv(struct 
cipher_data *cdata,
memcpy(cdata-async.iv, iv, min(iv_size, sizeof(cdata-async.iv)));
 }
 
+inline static void cryptodev_cipher_get_iv(struct cipher_data *cdata,
+   void *iv, size_t iv_size)
+{
+   memcpy(iv, cdata-async.iv, min(iv_size, sizeof(cdata-async.iv)));
+}
+
 /* hash stuff */
 struct hash_data {
int init; /* 0 uninitialized */
diff --git a/cryptodev_main.c b/cryptodev_main.c
index 871690b..cb81107 100644
--- a/cryptodev_main.c
+++ b/cryptodev_main.c
@@ -738,6 +738,11 @@ static int crypto_run(struct fcrypt *fcr, struct 
kernel_crypt_op *kcop)
goto out_unlock;
}
 
+   if (ses_ptr-cdata.init != 0) {
+   cryptodev_cipher_get_iv(ses_ptr-cdata, kcop-iv,
+   min(ses_ptr-cdata.ivsize, kcop-ivlen));
+   }
+
if (ses_ptr-hdata.init != 0 
((cop-flags  COP_FLAG_FINAL) ||
   (!(cop-flags  COP_FLAG_UPDATE) || cop-len == 0))) {
@@ -999,6 +1004,26 @@ static int fill_kcop_from_cop(struct kernel_crypt_op 
*kcop, struct fcrypt *fcr)
return 0;
 }
 
+/* this function has to be called from process context */
+static int fill_cop_from_kcop(struct kernel_crypt_op *kcop, struct fcrypt *fcr)
+{
+   int ret;
+
+   if (kcop-digestsize) {
+   ret = copy_to_user(kcop-cop.mac,
+   kcop-hash_output, kcop-digestsize);
+   if (unlikely(ret))
+   return -EFAULT;
+   }
+   if (kcop-ivlen  kcop-cop.flags  COP_FLAG_WRITE_IV) {
+   ret = copy_to_user(kcop-cop.iv,
+   kcop-iv, kcop-ivlen);
+   if (unlikely(ret))
+   return -EFAULT;
+   }
+   return 0;
+}
+
 static int kcop_from_user(struct kernel_crypt_op *kcop,
struct fcrypt *fcr, void __user *arg)
 {
@@ -1008,6 +1033,20 @@ static int kcop_from_user(struct kernel_crypt_op *kcop,
return fill_kcop_from_cop(kcop, fcr);
 }
 
+static int kcop_to_user(struct kernel_crypt_op *kcop,
+   struct fcrypt *fcr, void __user *arg)
+{
+   int ret;
+
+   ret = fill_cop_from_kcop(kcop, fcr);
+   if (unlikely(ret))
+   return ret;
+
+   if (unlikely(copy_to_user(arg, kcop-cop, sizeof(kcop-cop
+   return -EFAULT;
+   return 0;
+}
+
 static inline void tfm_info_to_alg_info(struct alg_info *dst, struct 
crypto_tfm *tfm)
 {
snprintf(dst-cra_name, CRYPTODEV_MAX_ALG_NAME,
@@ -1106,15 +1145,7 @@ cryptodev_ioctl(struct file *filp, unsigned int cmd, 
unsigned long arg_)
if (unlikely(ret))
return ret;
 
-   if (kcop.digestsize) {
-   ret = copy_to_user(kcop.cop.mac,
-   kcop.hash_output, kcop.digestsize);
-   if (unlikely(ret))
-   return -EFAULT;
-   }
-   if (unlikely(copy_to_user(arg, kcop.cop, sizeof(kcop.cop
-   return -EFAULT;
-   return 0;
+   return kcop_to_user(kcop, fcr, arg);
case CIOCASYNCCRYPT:
if (unlikely(ret = kcop_from_user(kcop, fcr, arg)))
return ret;
@@ -1125,15 +1156,7 @@ cryptodev_ioctl(struct file *filp, unsigned int cmd, 
unsigned long arg_)
if (unlikely(ret))
return ret;
 
-   if (kcop.digestsize) {
-   ret = copy_to_user(kcop.cop.mac,
-   kcop.hash_output, kcop.digestsize);
-   if 

[Cryptodev-linux-devel] [PATCH 1/5] enable CIOCGSESSINFO for compat-calls, too

[Phil Sutter]

---
 cryptodev_main.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/cryptodev_main.c b/cryptodev_main.c
index cb81107..0f93010 100644
--- a/cryptodev_main.c
+++ b/cryptodev_main.c
@@ -1267,6 +1267,7 @@ cryptodev_compat_ioctl(struct file *file, unsigned int 
cmd, unsigned long arg_)
case CIOCASYMFEAT:
case CRIOGET:
case CIOCFSESSION:
+   case CIOCGSESSINFO:
return cryptodev_ioctl(file, cmd, arg_);
 
case COMPAT_CIOCGSESSION:
-- 
1.7.3.4



___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

[Cryptodev-linux-devel] [PATCH 2/5] alignment checks: prevent spurious warnings when ses-alignmask == 0

[Phil Sutter]

---
 cryptodev_main.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cryptodev_main.c b/cryptodev_main.c
index 0f93010..efd08c8 100644
--- a/cryptodev_main.c
+++ b/cryptodev_main.c
@@ -595,7 +595,7 @@ static int get_userbuf(struct csession *ses, struct 
kernel_crypt_op *kcop,
if (cop-src == NULL)
return -EINVAL;
 
-   if (!IS_ALIGNED((unsigned long)cop-src, ses-alignmask)) {
+   if (ses-alignmask  !IS_ALIGNED((unsigned long)cop-src, 
ses-alignmask)) {
dprintk(2, KERN_WARNING, %s: careful - source address %lx is 
not %d byte aligned\n,
__func__, (unsigned long)cop-src, 
ses-alignmask + 1);
}
@@ -610,7 +610,7 @@ static int get_userbuf(struct csession *ses, struct 
kernel_crypt_op *kcop,
dst_pagecount = PAGECOUNT(cop-dst, cop-len);
write_src = 0;
 
-   if (!IS_ALIGNED((unsigned long)cop-dst, ses-alignmask)) {
+   if (ses-alignmask  !IS_ALIGNED((unsigned long)cop-dst, 
ses-alignmask)) {
dprintk(2, KERN_WARNING, %s: careful - destination 
address %lx is not %d byte aligned\n,
__func__, (unsigned long)cop-dst, 
ses-alignmask + 1);
}
-- 
1.7.3.4



___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] mob broken?

[Phil Sutter]

Hi,

On Fri, Dec 31, 2010 at 12:50:35AM +0200, Nikos Mavrogiannopoulos wrote:
 I've removed the mob branch few weeks before. I didn't find it much
 easy to use...
 Was it of use to you?

Well, actually I didn't know about mob before getting in contact with
cryptodev-linux. Although the idea of having something users can push to
without having to authenticate is quite nice, things may get a bit
chaotic.

I just thought pushing more or less trivial patches to mob could make it
easier for you to apply them. But yes, you won't get around reviewing
them either way, which should take the most time, anyway.

Greetings, Phil

-- 
Viprinet GmbH
Mainzer Str. 43
55411 Bingen am Rhein
Germany

Zentrale: +49-6721-49030-0
Durchwahl:+49-6721-49030-134
Fax:  +49-6721-49030-209

phil.sut...@viprinet.com
http://www.viprinet.com

Sitz der Gesellschaft: Bingen am Rhein
Handelsregister: Amtsgericht Mainz HRB40380
Geschäftsführer: Simon Kissel


___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

[Cryptodev-linux-devel] [PATCH] speed, async_speed: fix measurement and units

[Phil Sutter]

Since measurement is done in bytes, use a capital B as unit. Also,
stepping by 1024 from one magnitude to the next is more common. This is
indicated by the 'i' in between the magnitude and unit symbols.
---
 examples/async_speed.c |   32 +++-
 examples/speed.c   |   32 +++-
 2 files changed, 22 insertions(+), 42 deletions(-)

diff --git a/examples/async_speed.c b/examples/async_speed.c
index 679219b..940d923 100644
--- a/examples/async_speed.c
+++ b/examples/async_speed.c
@@ -43,29 +43,19 @@ static void alarm_handler(int signo)
pfd.events = POLLIN;
 }
 
+static char *units[] = { , Ki, Mi, Gi, Ti, 0};
+
 static void value2human(double bytes, double time, double* data, double* 
speed,char* metric)
 {
-if (bytes  1000  bytes  1000*1000) {
-*data = ((double)bytes)/1000;
-*speed = *data/time;
-strcpy(metric, Kb);
-return;
-} else if (bytes = 1000*1000  bytes  1000*1000*1000) {
-*data = ((double)bytes)/(1000*1000);
-*speed = *data/time;
-strcpy(metric, Mb);
-return;
-} else if (bytes = 1000*1000*1000) {
-*data = ((double)bytes)/(1000*1000*1000);
-*speed = *data/time;
-strcpy(metric, Gb);
-return;
-} else {
-*data = (double)bytes;
-*speed = *data/time;
-strcpy(metric, bytes);
-return;
-}
+   int unit = 0;
+
+   *data = bytes;
+   while (*data  1024  units[unit + 1]) {
+   *data /= 1024;
+   unit++;
+   }
+   *speed = *data / time;
+   sprintf(metric, %sB, units[unit]);
 }
 
 
diff --git a/examples/speed.c b/examples/speed.c
index 998772d..63218a9 100644
--- a/examples/speed.c
+++ b/examples/speed.c
@@ -39,29 +39,19 @@ static void alarm_handler(int signo)
 must_finish = 1;
 }
 
+static char *units[] = { , Ki, Mi, Gi, Ti, 0};
+
 static void value2human(double bytes, double time, double* data, double* 
speed,char* metric)
 {
-if (bytes  1000  bytes  1000*1000) {
-*data = ((double)bytes)/1000;
-*speed = *data/time;
-strcpy(metric, Kb);
-return;
-} else if (bytes = 1000*1000  bytes  1000*1000*1000) {
-*data = ((double)bytes)/(1000*1000);
-*speed = *data/time;
-strcpy(metric, Mb);
-return;
-} else if (bytes = 1000*1000*1000) {
-*data = ((double)bytes)/(1000*1000*1000);
-*speed = *data/time;
-strcpy(metric, Gb);
-return;
-} else {
-*data = (double)bytes;
-*speed = *data/time;
-strcpy(metric, bytes);
-return;
-}
+   int unit = 0;
+
+   *data = bytes;
+   while (*data  1024  units[unit + 1]) {
+   *data /= 1024;
+   unit++;
+   }
+   *speed = *data / time;
+   sprintf(metric, %sB, units[unit]);
 }
 
 
-- 
1.7.3.2



___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

[Cryptodev-linux-devel] [PATCH 2/4] crypto_run: cop-len is likely not zero

[Phil Sutter]

---
 cryptodev_main.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/cryptodev_main.c b/cryptodev_main.c
index 38e1a44..69f2d29 100644
--- a/cryptodev_main.c
+++ b/cryptodev_main.c
@@ -730,7 +730,7 @@ static int crypto_run(struct fcrypt *fcr, struct 
kernel_crypt_op *kcop)
min(ses_ptr-cdata.ivsize, kcop-ivlen));
}
 
-   if (cop-len != 0) {
+   if (likely(cop-len)) {
ret = __crypto_run_zc(ses_ptr, kcop);
if (unlikely(ret))
goto out_unlock;
-- 
1.7.3.2



___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

[Cryptodev-linux-devel] [PATCH 3/4] minor whitespace cleanup

[Phil Sutter]

---
 cryptodev_main.c |4 ++--
 examples/hmac.c  |4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/cryptodev_main.c b/cryptodev_main.c
index 69f2d29..e869557 100644
--- a/cryptodev_main.c
+++ b/cryptodev_main.c
@@ -993,7 +993,7 @@ static int fill_kcop_from_cop(struct kernel_crypt_op *kcop, 
struct fcrypt *fcr)
return -EFAULT;
}
}
-   
+
return 0;
 }
 
@@ -1060,7 +1060,7 @@ cryptodev_ioctl(struct file *filp, unsigned int cmd, 
unsigned long arg_)
ret = crypto_run(fcr, kcop);
if (unlikely(ret))
return ret;
-   
+
if (kcop.digestsize) {
ret = copy_to_user(kcop.cop.mac,
kcop.hash_output, kcop.digestsize);
diff --git a/examples/hmac.c b/examples/hmac.c
index 166b5dd..3a308c9 100644
--- a/examples/hmac.c
+++ b/examples/hmac.c
@@ -130,9 +130,9 @@ test_crypto(int cfd)
perror(ioctl(CIOCCRYPT));
return 1;
}
-   
+
memcpy(oldmac, mac, sizeof(mac));
-   
+
/* Decrypt data.encrypted to data.decrypted */
cryp.src = data.encrypted;
cryp.dst = data.decrypted;
-- 
1.7.3.2



___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

[Cryptodev-linux-devel] [PATCH 1/4] since COP_FLAG_* are bits, we can OR them together for checks

[Phil Sutter]

---
 cryptodev_main.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/cryptodev_main.c b/cryptodev_main.c
index 2a6e4bc..38e1a44 100644
--- a/cryptodev_main.c
+++ b/cryptodev_main.c
@@ -705,8 +705,7 @@ static int crypto_run(struct fcrypt *fcr, struct 
kernel_crypt_op *kcop)
return -EINVAL;
}
 
-   if (ses_ptr-hdata.init != 0  !(cop-flags  COP_FLAG_UPDATE) 
-   !(cop-flags  COP_FLAG_FINAL)) {
+   if (ses_ptr-hdata.init != 0  !(cop-flags  (COP_FLAG_UPDATE | 
COP_FLAG_FINAL))) {
ret = cryptodev_hash_reset(ses_ptr-hdata);
if (unlikely(ret)) {
dprintk(1, KERN_ERR,
-- 
1.7.3.2



___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

[Cryptodev-linux-devel] [PATCH 1/6] add support for CRYPTO_AES_ECB

[Phil Sutter]

---
 cryptodev.h  |1 +
 cryptodev_main.c |3 +++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/cryptodev.h b/cryptodev.h
index adfc374..e16d30a 100644
--- a/cryptodev.h
+++ b/cryptodev.h
@@ -40,6 +40,7 @@ enum cryptodev_crypto_op_t {
CRYPTO_SHA2_512_HMAC = 20,
CRYPTO_AES_CTR = 21,
CRYPTO_AES_XTS = 22,
+   CRYPTO_AES_ECB = 23,
 
CRYPTO_CAMELLIA_CBC = 101,
CRYPTO_RIPEMD160,
diff --git a/cryptodev_main.c b/cryptodev_main.c
index 134..7326c3d 100644
--- a/cryptodev_main.c
+++ b/cryptodev_main.c
@@ -156,6 +156,9 @@ crypto_create_session(struct fcrypt *fcr, struct session_op 
*sop)
case CRYPTO_AES_CBC:
alg_name = cbc(aes);
break;
+   case CRYPTO_AES_ECB:
+   alg_name = ecb(aes);
+   break;
case CRYPTO_CAMELLIA_CBC:
alg_name = cbc(camelia);
break;
-- 
1.7.3.2



___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

[Cryptodev-linux-devel] [PATCH 5/8] implement asynchronous operation mode

[Phil Sutter]

In order to submit a job for asynchronous completion, one uses the ioctl
CIOCASYNCCRYPT, which is syntactically equal to CIOCCRYPT.  But, the
former will return immediately, the result has to be fetched later using
CIOCASYNCFETCH (which will always return the first completed job, asking
for a specific one is not possible).

These ioctls can return -EBUSY in case there are either no free slots
left for enqueueing a new job or there are no completed jobs waiting,
depending on the ioctl's direction.

For now, the number of slots (i.e. maximum length of the job queue) is
limited to 16 items, defined in DEF_COP_RINGSIZE.
---
 cryptodev.h  |4 +
 cryptodev_int.h  |8 ++-
 cryptodev_main.c |  274 --
 3 files changed, 255 insertions(+), 31 deletions(-)

diff --git a/cryptodev.h b/cryptodev.h
index 8cee01b..adfc374 100644
--- a/cryptodev.h
+++ b/cryptodev.h
@@ -164,4 +164,8 @@ enum cryptodev_crk_op_t {
  */
 #define CRIOGET_NOT_NEEDED 1
 
+/* additional ioctls for asynchronous  operation */
+#define CIOCASYNCCRYPT_IOW('c', 107, struct crypt_op)
+#define CIOCASYNCFETCH_IOR('c', 108, struct crypt_op)
+
 #endif /* L_CRYPTODEV_H */
diff --git a/cryptodev_int.h b/cryptodev_int.h
index 63e2ebd..8066965 100644
--- a/cryptodev_int.h
+++ b/cryptodev_int.h
@@ -26,7 +26,8 @@ extern int cryptodev_verbosity;
 
 /* For zero copy */
 int __get_userbuf(uint8_t __user *addr, uint32_t len, int write,
-   int pgcount, struct page **pg, struct scatterlist *sg);
+   int pgcount, struct page **pg, struct scatterlist *sg,
+   struct task_struct *task, struct mm_struct *mm);
 void release_user_pages(struct page **pg, int pagecount);
 
 /* last page - first page + 1 */
@@ -114,6 +115,8 @@ struct compat_session_op {
 /* compat ioctls, defined for the above structs */
 #define COMPAT_CIOCGSESSION_IOWR('c', 102, struct compat_session_op)
 #define COMPAT_CIOCCRYPT   _IOWR('c', 104, struct compat_crypt_op)
+#define COMPAT_CIOCASYNCCRYPT  _IOW('c', 107, struct compat_crypt_op)
+#define COMPAT_CIOCASYNCFETCH  _IOR('c', 108, struct compat_crypt_op)
 
 #endif /* CONFIG_COMPAT */
 
@@ -126,6 +129,9 @@ struct kernel_crypt_op {
 
int digestsize;
uint8_t hash_output[AALG_MAX_RESULT_LEN];
+
+   struct task_struct *task;
+   struct mm_struct *mm;
 };
 
 #endif /* CRYPTODEV_INT_H */
diff --git a/cryptodev_main.c b/cryptodev_main.c
index d2c78c1..6ab235f 100644
--- a/cryptodev_main.c
+++ b/cryptodev_main.c
@@ -53,6 +53,10 @@ MODULE_LICENSE(GPL);
 
 #define CRYPTODEV_STATS
 
+/* Default (pre-allocated) size of the job queue.
+ * These are free, pending and done items all together. */
+#define DEF_COP_RINGSIZE 16
+
 /* == Module parameters == */
 
 int cryptodev_verbosity;
@@ -71,8 +75,22 @@ struct fcrypt {
struct mutex sem;
 };
 
+struct todo_list_item {
+   struct list_head __hook;
+   struct kernel_crypt_op kcop;
+   int result;
+};
+
+struct locked_list {
+   struct list_head list;
+   struct mutex lock;
+};
+
 struct crypt_priv {
struct fcrypt fcrypt;
+   struct locked_list free, todo, done;
+   int itemcount;
+   struct work_struct cryptask;
 };
 
 #define FILL_SG(sg, ptr, len)  \
@@ -101,6 +119,9 @@ struct csession {
struct scatterlist *sg;
 };
 
+/* cryptodev's own workqueue, keeps crypto tasks from disturbing the force */
+static struct workqueue_struct *cryptodev_wq;
+
 /* Prepare session for future use. */
 static int
 crypto_create_session(struct fcrypt *fcr, struct session_op *sop)
@@ -521,15 +542,16 @@ void release_user_pages(struct page **pg, int pagecount)
 
 /* fetch the pages addr resides in into pg and initialise sg with them */
 int __get_userbuf(uint8_t __user *addr, uint32_t len, int write,
-   int pgcount, struct page **pg, struct scatterlist *sg)
+   int pgcount, struct page **pg, struct scatterlist *sg,
+   struct task_struct *task, struct mm_struct *mm)
 {
int ret, pglen, i = 0;
struct scatterlist *sgp;
 
-   down_write(current-mm-mmap_sem);
-   ret = get_user_pages(current, current-mm,
+   down_write(mm-mmap_sem);
+   ret = get_user_pages(task, mm,
(unsigned long)addr, pgcount, write, 0, pg, NULL);
-   up_write(current-mm-mmap_sem);
+   up_write(mm-mmap_sem);
if (ret != pgcount)
return -EINVAL;
 
@@ -555,6 +577,7 @@ static int get_userbuf(struct csession *ses, struct 
kernel_crypt_op *kcop,
 {
int src_pagecount, dst_pagecount = 0, pagecount, write_src = 1;
struct crypt_op *cop = kcop-cop;
+   int rc;
 
if (cop-src == NULL)
return -EINVAL;
@@ -595,24 +618,28 @@ static int get_userbuf(struct csession *ses, struct 
kernel_crypt_op *kcop,
ses-array_size = array_size;
}
 
-   if (__get_userbuf(cop-src, cop-len, 

[Cryptodev-linux-devel] [PATCH 2/8] keep a copy of the IV and size in struct kernel_crypt_op

[Phil Sutter]

---
 cryptodev_int.h  |3 +++
 cryptodev_main.c |   28 +++-
 2 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/cryptodev_int.h b/cryptodev_int.h
index 6b0b909..5072e8f 100644
--- a/cryptodev_int.h
+++ b/cryptodev_int.h
@@ -120,6 +120,9 @@ struct compat_session_op {
 /* kernel-internal extension to struct crypt_op */
 struct kernel_crypt_op {
struct crypt_op cop;
+
+   int ivlen;
+   __u8 iv[EALG_MAX_BLOCK_LEN];
 };
 
 #endif /* CRYPTODEV_INT_H */
diff --git a/cryptodev_main.c b/cryptodev_main.c
index fb51723..5fd5907 100644
--- a/cryptodev_main.c
+++ b/cryptodev_main.c
@@ -679,23 +679,8 @@ static int crypto_run(struct fcrypt *fcr, struct 
kernel_crypt_op *kcop)
goto out_unlock;
}
 
-   if (cop-iv) {
-   uint8_t iv[EALG_MAX_BLOCK_LEN];
-
-   ret = copy_from_user(iv, cop-iv,
-   min((int)sizeof(iv), (ses_ptr-cdata.ivsize)));
-   if (unlikely(ret)) {
-   dprintk(1, KERN_ERR,
-   error copying IV (%d bytes)\n,
-   min((int)sizeof(iv),
-   (ses_ptr-cdata.ivsize)));
-   ret = -EFAULT;
-   goto out_unlock;
-   }
-
-   cryptodev_cipher_set_iv(ses_ptr-cdata, iv,
-   ses_ptr-cdata.ivsize);
-   }
+   cryptodev_cipher_set_iv(ses_ptr-cdata, kcop-iv,
+   min(ses_ptr-cdata.ivsize, kcop-ivlen));
}
 
if (cop-len != 0) {
@@ -786,6 +771,7 @@ static int fill_kcop_from_cop(struct kernel_crypt_op *kcop, 
struct fcrypt *fcr)
 {
struct crypt_op *cop = kcop-cop;
struct csession *ses_ptr;
+   int rc;
 
/* this also enters ses_ptr-sem */
ses_ptr = crypto_get_session_by_sid(fcr, cop-ses);
@@ -793,8 +779,16 @@ static int fill_kcop_from_cop(struct kernel_crypt_op 
*kcop, struct fcrypt *fcr)
dprintk(1, KERN_ERR, invalid session ID=0x%08X\n, cop-ses);
return -EINVAL;
}
+   kcop-ivlen = cop-iv ? ses_ptr-cdata.ivsize : 0;
mutex_unlock(ses_ptr-sem);
 
+   if (unlikely(rc = copy_from_user(kcop-iv, cop-iv, kcop-ivlen))) {
+   dprintk(1, KERN_ERR,
+   error copying IV (%d bytes), copy_from_user returned 
%d for address %lx\n,
+   kcop-ivlen, rc, (unsigned long)cop-iv);
+   return -EFAULT;
+   }
+
return 0;
 }
 
-- 
1.7.2.2



___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

[Cryptodev-linux-devel] [PATCH 8/8] add async versions of the code samples

[Phil Sutter]

);
+
+   if (memcmp(mac, oldmac, 20) != 0) {
+   fprintf(stderr,
+   FAIL: Hash in decrypted data different than in 
encrypted.\n);
+   return 1;
+   } else
+   printf(HMAC Test 2: passed\n);
+
+   /* Finish crypto session */
+   if (ioctl(cfd, CIOCFSESSION, sess.ses)) {
+   perror(ioctl(CIOCFSESSION));
+   return 1;
+   }
+
+   return 0;
+}
+
+static int
+test_extras(int cfd)
+{
+   struct session_op sess;
+   struct crypt_op cryp;
+   uint8_t mac[AALG_MAX_RESULT_LEN];
+   uint8_t oldmac[AALG_MAX_RESULT_LEN];
+   uint8_t md5_hmac_out[] = 
\x75\x0c\x78\x3e\x6a\xb0\xb5\x03\xea\xa8\x6e\x31\x0a\x5d\xb7\x38;
+   uint8_t sha1_out[] = 
\x8f\x82\x03\x94\xf9\x53\x35\x18\x20\x45\xda\x24\xf3\x4d\xe5\x2b\xf8\xbc\x34\x32;
+   int i;
+
+   memset(sess, 0, sizeof(sess));
+   memset(cryp, 0, sizeof(cryp));
+
+   /* Use the garbage that is on the stack :-) */
+   /* memset(data, 0, sizeof(data)); */
+
+   /* SHA1 plain test */
+   memset(mac, 0, sizeof(mac));
+
+   sess.cipher = 0;
+   sess.mac = CRYPTO_SHA1;
+   if (ioctl(cfd, CIOCGSESSION, sess)) {
+   perror(ioctl(CIOCGSESSION));
+   return 1;
+   }
+
+   cryp.ses = sess.ses;
+   cryp.len = sizeof(what do)-1;
+   cryp.src = what do;
+   cryp.mac = mac;
+   cryp.op = COP_ENCRYPT;
+   cryp.flags = COP_FLAG_UPDATE;
+
+   DO_OR_DIE(do_async_crypt(cfd, cryp), 0);
+   DO_OR_DIE(do_async_fetch(cfd, cryp), 0);
+
+   cryp.ses = sess.ses;
+   cryp.len = sizeof( ya want for nothing?)-1;
+   cryp.src =  ya want for nothing?;
+   cryp.mac = mac;
+   cryp.op = COP_ENCRYPT;
+   cryp.flags = COP_FLAG_FINAL;
+
+   DO_OR_DIE(do_async_crypt(cfd, cryp), 0);
+   DO_OR_DIE(do_async_fetch(cfd, cryp), 0);
+
+   if (memcmp(mac, sha1_out, 20)!=0) {
+   printf(mac: );
+   for (i=0;iSHA1_HASH_LEN;i++) {
+   printf(%.2x, (uint8_t)mac[i]);
+   }
+   puts(\n);
+   fprintf(stderr, HASH test [update]: failed\n);
+   } else {
+   fprintf(stderr, HASH test [update]: passed\n);
+   }
+
+   memset(mac, 0, sizeof(mac));
+
+   /* Finish crypto session */
+   if (ioctl(cfd, CIOCFSESSION, sess.ses)) {
+   perror(ioctl(CIOCFSESSION));
+   return 1;
+   }
+
+   return 0;
+}
+
+
+int
+main()
+{
+   int fd = -1, cfd = -1;
+
+   /* Open the crypto device */
+   fd = open(/dev/crypto, O_RDWR, 0);
+   if (fd  0) {
+   perror(open(/dev/crypto));
+   return 1;
+   }
+
+   /* Clone file descriptor */
+   if (ioctl(fd, CRIOGET, cfd)) {
+   perror(ioctl(CRIOGET));
+   return 1;
+   }
+
+   /* Set close-on-exec (not really neede here) */
+   if (fcntl(cfd, F_SETFD, 1) == -1) {
+   perror(fcntl(F_SETFD));
+   return 1;
+   }
+
+   /* Run the test itself */
+   if (test_crypto(cfd))
+   return 1;
+
+   if (test_extras(cfd))
+   return 1;
+
+   /* Close cloned descriptor */
+   if (close(cfd)) {
+   perror(close(cfd));
+   return 1;
+   }
+
+   /* Close the original descriptor */
+   if (close(fd)) {
+   perror(close(fd));
+   return 1;
+   }
+
+   return 0;
+}
diff --git a/examples/async_speed.c b/examples/async_speed.c
new file mode 100644
index 000..a288144
--- /dev/null
+++ b/examples/async_speed.c
@@ -0,0 +1,197 @@
+/*  cryptodev_test - simple benchmark tool for cryptodev
+ *
+ *Copyright (C) 2010 by Phil Sutter phil.sut...@viprinet.com
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  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
+ */
+#include errno.h
+#include fcntl.h
+#include poll.h
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include sys/ioctl.h
+#include sys/time.h
+#include sys/types.h
+#include signal.h
+#include crypto/cryptodev.h
+
+static double udifftimeval(struct timeval start, struct timeval end)
+{
+   return (double)(end.tv_usec - start.tv_usec) +
+  (double)(end.tv_sec - start.tv_sec) * 1000 * 1000;
+}
+
+static int

[Cryptodev-linux-devel] [PATCH 6/8] increase the number of slots for asynchronous completion dynamically

[Phil Sutter]

---
 cryptodev_main.c |   20 
 1 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/cryptodev_main.c b/cryptodev_main.c
index 6ab235f..09d2c41 100644
--- a/cryptodev_main.c
+++ b/cryptodev_main.c
@@ -53,9 +53,10 @@ MODULE_LICENSE(GPL);
 
 #define CRYPTODEV_STATS
 
-/* Default (pre-allocated) size of the job queue.
+/* Default (pre-allocated) and maximum size of the job queue.
  * These are free, pending and done items all together. */
 #define DEF_COP_RINGSIZE 16
+#define MAX_COP_RINGSIZE 64
 
 /* == Module parameters == */
 
@@ -866,21 +867,32 @@ clonefd(struct file *filp)
  *
  * returns:
  * -EBUSY when there are no free queue slots left
+ *(and the number of slots has reached it MAX_COP_RINGSIZE)
+ * -EFAULT when there was a memory allocation error
  * 0 on success */
 static int crypto_async_run(struct crypt_priv *pcr, struct kernel_crypt_op 
*kcop)
 {
struct todo_list_item *item = NULL;
 
mutex_lock(pcr-free.lock);
-   if (!list_empty(pcr-free.list)) {
+   if (likely(!list_empty(pcr-free.list))) {
item = list_first_entry(pcr-free.list,
struct todo_list_item, __hook);
list_del(item-__hook);
+   } else if (pcr-itemcount  MAX_COP_RINGSIZE) {
+   pcr-itemcount++;
+   } else {
+   mutex_unlock(pcr-free.lock);
+   return -EBUSY;
}
mutex_unlock(pcr-free.lock);
 
-   if (!item) {
-   return -EBUSY;
+   if (unlikely(!item)) {
+   item = kzalloc(sizeof(struct todo_list_item), GFP_KERNEL);
+   if (unlikely(!item))
+   return -EFAULT;
+   dprintk(1, KERN_INFO, %s: increased item count to %d\n,
+   __func__, pcr-itemcount);
}
 
memcpy(item-kcop, kcop, sizeof(struct kernel_crypt_op));
-- 
1.7.2.2



___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel

Re: [Cryptodev-linux-devel] [PATCH] RFC: third working version of the async ops

[Phil Sutter]

Hey,

On Wed, Oct 20, 2010 at 12:35:51PM +0200, Nikos Mavrogiannopoulos wrote:
 On Wed, Oct 13, 2010 at 5:17 PM, Phil Sutter phil.sut...@viprinet.com wrote:
  Still ugly as hell (and therefore TODO):
  - having to pass task_struct and mm_struct from the calling process
   around
  - need to copy the IV right at write() time (copy_from_user seems to
   work in process context only)
 
 I'm still trying to find time to work on those. I think they are important
 issue to be dealt before adding it.

Yes, me too. My solution introduces a struct kernel_crypt_op, containing
a struct crypt_op and some other fields (amongst others the task_struct,
mm_struct, IV and IV length). Looks fine so far, I'm currently sorting
things a little for easier review, then submit the next version for
review to the list. :)

  Depending on the chunk size, the throughput drops in a range between 80%
  and 7%. So if one can afford to wait for cryptodev, it's adviceable to
  also do so.
 
 I wonder why is that drop in performance? Did you test multiple submissions of
 jobs or just a single one?

Not really sure about that. On one hand there is the locking of the
lists (although I managed to have that done quite efficiently), on the
other hand userspace needs three syscalls (poll(),
ioctl(CIOCASYNCCRYPT), ioctl(CIOCASYNCFETCH)) for a single operation
instead of one. My async speed tester optimises that a little (polling
for both POLLIN and POLLOUT, so a single poll() is used for fetching the
last result and submitting the next job). And I should perhaps have
mentioned that the given values were measured using the null-cipher
test. With AES-128, the worst-case throughput is at about 40%, so not so
bad at all.

Besides having a bug somewhere, I don't care too much about the lower
performance. The async-mode is useful mainly for programs doing other
stuff at the same time, in which case having to wait for each crypto
operation to complete before being able to do anything else should be
such a PITA that performance is actually higher using the async
interface.

Hmm, I can't wait for Herbert's interface to getting finished. The plan
is still to adjust speed.c for using it and to compare the throughput.
Just to be able to say: you got it exactly how you wanted it, and look
here, it sucks. :)

Greetings, Phil

-- 
Viprinet GmbH
Mainzer Str. 43
55411 Bingen am Rhein
Germany

Zentrale: +49-6721-49030-0
Durchwahl:+49-6721-49030-134
Fax:  +49-6721-49030-209

phil.sut...@viprinet.com
http://www.viprinet.com

Sitz der Gesellschaft: Bingen am Rhein
Handelsregister: Amtsgericht Mainz HRB40380
Geschäftsführer: Simon Kissel


___
Cryptodev-linux-devel mailing list
Cryptodev-linux-devel@gna.org
https://mail.gna.org/listinfo/cryptodev-linux-devel