Re: [v2 PATCH 7/16] crypto: simd - Add simd skcipher helper

2016-11-15 Thread Herbert Xu
On Sun, Nov 13, 2016 at 06:27:40PM -0800, Eric Biggers wrote:
> On Sun, Nov 13, 2016 at 07:45:38PM +0800, Herbert Xu wrote:
> > This patch adds the simd skcipher helper which is meant to be
> > a replacement for ablk helper.  It replaces the underlying blkcipher
> > interface with skcipher, and also presents the top-level algorithm
> > as an skcipher.
> 
> I assume this means it's planned for all users of ablk_helper to be migrated 
> to
> crypto_simd, and ablk_helper will be removed?

Yes that's the idea.

> > +   salg = kzalloc(sizeof(*alg), GFP_KERNEL);
> > +   if (!salg) {
> > +   salg = ERR_PTR(-ENOMEM);
> > +   goto out_put_tfm;
> > +   }
> 
> Shouldn't this be 'sizeof(*salg)'?

Good catch.  I'll fix this.

> > +   tfm = crypto_alloc_skcipher(basename, CRYPTO_ALG_INTERNAL,
> > +   CRYPTO_ALG_INTERNAL | CRYPTO_ALG_ASYNC);
> > +   if (IS_ERR(tfm))
> > +   return ERR_CAST(tfm);
> > +
> > +   ialg = crypto_skcipher_alg(tfm);
> 
> It seems this really just needs an algorithm and not a transform.  Perhaps it
> should be calling crypto_find_alg() directly?

crypto_find_alg is an internal API function that should not be used
unless you're implementing a crypto_type.  Ugh I see that it is being
abused already.  I'll get this fixed.

As a rule internal.h should only be used by API implementors, not
algorithm implementors.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [v2 PATCH 7/16] crypto: simd - Add simd skcipher helper

2016-11-13 Thread Eric Biggers
On Sun, Nov 13, 2016 at 07:45:38PM +0800, Herbert Xu wrote:
> This patch adds the simd skcipher helper which is meant to be
> a replacement for ablk helper.  It replaces the underlying blkcipher
> interface with skcipher, and also presents the top-level algorithm
> as an skcipher.

I assume this means it's planned for all users of ablk_helper to be migrated to
crypto_simd, and ablk_helper will be removed?

> + salg = kzalloc(sizeof(*alg), GFP_KERNEL);
> + if (!salg) {
> + salg = ERR_PTR(-ENOMEM);
> + goto out_put_tfm;
> + }

Shouldn't this be 'sizeof(*salg)'?

> + tfm = crypto_alloc_skcipher(basename, CRYPTO_ALG_INTERNAL,
> + CRYPTO_ALG_INTERNAL | CRYPTO_ALG_ASYNC);
> + if (IS_ERR(tfm))
> + return ERR_CAST(tfm);
> +
> + ialg = crypto_skcipher_alg(tfm);

It seems this really just needs an algorithm and not a transform.  Perhaps it
should be calling crypto_find_alg() directly?

> + err = -ENAMETOOLONG;
> + if (snprintf(alg->base.cra_name, CRYPTO_MAX_ALG_NAME, "%s", algname) >=
> + CRYPTO_MAX_ALG_NAME)
> + goto out_free_salg;
> +
> + if (snprintf(alg->base.cra_driver_name, CRYPTO_MAX_ALG_NAME, "%s",
> +  drvname) >= CRYPTO_MAX_ALG_NAME)
> + goto out_free_salg;

Could use strscpy() or strlcpy() here.

> +static int simd_skcipher_encrypt(struct skcipher_request *req)
> +{
> + struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
> + struct simd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
> + struct skcipher_request *subreq;
> + struct crypto_skcipher *child;
> +
> + subreq = skcipher_request_ctx(req);
> + *subreq = *req;
> +
> + if (!may_use_simd() ||
> + (in_atomic() && cryptd_skcipher_queued(ctx->cryptd_tfm)))
> + child = &ctx->cryptd_tfm->base;
> + else
> + child = cryptd_skcipher_child(ctx->cryptd_tfm);
> +
> + skcipher_request_set_tfm(subreq, child);
> +
> + return crypto_skcipher_encrypt(subreq);
> +}
> +
> +static int simd_skcipher_decrypt(struct skcipher_request *req)
> +{
> + struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
> + struct simd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
> + struct skcipher_request *subreq;
> + struct crypto_skcipher *child;
> +
> + subreq = skcipher_request_ctx(req);
> + *subreq = *req;
> +
> + if (!may_use_simd() ||
> + (in_atomic() && cryptd_skcipher_queued(ctx->cryptd_tfm)))
> + child = &ctx->cryptd_tfm->base;
> + else
> + child = cryptd_skcipher_child(ctx->cryptd_tfm);
> +
> + skcipher_request_set_tfm(subreq, child);
> +
> + return crypto_skcipher_decrypt(subreq);
> +}

These are the same except for the
crypto_skcipher_encrypt/crypto_skcipher_decrypt at the end, so they could be
mostly shared.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[v2 PATCH 7/16] crypto: simd - Add simd skcipher helper

2016-11-13 Thread Herbert Xu
This patch adds the simd skcipher helper which is meant to be
a replacement for ablk helper.  It replaces the underlying blkcipher
interface with skcipher, and also presents the top-level algorithm
as an skcipher.

Signed-off-by: Herbert Xu 
---

 crypto/Kconfig |4 
 crypto/Makefile|2 
 crypto/simd.c  |  226 +
 include/crypto/internal/simd.h |   17 +++
 4 files changed, 249 insertions(+)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 1db2a19..3d31181 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -246,6 +246,10 @@ config CRYPTO_ABLK_HELPER
tristate
select CRYPTO_CRYPTD
 
+config CRYPTO_SIMD
+   tristate
+   select CRYPTO_CRYPTD
+
 config CRYPTO_GLUE_HELPER_X86
tristate
depends on X86
diff --git a/crypto/Makefile b/crypto/Makefile
index 82ffeee..a05590e 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -142,3 +142,5 @@ obj-$(CONFIG_ASYNC_CORE) += async_tx/
 obj-$(CONFIG_ASYMMETRIC_KEY_TYPE) += asymmetric_keys/
 obj-$(CONFIG_CRYPTO_HASH_INFO) += hash_info.o
 obj-$(CONFIG_CRYPTO_ABLK_HELPER) += ablk_helper.o
+crypto_simd-y := simd.o
+obj-$(CONFIG_CRYPTO_SIMD) += crypto_simd.o
diff --git a/crypto/simd.c b/crypto/simd.c
new file mode 100644
index 000..2b10cbe
--- /dev/null
+++ b/crypto/simd.c
@@ -0,0 +1,226 @@
+/*
+ * Shared crypto simd helpers
+ *
+ * Copyright (c) 2012 Jussi Kivilinna 
+ * Copyright (c) 2016 Herbert Xu 
+ *
+ * Based on aesni-intel_glue.c by:
+ *  Copyright (C) 2008, Intel Corp.
+ *Author: Huang Ying 
+ *
+ * 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., 59 Temple Place, Suite 330, Boston, MA  02111-1307
+ * USA
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct simd_skcipher_alg {
+   const char *ialg_name;
+   struct skcipher_alg alg;
+};
+
+struct simd_skcipher_ctx {
+   struct cryptd_skcipher *cryptd_tfm;
+};
+
+static int simd_skcipher_setkey(struct crypto_skcipher *tfm, const u8 *key,
+   unsigned int key_len)
+{
+   struct simd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
+   struct crypto_skcipher *child = &ctx->cryptd_tfm->base;
+   int err;
+
+   crypto_skcipher_clear_flags(child, CRYPTO_TFM_REQ_MASK);
+   crypto_skcipher_set_flags(child, crypto_skcipher_get_flags(tfm) &
+CRYPTO_TFM_REQ_MASK);
+   err = crypto_skcipher_setkey(child, key, key_len);
+   crypto_skcipher_set_flags(tfm, crypto_skcipher_get_flags(child) &
+  CRYPTO_TFM_RES_MASK);
+   return err;
+}
+
+static int simd_skcipher_encrypt(struct skcipher_request *req)
+{
+   struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+   struct simd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
+   struct skcipher_request *subreq;
+   struct crypto_skcipher *child;
+
+   subreq = skcipher_request_ctx(req);
+   *subreq = *req;
+
+   if (!may_use_simd() ||
+   (in_atomic() && cryptd_skcipher_queued(ctx->cryptd_tfm)))
+   child = &ctx->cryptd_tfm->base;
+   else
+   child = cryptd_skcipher_child(ctx->cryptd_tfm);
+
+   skcipher_request_set_tfm(subreq, child);
+
+   return crypto_skcipher_encrypt(subreq);
+}
+
+static int simd_skcipher_decrypt(struct skcipher_request *req)
+{
+   struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+   struct simd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
+   struct skcipher_request *subreq;
+   struct crypto_skcipher *child;
+
+   subreq = skcipher_request_ctx(req);
+   *subreq = *req;
+
+   if (!may_use_simd() ||
+   (in_atomic() && cryptd_skcipher_queued(ctx->cryptd_tfm)))
+   child = &ctx->cryptd_tfm->base;
+   else
+   child = cryptd_skcipher_child(ctx->cryptd_tfm);
+
+   skcipher_request_set_tfm(subreq, child);
+
+   return crypto_skcipher_decrypt(subreq);
+}
+
+static void simd_skcipher_exit(struct crypto_skcipher *tfm)
+{
+   struct simd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
+
+   cryptd_free_skcipher(ctx->cryptd_tfm);
+}
+
+static int simd_skcipher_init(struct crypto_skcipher *tfm)
+{
+   struct simd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
+   struct cry