* [PATCH 1/5] crypto: akcipher - Add sync interface without SG lists
2023-06-15 10:26 ` [v2 PATCH " Herbert Xu
@ 2023-06-15 10:28 ` Herbert Xu
2023-06-15 10:28 ` [PATCH 2/5] crypto: sig - Add interface for sign/verify Herbert Xu
` (4 subsequent siblings)
5 siblings, 0 replies; 28+ messages in thread
From: Herbert Xu @ 2023-06-15 10:28 UTC (permalink / raw)
To: Linus Torvalds, Roberto Sassu, David Howells, Eric Biggers,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
Ard Biesheuvel, keyrings, Linux Crypto Mailing List
The only user of akcipher does not use SG lists. Therefore forcing
users to use SG lists only results unnecessary overhead. Add a new
interface that supports arbitrary kernel pointers.
For the time being the copy will be performed unconditionally. But
this will go away once the underlying interface is updated.
Note also that only encryption and decryption is addressed by this
patch as sign/verify will go into a new interface (sig).
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
crypto/akcipher.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++
include/crypto/akcipher.h | 36 +++++++++++++++++
2 files changed, 131 insertions(+)
diff --git a/crypto/akcipher.c b/crypto/akcipher.c
index 7960ceb528c3..2d10b58c4010 100644
--- a/crypto/akcipher.c
+++ b/crypto/akcipher.c
@@ -10,6 +10,7 @@
#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/scatterlist.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
#include <linux/string.h>
@@ -17,6 +18,19 @@
#include "internal.h"
+struct crypto_akcipher_sync_data {
+ struct crypto_akcipher *tfm;
+ const void *src;
+ void *dst;
+ unsigned int slen;
+ unsigned int dlen;
+
+ struct akcipher_request *req;
+ struct crypto_wait cwait;
+ struct scatterlist sg;
+ u8 *buf;
+};
+
static int __maybe_unused crypto_akcipher_report(
struct sk_buff *skb, struct crypto_alg *alg)
{
@@ -186,5 +200,86 @@ int akcipher_register_instance(struct crypto_template *tmpl,
}
EXPORT_SYMBOL_GPL(akcipher_register_instance);
+static int crypto_akcipher_sync_prep(struct crypto_akcipher_sync_data *data)
+{
+ unsigned int reqsize = crypto_akcipher_reqsize(data->tfm);
+ unsigned int mlen = max(data->slen, data->dlen);
+ struct akcipher_request *req;
+ struct scatterlist *sg;
+ unsigned int len;
+ u8 *buf;
+
+ len = sizeof(*req) + reqsize + mlen;
+ if (len < mlen)
+ return -EOVERFLOW;
+
+ req = kzalloc(len, GFP_KERNEL);
+ if (!req)
+ return -ENOMEM;
+
+ data->req = req;
+
+ buf = (u8 *)(req + 1) + reqsize;
+ data->buf = buf;
+ memcpy(buf, data->src, data->slen);
+
+ sg = &data->sg;
+ sg_init_one(sg, buf, mlen);
+ akcipher_request_set_crypt(req, sg, sg, data->slen, data->dlen);
+
+ crypto_init_wait(&data->cwait);
+ akcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
+ crypto_req_done, &data->cwait);
+
+ return 0;
+}
+
+static int crypto_akcipher_sync_post(struct crypto_akcipher_sync_data *data,
+ int err)
+{
+ err = crypto_wait_req(err, &data->cwait);
+ memcpy(data->dst, data->buf, data->dlen);
+ data->dlen = data->req->dst_len;
+ kfree_sensitive(data->req);
+ return err;
+}
+
+int crypto_akcipher_sync_encrypt(struct crypto_akcipher *tfm,
+ const void *src, unsigned int slen,
+ void *dst, unsigned int dlen)
+{
+ struct crypto_akcipher_sync_data data = {
+ .tfm = tfm,
+ .src = src,
+ .dst = dst,
+ .slen = slen,
+ .dlen = dlen,
+ };
+
+ return crypto_akcipher_sync_prep(&data) ?:
+ crypto_akcipher_sync_post(&data,
+ crypto_akcipher_encrypt(data.req));
+}
+EXPORT_SYMBOL_GPL(crypto_akcipher_sync_encrypt);
+
+int crypto_akcipher_sync_decrypt(struct crypto_akcipher *tfm,
+ const void *src, unsigned int slen,
+ void *dst, unsigned int dlen)
+{
+ struct crypto_akcipher_sync_data data = {
+ .tfm = tfm,
+ .src = src,
+ .dst = dst,
+ .slen = slen,
+ .dlen = dlen,
+ };
+
+ return crypto_akcipher_sync_prep(&data) ?:
+ crypto_akcipher_sync_post(&data,
+ crypto_akcipher_decrypt(data.req)) ?:
+ data.dlen;
+}
+EXPORT_SYMBOL_GPL(crypto_akcipher_sync_decrypt);
+
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("Generic public key cipher type");
diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
index f35fd653e4e5..670508f1dca1 100644
--- a/include/crypto/akcipher.h
+++ b/include/crypto/akcipher.h
@@ -373,6 +373,42 @@ static inline int crypto_akcipher_decrypt(struct akcipher_request *req)
return crypto_akcipher_errstat(alg, alg->decrypt(req));
}
+/**
+ * crypto_akcipher_sync_encrypt() - Invoke public key encrypt operation
+ *
+ * Function invokes the specific public key encrypt operation for a given
+ * public key algorithm
+ *
+ * @tfm: AKCIPHER tfm handle allocated with crypto_alloc_akcipher()
+ * @src: source buffer
+ * @slen: source length
+ * @dst: destinatino obuffer
+ * @dlen: destination length
+ *
+ * Return: zero on success; error code in case of error
+ */
+int crypto_akcipher_sync_encrypt(struct crypto_akcipher *tfm,
+ const void *src, unsigned int slen,
+ void *dst, unsigned int dlen);
+
+/**
+ * crypto_akcipher_sync_decrypt() - Invoke public key decrypt operation
+ *
+ * Function invokes the specific public key decrypt operation for a given
+ * public key algorithm
+ *
+ * @tfm: AKCIPHER tfm handle allocated with crypto_alloc_akcipher()
+ * @src: source buffer
+ * @slen: source length
+ * @dst: destinatino obuffer
+ * @dlen: destination length
+ *
+ * Return: Output length on success; error code in case of error
+ */
+int crypto_akcipher_sync_decrypt(struct crypto_akcipher *tfm,
+ const void *src, unsigned int slen,
+ void *dst, unsigned int dlen);
+
/**
* crypto_akcipher_sign() - Invoke public key sign operation
*
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 2/5] crypto: sig - Add interface for sign/verify
2023-06-15 10:26 ` [v2 PATCH " Herbert Xu
2023-06-15 10:28 ` [PATCH 1/5] crypto: akcipher - Add sync interface without SG lists Herbert Xu
@ 2023-06-15 10:28 ` Herbert Xu
2023-06-15 10:28 ` [PATCH 3/5] KEYS: Add forward declaration in asymmetric-parser.h Herbert Xu
` (3 subsequent siblings)
5 siblings, 0 replies; 28+ messages in thread
From: Herbert Xu @ 2023-06-15 10:28 UTC (permalink / raw)
To: Linus Torvalds, Roberto Sassu, David Howells, Eric Biggers,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
Ard Biesheuvel, keyrings, Linux Crypto Mailing List
Split out the sign/verify functionality from the existing akcipher
interface. Most algorithms in akcipher either support encryption
and decryption, or signing and verify. Only one supports both.
As a signature algorithm may not support encryption at all, these
two should be spearated.
For now sig is simply a wrapper around akcipher as all algorithms
remain unchanged. This is a first step and allows users to start
allocating sig instead of akcipher.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
crypto/Kconfig | 10 ++
crypto/Makefile | 1
crypto/akcipher.c | 53 +++++++++-----
crypto/internal.h | 20 +++++
crypto/sig.c | 159 ++++++++++++++++++++++++++++++++++++++++++
include/crypto/internal/sig.h | 17 ++++
include/crypto/sig.h | 140 ++++++++++++++++++++++++++++++++++++
include/linux/crypto.h | 3
8 files changed, 385 insertions(+), 18 deletions(-)
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 8b8bb97d1d77..650b1b3620d8 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -72,6 +72,15 @@ config CRYPTO_AEAD2
tristate
select CRYPTO_ALGAPI2
+config CRYPTO_SIG
+ tristate
+ select CRYPTO_SIG2
+ select CRYPTO_ALGAPI
+
+config CRYPTO_SIG2
+ tristate
+ select CRYPTO_ALGAPI2
+
config CRYPTO_SKCIPHER
tristate
select CRYPTO_SKCIPHER2
@@ -143,6 +152,7 @@ config CRYPTO_MANAGER2
select CRYPTO_ACOMP2
select CRYPTO_AEAD2
select CRYPTO_AKCIPHER2
+ select CRYPTO_SIG2
select CRYPTO_HASH2
select CRYPTO_KPP2
select CRYPTO_RNG2
diff --git a/crypto/Makefile b/crypto/Makefile
index 155ab671a1b4..953a7e105e58 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -25,6 +25,7 @@ crypto_hash-y += shash.o
obj-$(CONFIG_CRYPTO_HASH2) += crypto_hash.o
obj-$(CONFIG_CRYPTO_AKCIPHER2) += akcipher.o
+obj-$(CONFIG_CRYPTO_SIG2) += sig.o
obj-$(CONFIG_CRYPTO_KPP2) += kpp.o
dh_generic-y := dh.o
diff --git a/crypto/akcipher.c b/crypto/akcipher.c
index 2d10b58c4010..152cfba1346c 100644
--- a/crypto/akcipher.c
+++ b/crypto/akcipher.c
@@ -18,18 +18,7 @@
#include "internal.h"
-struct crypto_akcipher_sync_data {
- struct crypto_akcipher *tfm;
- const void *src;
- void *dst;
- unsigned int slen;
- unsigned int dlen;
-
- struct akcipher_request *req;
- struct crypto_wait cwait;
- struct scatterlist sg;
- u8 *buf;
-};
+#define CRYPTO_ALG_TYPE_AHASH_MASK 0x0000000e
static int __maybe_unused crypto_akcipher_report(
struct sk_buff *skb, struct crypto_alg *alg)
@@ -119,7 +108,7 @@ static const struct crypto_type crypto_akcipher_type = {
.report_stat = crypto_akcipher_report_stat,
#endif
.maskclear = ~CRYPTO_ALG_TYPE_MASK,
- .maskset = CRYPTO_ALG_TYPE_MASK,
+ .maskset = CRYPTO_ALG_TYPE_AHASH_MASK,
.type = CRYPTO_ALG_TYPE_AKCIPHER,
.tfmsize = offsetof(struct crypto_akcipher, base),
};
@@ -200,7 +189,7 @@ int akcipher_register_instance(struct crypto_template *tmpl,
}
EXPORT_SYMBOL_GPL(akcipher_register_instance);
-static int crypto_akcipher_sync_prep(struct crypto_akcipher_sync_data *data)
+int crypto_akcipher_sync_prep(struct crypto_akcipher_sync_data *data)
{
unsigned int reqsize = crypto_akcipher_reqsize(data->tfm);
unsigned int mlen = max(data->slen, data->dlen);
@@ -223,7 +212,7 @@ static int crypto_akcipher_sync_prep(struct crypto_akcipher_sync_data *data)
data->buf = buf;
memcpy(buf, data->src, data->slen);
- sg = &data->sg;
+ sg = data->sg;
sg_init_one(sg, buf, mlen);
akcipher_request_set_crypt(req, sg, sg, data->slen, data->dlen);
@@ -233,9 +222,9 @@ static int crypto_akcipher_sync_prep(struct crypto_akcipher_sync_data *data)
return 0;
}
+EXPORT_SYMBOL_GPL(crypto_akcipher_sync_prep);
-static int crypto_akcipher_sync_post(struct crypto_akcipher_sync_data *data,
- int err)
+int crypto_akcipher_sync_post(struct crypto_akcipher_sync_data *data, int err)
{
err = crypto_wait_req(err, &data->cwait);
memcpy(data->dst, data->buf, data->dlen);
@@ -243,6 +232,7 @@ static int crypto_akcipher_sync_post(struct crypto_akcipher_sync_data *data,
kfree_sensitive(data->req);
return err;
}
+EXPORT_SYMBOL_GPL(crypto_akcipher_sync_post);
int crypto_akcipher_sync_encrypt(struct crypto_akcipher *tfm,
const void *src, unsigned int slen,
@@ -281,5 +271,34 @@ int crypto_akcipher_sync_decrypt(struct crypto_akcipher *tfm,
}
EXPORT_SYMBOL_GPL(crypto_akcipher_sync_decrypt);
+static void crypto_exit_akcipher_ops_sig(struct crypto_tfm *tfm)
+{
+ struct crypto_akcipher **ctx = crypto_tfm_ctx(tfm);
+
+ crypto_free_akcipher(*ctx);
+}
+
+int crypto_init_akcipher_ops_sig(struct crypto_tfm *tfm)
+{
+ struct crypto_akcipher **ctx = crypto_tfm_ctx(tfm);
+ struct crypto_alg *calg = tfm->__crt_alg;
+ struct crypto_akcipher *akcipher;
+
+ if (!crypto_mod_get(calg))
+ return -EAGAIN;
+
+ akcipher = crypto_create_tfm(calg, &crypto_akcipher_type);
+ if (IS_ERR(akcipher)) {
+ crypto_mod_put(calg);
+ return PTR_ERR(akcipher);
+ }
+
+ *ctx = akcipher;
+ tfm->exit = crypto_exit_akcipher_ops_sig;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(crypto_init_akcipher_ops_sig);
+
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("Generic public key cipher type");
diff --git a/crypto/internal.h b/crypto/internal.h
index 8dd746b1130b..dc8bff1720fe 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -18,9 +18,12 @@
#include <linux/numa.h>
#include <linux/refcount.h>
#include <linux/rwsem.h>
+#include <linux/scatterlist.h>
#include <linux/sched.h>
#include <linux/types.h>
+struct akcipher_request;
+struct crypto_akcipher;
struct crypto_instance;
struct crypto_template;
@@ -32,6 +35,19 @@ struct crypto_larval {
bool test_started;
};
+struct crypto_akcipher_sync_data {
+ struct crypto_akcipher *tfm;
+ const void *src;
+ void *dst;
+ unsigned int slen;
+ unsigned int dlen;
+
+ struct akcipher_request *req;
+ struct crypto_wait cwait;
+ struct scatterlist sg[2];
+ u8 *buf;
+};
+
enum {
CRYPTOA_UNSPEC,
CRYPTOA_ALG,
@@ -109,6 +125,10 @@ void *crypto_create_tfm_node(struct crypto_alg *alg,
void *crypto_clone_tfm(const struct crypto_type *frontend,
struct crypto_tfm *otfm);
+int crypto_akcipher_sync_prep(struct crypto_akcipher_sync_data *data);
+int crypto_akcipher_sync_post(struct crypto_akcipher_sync_data *data, int err);
+int crypto_init_akcipher_ops_sig(struct crypto_tfm *tfm);
+
static inline void *crypto_create_tfm(struct crypto_alg *alg,
const struct crypto_type *frontend)
{
diff --git a/crypto/sig.c b/crypto/sig.c
new file mode 100644
index 000000000000..d812555c88af
--- /dev/null
+++ b/crypto/sig.c
@@ -0,0 +1,159 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Public Key Signature Algorithm
+ *
+ * Copyright (c) 2023 Herbert Xu <herbert@gondor.apana.org.au>
+ */
+
+#include <crypto/akcipher.h>
+#include <crypto/internal/sig.h>
+#include <linux/cryptouser.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/scatterlist.h>
+#include <linux/seq_file.h>
+#include <linux/string.h>
+#include <net/netlink.h>
+
+#include "internal.h"
+
+#define CRYPTO_ALG_TYPE_SIG_MASK 0x0000000e
+
+static const struct crypto_type crypto_sig_type;
+
+static inline struct crypto_sig *__crypto_sig_tfm(struct crypto_tfm *tfm)
+{
+ return container_of(tfm, struct crypto_sig, base);
+}
+
+static int crypto_sig_init_tfm(struct crypto_tfm *tfm)
+{
+ if (tfm->__crt_alg->cra_type != &crypto_sig_type)
+ return crypto_init_akcipher_ops_sig(tfm);
+
+ return 0;
+}
+
+static void __maybe_unused crypto_sig_show(struct seq_file *m,
+ struct crypto_alg *alg)
+{
+ seq_puts(m, "type : sig\n");
+}
+
+static int __maybe_unused crypto_sig_report(struct sk_buff *skb,
+ struct crypto_alg *alg)
+{
+ struct crypto_report_akcipher rsig = {};
+
+ strscpy(rsig.type, "sig", sizeof(rsig.type));
+
+ return nla_put(skb, CRYPTOCFGA_REPORT_AKCIPHER, sizeof(rsig), &rsig);
+}
+
+static int __maybe_unused crypto_sig_report_stat(struct sk_buff *skb,
+ struct crypto_alg *alg)
+{
+ struct crypto_stat_akcipher rsig = {};
+
+ strscpy(rsig.type, "sig", sizeof(rsig.type));
+
+ return nla_put(skb, CRYPTOCFGA_STAT_AKCIPHER, sizeof(rsig), &rsig);
+}
+
+static const struct crypto_type crypto_sig_type = {
+ .extsize = crypto_alg_extsize,
+ .init_tfm = crypto_sig_init_tfm,
+#ifdef CONFIG_PROC_FS
+ .show = crypto_sig_show,
+#endif
+#if IS_ENABLED(CONFIG_CRYPTO_USER)
+ .report = crypto_sig_report,
+#endif
+#ifdef CONFIG_CRYPTO_STATS
+ .report_stat = crypto_sig_report_stat,
+#endif
+ .maskclear = ~CRYPTO_ALG_TYPE_MASK,
+ .maskset = CRYPTO_ALG_TYPE_SIG_MASK,
+ .type = CRYPTO_ALG_TYPE_SIG,
+ .tfmsize = offsetof(struct crypto_sig, base),
+};
+
+struct crypto_sig *crypto_alloc_sig(const char *alg_name, u32 type, u32 mask)
+{
+ return crypto_alloc_tfm(alg_name, &crypto_sig_type, type, mask);
+}
+EXPORT_SYMBOL_GPL(crypto_alloc_sig);
+
+int crypto_sig_maxsize(struct crypto_sig *tfm)
+{
+ struct crypto_akcipher **ctx = crypto_sig_ctx(tfm);
+
+ return crypto_akcipher_maxsize(*ctx);
+}
+EXPORT_SYMBOL_GPL(crypto_sig_maxsize);
+
+int crypto_sig_sign(struct crypto_sig *tfm,
+ const void *src, unsigned int slen,
+ void *dst, unsigned int dlen)
+{
+ struct crypto_akcipher **ctx = crypto_sig_ctx(tfm);
+ struct crypto_akcipher_sync_data data = {
+ .tfm = *ctx,
+ .src = src,
+ .dst = dst,
+ .slen = slen,
+ .dlen = dlen,
+ };
+
+ return crypto_akcipher_sync_prep(&data) ?:
+ crypto_akcipher_sync_post(&data,
+ crypto_akcipher_sign(data.req));
+}
+EXPORT_SYMBOL_GPL(crypto_sig_sign);
+
+int crypto_sig_verify(struct crypto_sig *tfm,
+ const void *src, unsigned int slen,
+ const void *digest, unsigned int dlen)
+{
+ struct crypto_akcipher **ctx = crypto_sig_ctx(tfm);
+ struct crypto_akcipher_sync_data data = {
+ .tfm = *ctx,
+ .src = src,
+ .slen = slen,
+ .dlen = dlen,
+ };
+ int err;
+
+ err = crypto_akcipher_sync_prep(&data);
+ if (err)
+ return err;
+
+ sg_init_table(data.sg, 2);
+ sg_set_buf(&data.sg[0], src, slen);
+ sg_set_buf(&data.sg[1], digest, dlen);
+
+ return crypto_akcipher_sync_post(&data,
+ crypto_akcipher_verify(data.req));
+}
+EXPORT_SYMBOL_GPL(crypto_sig_verify);
+
+int crypto_sig_set_pubkey(struct crypto_sig *tfm,
+ const void *key, unsigned int keylen)
+{
+ struct crypto_akcipher **ctx = crypto_sig_ctx(tfm);
+
+ return crypto_akcipher_set_pub_key(*ctx, key, keylen);
+}
+EXPORT_SYMBOL_GPL(crypto_sig_set_pubkey);
+
+int crypto_sig_set_privkey(struct crypto_sig *tfm,
+ const void *key, unsigned int keylen)
+{
+ struct crypto_akcipher **ctx = crypto_sig_ctx(tfm);
+
+ return crypto_akcipher_set_priv_key(*ctx, key, keylen);
+}
+EXPORT_SYMBOL_GPL(crypto_sig_set_privkey);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Public Key Signature Algorithms");
diff --git a/include/crypto/internal/sig.h b/include/crypto/internal/sig.h
new file mode 100644
index 000000000000..97cb26ef8115
--- /dev/null
+++ b/include/crypto/internal/sig.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Public Key Signature Algorithm
+ *
+ * Copyright (c) 2023 Herbert Xu <herbert@gondor.apana.org.au>
+ */
+#ifndef _CRYPTO_INTERNAL_SIG_H
+#define _CRYPTO_INTERNAL_SIG_H
+
+#include <crypto/algapi.h>
+#include <crypto/sig.h>
+
+static inline void *crypto_sig_ctx(struct crypto_sig *tfm)
+{
+ return crypto_tfm_ctx(&tfm->base);
+}
+#endif
diff --git a/include/crypto/sig.h b/include/crypto/sig.h
new file mode 100644
index 000000000000..641b4714c448
--- /dev/null
+++ b/include/crypto/sig.h
@@ -0,0 +1,140 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Public Key Signature Algorithm
+ *
+ * Copyright (c) 2023 Herbert Xu <herbert@gondor.apana.org.au>
+ */
+#ifndef _CRYPTO_SIG_H
+#define _CRYPTO_SIG_H
+
+#include <linux/crypto.h>
+
+/**
+ * struct crypto_sig - user-instantiated objects which encapsulate
+ * algorithms and core processing logic
+ *
+ * @base: Common crypto API algorithm data structure
+ */
+struct crypto_sig {
+ struct crypto_tfm base;
+};
+
+/**
+ * DOC: Generic Public Key Signature API
+ *
+ * The Public Key Signature API is used with the algorithms of type
+ * CRYPTO_ALG_TYPE_SIG (listed as type "sig" in /proc/crypto)
+ */
+
+/**
+ * crypto_alloc_sig() - allocate signature tfm handle
+ * @alg_name: is the cra_name / name or cra_driver_name / driver name of the
+ * signing algorithm e.g. "ecdsa"
+ * @type: specifies the type of the algorithm
+ * @mask: specifies the mask for the algorithm
+ *
+ * Allocate a handle for public key signature algorithm. The returned struct
+ * crypto_sig is the handle that is required for any subsequent
+ * API invocation for signature operations.
+ *
+ * Return: allocated handle in case of success; IS_ERR() is true in case
+ * of an error, PTR_ERR() returns the error code.
+ */
+struct crypto_sig *crypto_alloc_sig(const char *alg_name, u32 type, u32 mask);
+
+static inline struct crypto_tfm *crypto_sig_tfm(struct crypto_sig *tfm)
+{
+ return &tfm->base;
+}
+
+/**
+ * crypto_free_sig() - free signature tfm handle
+ *
+ * @tfm: signature tfm handle allocated with crypto_alloc_sig()
+ *
+ * If @tfm is a NULL or error pointer, this function does nothing.
+ */
+static inline void crypto_free_sig(struct crypto_sig *tfm)
+{
+ crypto_destroy_tfm(tfm, crypto_sig_tfm(tfm));
+}
+
+/**
+ * crypto_sig_maxsize() - Get len for output buffer
+ *
+ * Function returns the dest buffer size required for a given key.
+ * Function assumes that the key is already set in the transformation. If this
+ * function is called without a setkey or with a failed setkey, you will end up
+ * in a NULL dereference.
+ *
+ * @tfm: signature tfm handle allocated with crypto_alloc_sig()
+ */
+int crypto_sig_maxsize(struct crypto_sig *tfm);
+
+/**
+ * crypto_sig_sign() - Invoke signing operation
+ *
+ * Function invokes the specific signing operation for a given algorithm
+ *
+ * @tfm: signature tfm handle allocated with crypto_alloc_sig()
+ * @src: source buffer
+ * @slen: source length
+ * @dst: destinatino obuffer
+ * @dlen: destination length
+ *
+ * Return: zero on success; error code in case of error
+ */
+int crypto_sig_sign(struct crypto_sig *tfm,
+ const void *src, unsigned int slen,
+ void *dst, unsigned int dlen);
+
+/**
+ * crypto_sig_verify() - Invoke signature verification
+ *
+ * Function invokes the specific signature verification operation
+ * for a given algorithm.
+ *
+ * @tfm: signature tfm handle allocated with crypto_alloc_sig()
+ * @src: source buffer
+ * @slen: source length
+ * @digest: digest
+ * @dlen: digest length
+ *
+ * Return: zero on verification success; error code in case of error.
+ */
+int crypto_sig_verify(struct crypto_sig *tfm,
+ const void *src, unsigned int slen,
+ const void *digest, unsigned int dlen);
+
+/**
+ * crypto_sig_set_pubkey() - Invoke set public key operation
+ *
+ * Function invokes the algorithm specific set key function, which knows
+ * how to decode and interpret the encoded key and parameters
+ *
+ * @tfm: tfm handle
+ * @key: BER encoded public key, algo OID, paramlen, BER encoded
+ * parameters
+ * @keylen: length of the key (not including other data)
+ *
+ * Return: zero on success; error code in case of error
+ */
+int crypto_sig_set_pubkey(struct crypto_sig *tfm,
+ const void *key, unsigned int keylen);
+
+/**
+ * crypto_sig_set_privkey() - Invoke set private key operation
+ *
+ * Function invokes the algorithm specific set key function, which knows
+ * how to decode and interpret the encoded key and parameters
+ *
+ * @tfm: tfm handle
+ * @key: BER encoded private key, algo OID, paramlen, BER encoded
+ * parameters
+ * @keylen: length of the key (not including other data)
+ *
+ * Return: zero on success; error code in case of error
+ */
+int crypto_sig_set_privkey(struct crypto_sig *tfm,
+ const void *key, unsigned int keylen);
+#endif
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index fa310ac1db59..31f6fee0c36c 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -25,11 +25,12 @@
#define CRYPTO_ALG_TYPE_COMPRESS 0x00000002
#define CRYPTO_ALG_TYPE_AEAD 0x00000003
#define CRYPTO_ALG_TYPE_SKCIPHER 0x00000005
+#define CRYPTO_ALG_TYPE_AKCIPHER 0x00000006
+#define CRYPTO_ALG_TYPE_SIG 0x00000007
#define CRYPTO_ALG_TYPE_KPP 0x00000008
#define CRYPTO_ALG_TYPE_ACOMPRESS 0x0000000a
#define CRYPTO_ALG_TYPE_SCOMPRESS 0x0000000b
#define CRYPTO_ALG_TYPE_RNG 0x0000000c
-#define CRYPTO_ALG_TYPE_AKCIPHER 0x0000000d
#define CRYPTO_ALG_TYPE_HASH 0x0000000e
#define CRYPTO_ALG_TYPE_SHASH 0x0000000e
#define CRYPTO_ALG_TYPE_AHASH 0x0000000f
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 3/5] KEYS: Add forward declaration in asymmetric-parser.h
2023-06-15 10:26 ` [v2 PATCH " Herbert Xu
2023-06-15 10:28 ` [PATCH 1/5] crypto: akcipher - Add sync interface without SG lists Herbert Xu
2023-06-15 10:28 ` [PATCH 2/5] crypto: sig - Add interface for sign/verify Herbert Xu
@ 2023-06-15 10:28 ` Herbert Xu
2023-06-15 10:28 ` [PATCH 4/5] KEYS: asymmetric: Move sm2 code into x509_public_key Herbert Xu
` (2 subsequent siblings)
5 siblings, 0 replies; 28+ messages in thread
From: Herbert Xu @ 2023-06-15 10:28 UTC (permalink / raw)
To: Linus Torvalds, Roberto Sassu, David Howells, Eric Biggers,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
Ard Biesheuvel, keyrings, Linux Crypto Mailing List
Add forward declaration for struct key_preparsed_payload so that
this header file is self-contained.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
include/keys/asymmetric-parser.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/keys/asymmetric-parser.h b/include/keys/asymmetric-parser.h
index c47dc5405f79..516a3f51179e 100644
--- a/include/keys/asymmetric-parser.h
+++ b/include/keys/asymmetric-parser.h
@@ -10,6 +10,8 @@
#ifndef _KEYS_ASYMMETRIC_PARSER_H
#define _KEYS_ASYMMETRIC_PARSER_H
+struct key_preparsed_payload;
+
/*
* Key data parser. Called during key instantiation.
*/
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 4/5] KEYS: asymmetric: Move sm2 code into x509_public_key
2023-06-15 10:26 ` [v2 PATCH " Herbert Xu
` (2 preceding siblings ...)
2023-06-15 10:28 ` [PATCH 3/5] KEYS: Add forward declaration in asymmetric-parser.h Herbert Xu
@ 2023-06-15 10:28 ` Herbert Xu
2023-06-15 10:28 ` [PATCH 5/5] KEYS: asymmetric: Use new crypto interface without scatterlists Herbert Xu
2023-06-26 9:21 ` [v2 PATCH 0/5] crypto: Add akcipher interface without SGs Ard Biesheuvel
5 siblings, 0 replies; 28+ messages in thread
From: Herbert Xu @ 2023-06-15 10:28 UTC (permalink / raw)
To: Linus Torvalds, Roberto Sassu, David Howells, Eric Biggers,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
Ard Biesheuvel, keyrings, Linux Crypto Mailing List
The sm2 certificate requires a modified digest. Move the code
for the hashing from the signature verification path into the
code where we generate the digest.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
crypto/asymmetric_keys/public_key.c | 67 -------------------
crypto/asymmetric_keys/x509_public_key.c | 29 ++++++--
crypto/sm2.c | 106 ++++++++++++++++++++-----------
include/crypto/public_key.h | 2
include/crypto/sm2.h | 12 ---
5 files changed, 94 insertions(+), 122 deletions(-)
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index eca5671ad3f2..c795a12a3599 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -18,8 +18,6 @@
#include <keys/asymmetric-subtype.h>
#include <crypto/public_key.h>
#include <crypto/akcipher.h>
-#include <crypto/sm2.h>
-#include <crypto/sm3_base.h>
MODULE_DESCRIPTION("In-software asymmetric public-key subtype");
MODULE_AUTHOR("Red Hat, Inc.");
@@ -312,65 +310,6 @@ static int software_key_eds_op(struct kernel_pkey_params *params,
return ret;
}
-#if IS_REACHABLE(CONFIG_CRYPTO_SM2)
-static int cert_sig_digest_update(const struct public_key_signature *sig,
- struct crypto_akcipher *tfm_pkey)
-{
- struct crypto_shash *tfm;
- struct shash_desc *desc;
- size_t desc_size;
- unsigned char dgst[SM3_DIGEST_SIZE];
- int ret;
-
- BUG_ON(!sig->data);
-
- /* SM2 signatures always use the SM3 hash algorithm */
- if (!sig->hash_algo || strcmp(sig->hash_algo, "sm3") != 0)
- return -EINVAL;
-
- ret = sm2_compute_z_digest(tfm_pkey, SM2_DEFAULT_USERID,
- SM2_DEFAULT_USERID_LEN, dgst);
- if (ret)
- return ret;
-
- tfm = crypto_alloc_shash(sig->hash_algo, 0, 0);
- if (IS_ERR(tfm))
- return PTR_ERR(tfm);
-
- desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
- desc = kzalloc(desc_size, GFP_KERNEL);
- if (!desc) {
- ret = -ENOMEM;
- goto error_free_tfm;
- }
-
- desc->tfm = tfm;
-
- ret = crypto_shash_init(desc);
- if (ret < 0)
- goto error_free_desc;
-
- ret = crypto_shash_update(desc, dgst, SM3_DIGEST_SIZE);
- if (ret < 0)
- goto error_free_desc;
-
- ret = crypto_shash_finup(desc, sig->data, sig->data_size, sig->digest);
-
-error_free_desc:
- kfree(desc);
-error_free_tfm:
- crypto_free_shash(tfm);
- return ret;
-}
-#else
-static inline int cert_sig_digest_update(
- const struct public_key_signature *sig,
- struct crypto_akcipher *tfm_pkey)
-{
- return -ENOTSUPP;
-}
-#endif /* ! IS_REACHABLE(CONFIG_CRYPTO_SM2) */
-
/*
* Verify a signature using a public key.
*/
@@ -438,12 +377,6 @@ int public_key_verify_signature(const struct public_key *pkey,
if (ret)
goto error_free_key;
- if (strcmp(pkey->pkey_algo, "sm2") == 0 && sig->data_size) {
- ret = cert_sig_digest_update(sig, tfm);
- if (ret)
- goto error_free_key;
- }
-
sg_init_table(src_sg, 2);
sg_set_buf(&src_sg[0], sig->s, sig->s_size);
sg_set_buf(&src_sg[1], sig->digest, sig->digest_size);
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 0b4943a4592b..6fdfc82e23a8 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -6,13 +6,15 @@
*/
#define pr_fmt(fmt) "X.509: "fmt
+#include <crypto/hash.h>
+#include <crypto/sm2.h>
+#include <keys/asymmetric-parser.h>
+#include <keys/asymmetric-subtype.h>
+#include <keys/system_keyring.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/slab.h>
-#include <keys/asymmetric-subtype.h>
-#include <keys/asymmetric-parser.h>
-#include <keys/system_keyring.h>
-#include <crypto/hash.h>
+#include <linux/string.h>
#include "asymmetric_keys.h"
#include "x509_parser.h"
@@ -30,9 +32,6 @@ int x509_get_sig_params(struct x509_certificate *cert)
pr_devel("==>%s()\n", __func__);
- sig->data = cert->tbs;
- sig->data_size = cert->tbs_size;
-
sig->s = kmemdup(cert->raw_sig, cert->raw_sig_size, GFP_KERNEL);
if (!sig->s)
return -ENOMEM;
@@ -65,7 +64,21 @@ int x509_get_sig_params(struct x509_certificate *cert)
desc->tfm = tfm;
- ret = crypto_shash_digest(desc, cert->tbs, cert->tbs_size, sig->digest);
+ if (strcmp(cert->pub->pkey_algo, "sm2") == 0) {
+ ret = strcmp(sig->hash_algo, "sm3") != 0 ? -EINVAL :
+ crypto_shash_init(desc) ?:
+ sm2_compute_z_digest(desc, cert->pub->key,
+ cert->pub->keylen, sig->digest) ?:
+ crypto_shash_init(desc) ?:
+ crypto_shash_update(desc, sig->digest,
+ sig->digest_size) ?:
+ crypto_shash_finup(desc, cert->tbs, cert->tbs_size,
+ sig->digest);
+ } else {
+ ret = crypto_shash_digest(desc, cert->tbs, cert->tbs_size,
+ sig->digest);
+ }
+
if (ret < 0)
goto error_2;
diff --git a/crypto/sm2.c b/crypto/sm2.c
index ed9307dac3d1..285b3cb7c0bc 100644
--- a/crypto/sm2.c
+++ b/crypto/sm2.c
@@ -13,11 +13,14 @@
#include <crypto/internal/akcipher.h>
#include <crypto/akcipher.h>
#include <crypto/hash.h>
-#include <crypto/sm3.h>
#include <crypto/rng.h>
#include <crypto/sm2.h>
#include "sm2signature.asn1.h"
+/* The default user id as specified in GM/T 0009-2012 */
+#define SM2_DEFAULT_USERID "1234567812345678"
+#define SM2_DEFAULT_USERID_LEN 16
+
#define MPI_NBYTES(m) ((mpi_get_nbits(m) + 7) / 8)
struct ecc_domain_parms {
@@ -60,6 +63,9 @@ static const struct ecc_domain_parms sm2_ecp = {
.h = 1
};
+static int __sm2_set_pub_key(struct mpi_ec_ctx *ec,
+ const void *key, unsigned int keylen);
+
static int sm2_ec_ctx_init(struct mpi_ec_ctx *ec)
{
const struct ecc_domain_parms *ecp = &sm2_ecp;
@@ -213,12 +219,13 @@ int sm2_get_signature_s(void *context, size_t hdrlen, unsigned char tag,
return 0;
}
-static int sm2_z_digest_update(struct sm3_state *sctx,
- MPI m, unsigned int pbytes)
+static int sm2_z_digest_update(struct shash_desc *desc,
+ MPI m, unsigned int pbytes)
{
static const unsigned char zero[32];
unsigned char *in;
unsigned int inlen;
+ int err;
in = mpi_get_buffer(m, &inlen, NULL);
if (!in)
@@ -226,21 +233,22 @@ static int sm2_z_digest_update(struct sm3_state *sctx,
if (inlen < pbytes) {
/* padding with zero */
- sm3_update(sctx, zero, pbytes - inlen);
- sm3_update(sctx, in, inlen);
+ err = crypto_shash_update(desc, zero, pbytes - inlen) ?:
+ crypto_shash_update(desc, in, inlen);
} else if (inlen > pbytes) {
/* skip the starting zero */
- sm3_update(sctx, in + inlen - pbytes, pbytes);
+ err = crypto_shash_update(desc, in + inlen - pbytes, pbytes);
} else {
- sm3_update(sctx, in, inlen);
+ err = crypto_shash_update(desc, in, inlen);
}
kfree(in);
- return 0;
+ return err;
}
-static int sm2_z_digest_update_point(struct sm3_state *sctx,
- MPI_POINT point, struct mpi_ec_ctx *ec, unsigned int pbytes)
+static int sm2_z_digest_update_point(struct shash_desc *desc,
+ MPI_POINT point, struct mpi_ec_ctx *ec,
+ unsigned int pbytes)
{
MPI x, y;
int ret = -EINVAL;
@@ -248,50 +256,68 @@ static int sm2_z_digest_update_point(struct sm3_state *sctx,
x = mpi_new(0);
y = mpi_new(0);
- if (!mpi_ec_get_affine(x, y, point, ec) &&
- !sm2_z_digest_update(sctx, x, pbytes) &&
- !sm2_z_digest_update(sctx, y, pbytes))
- ret = 0;
+ ret = mpi_ec_get_affine(x, y, point, ec) ? -EINVAL :
+ sm2_z_digest_update(desc, x, pbytes) ?:
+ sm2_z_digest_update(desc, y, pbytes);
mpi_free(x);
mpi_free(y);
return ret;
}
-int sm2_compute_z_digest(struct crypto_akcipher *tfm,
- const unsigned char *id, size_t id_len,
- unsigned char dgst[SM3_DIGEST_SIZE])
+int sm2_compute_z_digest(struct shash_desc *desc,
+ const void *key, unsigned int keylen, void *dgst)
{
- struct mpi_ec_ctx *ec = akcipher_tfm_ctx(tfm);
- uint16_t bits_len;
- unsigned char entl[2];
- struct sm3_state sctx;
+ struct mpi_ec_ctx *ec;
+ unsigned int bits_len;
unsigned int pbytes;
+ u8 entl[2];
+ int err;
- if (id_len > (USHRT_MAX / 8) || !ec->Q)
- return -EINVAL;
+ ec = kmalloc(sizeof(*ec), GFP_KERNEL);
+ if (!ec)
+ return -ENOMEM;
+
+ err = __sm2_set_pub_key(ec, key, keylen);
+ if (err)
+ goto out_free_ec;
- bits_len = (uint16_t)(id_len * 8);
+ bits_len = SM2_DEFAULT_USERID_LEN * 8;
entl[0] = bits_len >> 8;
entl[1] = bits_len & 0xff;
pbytes = MPI_NBYTES(ec->p);
/* ZA = H256(ENTLA | IDA | a | b | xG | yG | xA | yA) */
- sm3_init(&sctx);
- sm3_update(&sctx, entl, 2);
- sm3_update(&sctx, id, id_len);
-
- if (sm2_z_digest_update(&sctx, ec->a, pbytes) ||
- sm2_z_digest_update(&sctx, ec->b, pbytes) ||
- sm2_z_digest_update_point(&sctx, ec->G, ec, pbytes) ||
- sm2_z_digest_update_point(&sctx, ec->Q, ec, pbytes))
- return -EINVAL;
+ err = crypto_shash_init(desc);
+ if (err)
+ goto out_deinit_ec;
- sm3_final(&sctx, dgst);
- return 0;
+ err = crypto_shash_update(desc, entl, 2);
+ if (err)
+ goto out_deinit_ec;
+
+ err = crypto_shash_update(desc, SM2_DEFAULT_USERID,
+ SM2_DEFAULT_USERID_LEN);
+ if (err)
+ goto out_deinit_ec;
+
+ err = sm2_z_digest_update(desc, ec->a, pbytes) ?:
+ sm2_z_digest_update(desc, ec->b, pbytes) ?:
+ sm2_z_digest_update_point(desc, ec->G, ec, pbytes) ?:
+ sm2_z_digest_update_point(desc, ec->Q, ec, pbytes);
+ if (err)
+ goto out_deinit_ec;
+
+ err = crypto_shash_final(desc, dgst);
+
+out_deinit_ec:
+ sm2_ec_ctx_deinit(ec);
+out_free_ec:
+ kfree(ec);
+ return err;
}
-EXPORT_SYMBOL(sm2_compute_z_digest);
+EXPORT_SYMBOL_GPL(sm2_compute_z_digest);
static int _sm2_verify(struct mpi_ec_ctx *ec, MPI hash, MPI sig_r, MPI sig_s)
{
@@ -391,6 +417,14 @@ static int sm2_set_pub_key(struct crypto_akcipher *tfm,
const void *key, unsigned int keylen)
{
struct mpi_ec_ctx *ec = akcipher_tfm_ctx(tfm);
+
+ return __sm2_set_pub_key(ec, key, keylen);
+
+}
+
+static int __sm2_set_pub_key(struct mpi_ec_ctx *ec,
+ const void *key, unsigned int keylen)
+{
MPI a;
int rc;
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index 653992a6e941..8fadd561c50e 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -48,8 +48,6 @@ struct public_key_signature {
const char *pkey_algo;
const char *hash_algo;
const char *encoding;
- const void *data;
- unsigned int data_size;
};
extern void public_key_signature_free(struct public_key_signature *sig);
diff --git a/include/crypto/sm2.h b/include/crypto/sm2.h
index af452556dcd4..7094d75ed54c 100644
--- a/include/crypto/sm2.h
+++ b/include/crypto/sm2.h
@@ -11,15 +11,9 @@
#ifndef _CRYPTO_SM2_H
#define _CRYPTO_SM2_H
-#include <crypto/sm3.h>
-#include <crypto/akcipher.h>
+struct shash_desc;
-/* The default user id as specified in GM/T 0009-2012 */
-#define SM2_DEFAULT_USERID "1234567812345678"
-#define SM2_DEFAULT_USERID_LEN 16
-
-extern int sm2_compute_z_digest(struct crypto_akcipher *tfm,
- const unsigned char *id, size_t id_len,
- unsigned char dgst[SM3_DIGEST_SIZE]);
+int sm2_compute_z_digest(struct shash_desc *desc,
+ const void *key, unsigned int keylen, void *dgst);
#endif /* _CRYPTO_SM2_H */
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 5/5] KEYS: asymmetric: Use new crypto interface without scatterlists
2023-06-15 10:26 ` [v2 PATCH " Herbert Xu
` (3 preceding siblings ...)
2023-06-15 10:28 ` [PATCH 4/5] KEYS: asymmetric: Move sm2 code into x509_public_key Herbert Xu
@ 2023-06-15 10:28 ` Herbert Xu
2023-06-26 9:21 ` [v2 PATCH 0/5] crypto: Add akcipher interface without SGs Ard Biesheuvel
5 siblings, 0 replies; 28+ messages in thread
From: Herbert Xu @ 2023-06-15 10:28 UTC (permalink / raw)
To: Linus Torvalds, Roberto Sassu, David Howells, Eric Biggers,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
Ard Biesheuvel, keyrings, Linux Crypto Mailing List
Use the new akcipher and sig interfaces which no longer have
scatterlists in them.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
crypto/asymmetric_keys/public_key.c | 234 +++++++++++++++++++++---------------
1 file changed, 137 insertions(+), 97 deletions(-)
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index c795a12a3599..e787598cb3f7 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -8,16 +8,17 @@
*/
#define pr_fmt(fmt) "PKEY: "fmt
-#include <linux/module.h>
-#include <linux/export.h>
+#include <crypto/akcipher.h>
+#include <crypto/public_key.h>
+#include <crypto/sig.h>
+#include <keys/asymmetric-subtype.h>
+#include <linux/asn1.h>
+#include <linux/err.h>
#include <linux/kernel.h>
-#include <linux/slab.h>
+#include <linux/module.h>
#include <linux/seq_file.h>
-#include <linux/scatterlist.h>
-#include <linux/asn1.h>
-#include <keys/asymmetric-subtype.h>
-#include <crypto/public_key.h>
-#include <crypto/akcipher.h>
+#include <linux/slab.h>
+#include <linux/string.h>
MODULE_DESCRIPTION("In-software asymmetric public-key subtype");
MODULE_AUTHOR("Red Hat, Inc.");
@@ -65,10 +66,13 @@ static void public_key_destroy(void *payload0, void *payload3)
static int
software_key_determine_akcipher(const struct public_key *pkey,
const char *encoding, const char *hash_algo,
- char alg_name[CRYPTO_MAX_ALG_NAME])
+ char alg_name[CRYPTO_MAX_ALG_NAME], bool *sig,
+ enum kernel_pkey_operation op)
{
int n;
+ *sig = true;
+
if (!encoding)
return -EINVAL;
@@ -77,14 +81,18 @@ software_key_determine_akcipher(const struct public_key *pkey,
* RSA signatures usually use EMSA-PKCS1-1_5 [RFC3447 sec 8.2].
*/
if (strcmp(encoding, "pkcs1") == 0) {
- if (!hash_algo)
+ if (!hash_algo) {
+ *sig = false;
n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
"pkcs1pad(%s)",
pkey->pkey_algo);
- else
+ } else {
+ *sig = op == kernel_pkey_sign ||
+ op == kernel_pkey_verify;
n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
"pkcs1pad(%s,%s)",
pkey->pkey_algo, hash_algo);
+ }
return n >= CRYPTO_MAX_ALG_NAME ? -EINVAL : 0;
}
if (strcmp(encoding, "raw") != 0)
@@ -95,6 +103,7 @@ software_key_determine_akcipher(const struct public_key *pkey,
*/
if (hash_algo)
return -EINVAL;
+ *sig = false;
} else if (strncmp(pkey->pkey_algo, "ecdsa", 5) == 0) {
if (strcmp(encoding, "x962") != 0)
return -EINVAL;
@@ -152,37 +161,70 @@ static int software_key_query(const struct kernel_pkey_params *params,
struct crypto_akcipher *tfm;
struct public_key *pkey = params->key->payload.data[asym_crypto];
char alg_name[CRYPTO_MAX_ALG_NAME];
+ struct crypto_sig *sig;
u8 *key, *ptr;
int ret, len;
+ bool issig;
ret = software_key_determine_akcipher(pkey, params->encoding,
- params->hash_algo, alg_name);
+ params->hash_algo, alg_name,
+ &issig, kernel_pkey_sign);
if (ret < 0)
return ret;
- tfm = crypto_alloc_akcipher(alg_name, 0, 0);
- if (IS_ERR(tfm))
- return PTR_ERR(tfm);
-
- ret = -ENOMEM;
key = kmalloc(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen,
GFP_KERNEL);
if (!key)
- goto error_free_tfm;
+ return -ENOMEM;
+
memcpy(key, pkey->key, pkey->keylen);
ptr = key + pkey->keylen;
ptr = pkey_pack_u32(ptr, pkey->algo);
ptr = pkey_pack_u32(ptr, pkey->paramlen);
memcpy(ptr, pkey->params, pkey->paramlen);
- if (pkey->key_is_private)
- ret = crypto_akcipher_set_priv_key(tfm, key, pkey->keylen);
- else
- ret = crypto_akcipher_set_pub_key(tfm, key, pkey->keylen);
- if (ret < 0)
- goto error_free_key;
+ if (issig) {
+ sig = crypto_alloc_sig(alg_name, 0, 0);
+ if (IS_ERR(sig))
+ goto error_free_key;
+
+ if (pkey->key_is_private)
+ ret = crypto_sig_set_privkey(sig, key, pkey->keylen);
+ else
+ ret = crypto_sig_set_pubkey(sig, key, pkey->keylen);
+ if (ret < 0)
+ goto error_free_tfm;
+
+ len = crypto_sig_maxsize(sig);
+
+ info->supported_ops = KEYCTL_SUPPORTS_VERIFY;
+ if (pkey->key_is_private)
+ info->supported_ops |= KEYCTL_SUPPORTS_SIGN;
+
+ if (strcmp(params->encoding, "pkcs1") == 0) {
+ info->supported_ops |= KEYCTL_SUPPORTS_ENCRYPT;
+ if (pkey->key_is_private)
+ info->supported_ops |= KEYCTL_SUPPORTS_DECRYPT;
+ }
+ } else {
+ tfm = crypto_alloc_akcipher(alg_name, 0, 0);
+ if (IS_ERR(tfm))
+ goto error_free_key;
+
+ if (pkey->key_is_private)
+ ret = crypto_akcipher_set_priv_key(tfm, key, pkey->keylen);
+ else
+ ret = crypto_akcipher_set_pub_key(tfm, key, pkey->keylen);
+ if (ret < 0)
+ goto error_free_tfm;
+
+ len = crypto_akcipher_maxsize(tfm);
+
+ info->supported_ops = KEYCTL_SUPPORTS_ENCRYPT;
+ if (pkey->key_is_private)
+ info->supported_ops |= KEYCTL_SUPPORTS_DECRYPT;
+ }
- len = crypto_akcipher_maxsize(tfm);
info->key_size = len * 8;
if (strncmp(pkey->pkey_algo, "ecdsa", 5) == 0) {
@@ -208,17 +250,16 @@ static int software_key_query(const struct kernel_pkey_params *params,
info->max_enc_size = len;
info->max_dec_size = len;
- info->supported_ops = (KEYCTL_SUPPORTS_ENCRYPT |
- KEYCTL_SUPPORTS_VERIFY);
- if (pkey->key_is_private)
- info->supported_ops |= (KEYCTL_SUPPORTS_DECRYPT |
- KEYCTL_SUPPORTS_SIGN);
+
ret = 0;
+error_free_tfm:
+ if (issig)
+ crypto_free_sig(sig);
+ else
+ crypto_free_akcipher(tfm);
error_free_key:
kfree(key);
-error_free_tfm:
- crypto_free_akcipher(tfm);
pr_devel("<==%s() = %d\n", __func__, ret);
return ret;
}
@@ -230,34 +271,26 @@ static int software_key_eds_op(struct kernel_pkey_params *params,
const void *in, void *out)
{
const struct public_key *pkey = params->key->payload.data[asym_crypto];
- struct akcipher_request *req;
- struct crypto_akcipher *tfm;
- struct crypto_wait cwait;
- struct scatterlist in_sg, out_sg;
char alg_name[CRYPTO_MAX_ALG_NAME];
+ struct crypto_akcipher *tfm;
+ struct crypto_sig *sig;
char *key, *ptr;
+ bool issig;
+ int ksz;
int ret;
pr_devel("==>%s()\n", __func__);
ret = software_key_determine_akcipher(pkey, params->encoding,
- params->hash_algo, alg_name);
+ params->hash_algo, alg_name,
+ &issig, params->op);
if (ret < 0)
return ret;
- tfm = crypto_alloc_akcipher(alg_name, 0, 0);
- if (IS_ERR(tfm))
- return PTR_ERR(tfm);
-
- ret = -ENOMEM;
- req = akcipher_request_alloc(tfm, GFP_KERNEL);
- if (!req)
- goto error_free_tfm;
-
key = kmalloc(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen,
GFP_KERNEL);
if (!key)
- goto error_free_req;
+ return -ENOMEM;
memcpy(key, pkey->key, pkey->keylen);
ptr = key + pkey->keylen;
@@ -265,47 +298,70 @@ static int software_key_eds_op(struct kernel_pkey_params *params,
ptr = pkey_pack_u32(ptr, pkey->paramlen);
memcpy(ptr, pkey->params, pkey->paramlen);
- if (pkey->key_is_private)
- ret = crypto_akcipher_set_priv_key(tfm, key, pkey->keylen);
- else
- ret = crypto_akcipher_set_pub_key(tfm, key, pkey->keylen);
- if (ret)
- goto error_free_key;
+ if (issig) {
+ sig = crypto_alloc_sig(alg_name, 0, 0);
+ if (IS_ERR(sig))
+ goto error_free_key;
+
+ if (pkey->key_is_private)
+ ret = crypto_sig_set_privkey(sig, key, pkey->keylen);
+ else
+ ret = crypto_sig_set_pubkey(sig, key, pkey->keylen);
+ if (ret)
+ goto error_free_tfm;
+
+ ksz = crypto_sig_maxsize(sig);
+ } else {
+ tfm = crypto_alloc_akcipher(alg_name, 0, 0);
+ if (IS_ERR(tfm))
+ goto error_free_key;
+
+ if (pkey->key_is_private)
+ ret = crypto_akcipher_set_priv_key(tfm, key, pkey->keylen);
+ else
+ ret = crypto_akcipher_set_pub_key(tfm, key, pkey->keylen);
+ if (ret)
+ goto error_free_tfm;
+
+ ksz = crypto_akcipher_maxsize(tfm);
+ }
- sg_init_one(&in_sg, in, params->in_len);
- sg_init_one(&out_sg, out, params->out_len);
- akcipher_request_set_crypt(req, &in_sg, &out_sg, params->in_len,
- params->out_len);
- crypto_init_wait(&cwait);
- akcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
- CRYPTO_TFM_REQ_MAY_SLEEP,
- crypto_req_done, &cwait);
+ ret = -EINVAL;
/* Perform the encryption calculation. */
switch (params->op) {
case kernel_pkey_encrypt:
- ret = crypto_akcipher_encrypt(req);
+ if (issig)
+ break;
+ ret = crypto_akcipher_sync_encrypt(tfm, in, params->in_len,
+ out, params->out_len);
break;
case kernel_pkey_decrypt:
- ret = crypto_akcipher_decrypt(req);
+ if (issig)
+ break;
+ ret = crypto_akcipher_sync_decrypt(tfm, in, params->in_len,
+ out, params->out_len);
break;
case kernel_pkey_sign:
- ret = crypto_akcipher_sign(req);
+ if (!issig)
+ break;
+ ret = crypto_sig_sign(sig, in, params->in_len,
+ out, params->out_len);
break;
default:
BUG();
}
- ret = crypto_wait_req(ret, &cwait);
if (ret == 0)
- ret = req->dst_len;
+ ret = ksz;
+error_free_tfm:
+ if (issig)
+ crypto_free_sig(sig);
+ else
+ crypto_free_akcipher(tfm);
error_free_key:
kfree(key);
-error_free_req:
- akcipher_request_free(req);
-error_free_tfm:
- crypto_free_akcipher(tfm);
pr_devel("<==%s() = %d\n", __func__, ret);
return ret;
}
@@ -316,12 +372,10 @@ static int software_key_eds_op(struct kernel_pkey_params *params,
int public_key_verify_signature(const struct public_key *pkey,
const struct public_key_signature *sig)
{
- struct crypto_wait cwait;
- struct crypto_akcipher *tfm;
- struct akcipher_request *req;
- struct scatterlist src_sg[2];
char alg_name[CRYPTO_MAX_ALG_NAME];
+ struct crypto_sig *tfm;
char *key, *ptr;
+ bool issig;
int ret;
pr_devel("==>%s()\n", __func__);
@@ -346,23 +400,19 @@ int public_key_verify_signature(const struct public_key *pkey,
}
ret = software_key_determine_akcipher(pkey, sig->encoding,
- sig->hash_algo, alg_name);
+ sig->hash_algo, alg_name,
+ &issig, kernel_pkey_verify);
if (ret < 0)
return ret;
- tfm = crypto_alloc_akcipher(alg_name, 0, 0);
+ tfm = crypto_alloc_sig(alg_name, 0, 0);
if (IS_ERR(tfm))
return PTR_ERR(tfm);
- ret = -ENOMEM;
- req = akcipher_request_alloc(tfm, GFP_KERNEL);
- if (!req)
- goto error_free_tfm;
-
key = kmalloc(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen,
GFP_KERNEL);
if (!key)
- goto error_free_req;
+ goto error_free_tfm;
memcpy(key, pkey->key, pkey->keylen);
ptr = key + pkey->keylen;
@@ -371,29 +421,19 @@ int public_key_verify_signature(const struct public_key *pkey,
memcpy(ptr, pkey->params, pkey->paramlen);
if (pkey->key_is_private)
- ret = crypto_akcipher_set_priv_key(tfm, key, pkey->keylen);
+ ret = crypto_sig_set_privkey(tfm, key, pkey->keylen);
else
- ret = crypto_akcipher_set_pub_key(tfm, key, pkey->keylen);
+ ret = crypto_sig_set_pubkey(tfm, key, pkey->keylen);
if (ret)
goto error_free_key;
- sg_init_table(src_sg, 2);
- sg_set_buf(&src_sg[0], sig->s, sig->s_size);
- sg_set_buf(&src_sg[1], sig->digest, sig->digest_size);
- akcipher_request_set_crypt(req, src_sg, NULL, sig->s_size,
- sig->digest_size);
- crypto_init_wait(&cwait);
- akcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
- CRYPTO_TFM_REQ_MAY_SLEEP,
- crypto_req_done, &cwait);
- ret = crypto_wait_req(crypto_akcipher_verify(req), &cwait);
+ ret = crypto_sig_verify(tfm, sig->s, sig->s_size,
+ sig->digest, sig->digest_size);
error_free_key:
kfree(key);
-error_free_req:
- akcipher_request_free(req);
error_free_tfm:
- crypto_free_akcipher(tfm);
+ crypto_free_sig(tfm);
pr_devel("<==%s() = %d\n", __func__, ret);
if (WARN_ON_ONCE(ret > 0))
ret = -EINVAL;
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [v2 PATCH 0/5] crypto: Add akcipher interface without SGs
2023-06-15 10:26 ` [v2 PATCH " Herbert Xu
` (4 preceding siblings ...)
2023-06-15 10:28 ` [PATCH 5/5] KEYS: asymmetric: Use new crypto interface without scatterlists Herbert Xu
@ 2023-06-26 9:21 ` Ard Biesheuvel
2023-06-26 9:52 ` Herbert Xu
5 siblings, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2023-06-26 9:21 UTC (permalink / raw)
To: Herbert Xu
Cc: David Howells, Linus Torvalds, Roberto Sassu, Eric Biggers,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
keyrings, Linux Crypto Mailing List
On Thu, 15 Jun 2023 at 12:26, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> v2 changes:
>
> - Rename dsa to sig.
> - Add braces around else clause.
>
> The crypto akcipher interface has exactly one user, the keyring
> subsystem. That user only deals with kernel pointers, not SG lists.
> Therefore the use of SG lists in the akcipher interface is
> completely pointless.
>
> As there is only one user, changing it isn't that hard. This
> patch series is a first step in that direction. It introduces
> a new interface for encryption and decryption without SG lists:
>
> int crypto_akcipher_sync_encrypt(struct crypto_akcipher *tfm,
> const void *src, unsigned int slen,
> void *dst, unsigned int dlen);
>
> int crypto_akcipher_sync_decrypt(struct crypto_akcipher *tfm,
> const void *src, unsigned int slen,
> void *dst, unsigned int dlen);
>
> I've decided to split out signing and verification because most
> (all but one) of our signature algorithms do not support encryption
> or decryption. These can now be accessed through the sig interface:
>
> int crypto_sig_sign(struct crypto_sig *tfm,
> const void *src, unsigned int slen,
> void *dst, unsigned int dlen);
>
> int crypto_sig_verify(struct crypto_sig *tfm,
> const void *src, unsigned int slen,
> const void *digest, unsigned int dlen);
>
> The keyring system has been converted to this interface.
>
This looks like a worthwhile improvement to me.
As I asked before, could we do the same for the acomp API? The only
existing user blocks on the completion, and the vast majority of
implementations is software only.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [v2 PATCH 0/5] crypto: Add akcipher interface without SGs
2023-06-26 9:21 ` [v2 PATCH 0/5] crypto: Add akcipher interface without SGs Ard Biesheuvel
@ 2023-06-26 9:52 ` Herbert Xu
2023-06-26 10:03 ` Ard Biesheuvel
0 siblings, 1 reply; 28+ messages in thread
From: Herbert Xu @ 2023-06-26 9:52 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: David Howells, Linus Torvalds, Roberto Sassu, Eric Biggers,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
keyrings, Linux Crypto Mailing List
On Mon, Jun 26, 2023 at 11:21:20AM +0200, Ard Biesheuvel wrote:
>
> As I asked before, could we do the same for the acomp API? The only
> existing user blocks on the completion, and the vast majority of
> implementations is software only.
We already have scomp. It's not exported so we could export it.
However, compression is quite different because it was one of the
original network stack algorithms (IPcomp). So SG will always be
needed.
In fact, if anything SG fits quite well with decompression because
it would allow us to allocate pages as we go, avoiding the need
to allocate a large chunk of memory at the start as we do today.
We don't make use of that ability today but that's something that
I'd like to address.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v2 PATCH 0/5] crypto: Add akcipher interface without SGs
2023-06-26 9:52 ` Herbert Xu
@ 2023-06-26 10:03 ` Ard Biesheuvel
2023-06-26 10:13 ` Herbert Xu
0 siblings, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2023-06-26 10:03 UTC (permalink / raw)
To: Herbert Xu
Cc: David Howells, Linus Torvalds, Roberto Sassu, Eric Biggers,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
keyrings, Linux Crypto Mailing List
On Mon, 26 Jun 2023 at 11:53, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Mon, Jun 26, 2023 at 11:21:20AM +0200, Ard Biesheuvel wrote:
> >
> > As I asked before, could we do the same for the acomp API? The only
> > existing user blocks on the completion, and the vast majority of
> > implementations is software only.
>
> We already have scomp. It's not exported so we could export it.
>
> However, compression is quite different because it was one of the
> original network stack algorithms (IPcomp). So SG will always be
> needed.
>
> In fact, if anything SG fits quite well with decompression because
> it would allow us to allocate pages as we go, avoiding the need
> to allocate a large chunk of memory at the start as we do today.
>
> We don't make use of that ability today but that's something that
> I'd like to address.
>
OK, so you are saying that you'd like to see the code in
net/xfrm/xfrm_ipcomp.c ported to acomp, so that IPcomp can be backed
using h/w offload?
Would that be a tidying up exercise? Or does that actually address a
real-world use case?
In any case, what I would like to see addressed is the horrid scomp to
acomp layer that ties up megabytes of memory in scratch space, just to
emulate the acomp interface on top of scomp drivers, while no code
exists that makes use of the async nature. Do you have an idea on how
we might address this particular issue?
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v2 PATCH 0/5] crypto: Add akcipher interface without SGs
2023-06-26 10:03 ` Ard Biesheuvel
@ 2023-06-26 10:13 ` Herbert Xu
2023-06-28 6:21 ` Eric Biggers
0 siblings, 1 reply; 28+ messages in thread
From: Herbert Xu @ 2023-06-26 10:13 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: David Howells, Linus Torvalds, Roberto Sassu, Eric Biggers,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
keyrings, Linux Crypto Mailing List
On Mon, Jun 26, 2023 at 12:03:04PM +0200, Ard Biesheuvel wrote:
>
> In any case, what I would like to see addressed is the horrid scomp to
> acomp layer that ties up megabytes of memory in scratch space, just to
> emulate the acomp interface on top of scomp drivers, while no code
> exists that makes use of the async nature. Do you have an idea on how
> we might address this particular issue?
The whole reason why need to allocate megabytes of memory is because
of the lack of SG lists in the underlying algorithm. If they
actually used SG lists and allocated pages as they went during
decompression, then we wouldn't need to pre-allocate any memory
at all.
It's not going to be easy of course since we need to convert
every single algorithm. Perhaps what we could do in the mean
time is to get the scomp implementations do exponential retries
(e.g., start with allocating size * 2, if that fails then double
it, up to whatever we currently pre-allocate).
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v2 PATCH 0/5] crypto: Add akcipher interface without SGs
2023-06-26 10:13 ` Herbert Xu
@ 2023-06-28 6:21 ` Eric Biggers
2023-06-28 16:58 ` Ard Biesheuvel
0 siblings, 1 reply; 28+ messages in thread
From: Eric Biggers @ 2023-06-28 6:21 UTC (permalink / raw)
To: Herbert Xu
Cc: Ard Biesheuvel, David Howells, Linus Torvalds, Roberto Sassu,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
keyrings, Linux Crypto Mailing List
On Mon, Jun 26, 2023 at 06:13:44PM +0800, Herbert Xu wrote:
> On Mon, Jun 26, 2023 at 12:03:04PM +0200, Ard Biesheuvel wrote:
> >
> > In any case, what I would like to see addressed is the horrid scomp to
> > acomp layer that ties up megabytes of memory in scratch space, just to
> > emulate the acomp interface on top of scomp drivers, while no code
> > exists that makes use of the async nature. Do you have an idea on how
> > we might address this particular issue?
>
> The whole reason why need to allocate megabytes of memory is because
> of the lack of SG lists in the underlying algorithm. If they
> actually used SG lists and allocated pages as they went during
> decompression, then we wouldn't need to pre-allocate any memory
> at all.
I don't think that is a realistic expectation. Decompressors generally need a
contiguous buffer for decompressed data anyway, up to a certain size which is
32KB for DEFLATE but can be much larger for the more modern algorithms. This is
because they decode "matches" that refer to previously decompressed data by
offset, and it has to be possible to index the data efficiently.
(Some decompressors, e.g. zlib, provide "streaming" APIs where you can read
arbitrary amounts. But that works by actually decompressing into an internal
buffer that has sufficient size, then copying to the user provided buffer.)
The same applies to compressors too, with regards to the original data.
I think the "input/output is a list of pages" model just fundamentally does not
work well for software compression and decompression. To support it, either
large temporary buffers are needed (they might be hidden inside the
(de)compressor, but they are there), or vmap() or vm_map_ram() is needed.
FWIW, f2fs compression uses vm_map_ram() and skips the crypto API entirely...
If acomp has to be kept for the hardware support, then maybe its scomp backend
should use vm_map_ram() instead of scratch buffers?
- Eric
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v2 PATCH 0/5] crypto: Add akcipher interface without SGs
2023-06-28 6:21 ` Eric Biggers
@ 2023-06-28 16:58 ` Ard Biesheuvel
2023-06-28 17:33 ` Eric Biggers
0 siblings, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2023-06-28 16:58 UTC (permalink / raw)
To: Eric Biggers
Cc: Herbert Xu, David Howells, Linus Torvalds, Roberto Sassu,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
keyrings, Linux Crypto Mailing List
On Wed, 28 Jun 2023 at 08:21, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Mon, Jun 26, 2023 at 06:13:44PM +0800, Herbert Xu wrote:
> > On Mon, Jun 26, 2023 at 12:03:04PM +0200, Ard Biesheuvel wrote:
> > >
> > > In any case, what I would like to see addressed is the horrid scomp to
> > > acomp layer that ties up megabytes of memory in scratch space, just to
> > > emulate the acomp interface on top of scomp drivers, while no code
> > > exists that makes use of the async nature. Do you have an idea on how
> > > we might address this particular issue?
> >
> > The whole reason why need to allocate megabytes of memory is because
> > of the lack of SG lists in the underlying algorithm. If they
> > actually used SG lists and allocated pages as they went during
> > decompression, then we wouldn't need to pre-allocate any memory
> > at all.
>
> I don't think that is a realistic expectation. Decompressors generally need a
> contiguous buffer for decompressed data anyway, up to a certain size which is
> 32KB for DEFLATE but can be much larger for the more modern algorithms. This is
> because they decode "matches" that refer to previously decompressed data by
> offset, and it has to be possible to index the data efficiently.
>
> (Some decompressors, e.g. zlib, provide "streaming" APIs where you can read
> arbitrary amounts. But that works by actually decompressing into an internal
> buffer that has sufficient size, then copying to the user provided buffer.)
>
> The same applies to compressors too, with regards to the original data.
>
> I think the "input/output is a list of pages" model just fundamentally does not
> work well for software compression and decompression. To support it, either
> large temporary buffers are needed (they might be hidden inside the
> (de)compressor, but they are there), or vmap() or vm_map_ram() is needed.
>
> FWIW, f2fs compression uses vm_map_ram() and skips the crypto API entirely...
>
> If acomp has to be kept for the hardware support, then maybe its scomp backend
> should use vm_map_ram() instead of scratch buffers?
>
Yeah, but we'll run into similar issues related to the fact that
scatterlists can describe arbitrary sequences of sub-page size memory
chunks, which means vmap()ing the pages may not be sufficient to get a
virtual linear representation of the buffers.
With zswap being the only current user, which uses a single contiguous
buffers for decompression out of place, and blocks on the completion,
the level of additional complexity we have in the acomp stack is mind
boggling. And the scomp-to-acomp adaptation layer, with its fixed size
per-CPU in and output buffer (implying that acomp in/output has a
hardcoded size limit) which are never freed makes it rather
unpalatable to me tbh.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v2 PATCH 0/5] crypto: Add akcipher interface without SGs
2023-06-28 16:58 ` Ard Biesheuvel
@ 2023-06-28 17:33 ` Eric Biggers
2023-06-28 17:44 ` Ard Biesheuvel
0 siblings, 1 reply; 28+ messages in thread
From: Eric Biggers @ 2023-06-28 17:33 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Herbert Xu, David Howells, Linus Torvalds, Roberto Sassu,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
keyrings, Linux Crypto Mailing List
On Wed, Jun 28, 2023 at 06:58:58PM +0200, Ard Biesheuvel wrote:
> On Wed, 28 Jun 2023 at 08:21, Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Mon, Jun 26, 2023 at 06:13:44PM +0800, Herbert Xu wrote:
> > > On Mon, Jun 26, 2023 at 12:03:04PM +0200, Ard Biesheuvel wrote:
> > > >
> > > > In any case, what I would like to see addressed is the horrid scomp to
> > > > acomp layer that ties up megabytes of memory in scratch space, just to
> > > > emulate the acomp interface on top of scomp drivers, while no code
> > > > exists that makes use of the async nature. Do you have an idea on how
> > > > we might address this particular issue?
> > >
> > > The whole reason why need to allocate megabytes of memory is because
> > > of the lack of SG lists in the underlying algorithm. If they
> > > actually used SG lists and allocated pages as they went during
> > > decompression, then we wouldn't need to pre-allocate any memory
> > > at all.
> >
> > I don't think that is a realistic expectation. Decompressors generally need a
> > contiguous buffer for decompressed data anyway, up to a certain size which is
> > 32KB for DEFLATE but can be much larger for the more modern algorithms. This is
> > because they decode "matches" that refer to previously decompressed data by
> > offset, and it has to be possible to index the data efficiently.
> >
> > (Some decompressors, e.g. zlib, provide "streaming" APIs where you can read
> > arbitrary amounts. But that works by actually decompressing into an internal
> > buffer that has sufficient size, then copying to the user provided buffer.)
> >
> > The same applies to compressors too, with regards to the original data.
> >
> > I think the "input/output is a list of pages" model just fundamentally does not
> > work well for software compression and decompression. To support it, either
> > large temporary buffers are needed (they might be hidden inside the
> > (de)compressor, but they are there), or vmap() or vm_map_ram() is needed.
> >
> > FWIW, f2fs compression uses vm_map_ram() and skips the crypto API entirely...
> >
> > If acomp has to be kept for the hardware support, then maybe its scomp backend
> > should use vm_map_ram() instead of scratch buffers?
> >
>
> Yeah, but we'll run into similar issues related to the fact that
> scatterlists can describe arbitrary sequences of sub-page size memory
> chunks, which means vmap()ing the pages may not be sufficient to get a
> virtual linear representation of the buffers.
Yes, that is annoying... Maybe the acomp API should not support arbitrary
scatterlists, but rather only ones that can be mapped contiguously?
>
> With zswap being the only current user, which uses a single contiguous
> buffers for decompression out of place, and blocks on the completion,
> the level of additional complexity we have in the acomp stack is mind
> boggling. And the scomp-to-acomp adaptation layer, with its fixed size
> per-CPU in and output buffer (implying that acomp in/output has a
> hardcoded size limit) which are never freed makes it rather
> unpalatable to me tbh.
Either way, I think the way out may be that zcomp should support *both* the
scomp and acomp APIs. It should use scomp by default, and if someone *really*
wants to use a hardware (de)compression accelerator, they can configure it to
use acomp.
Also, I see that crypto/scompress.c currently allocates scratch buffers (256KB
per CPU!) whenever any crypto_scomp tfm is initialized. That seems wrong. It
should only happen when a crypto_acomp tfm that is backed by a crypto_scomp tfm
is initialized. Then, if acomp is not used, all this craziness will be avoided.
- Eric
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v2 PATCH 0/5] crypto: Add akcipher interface without SGs
2023-06-28 17:33 ` Eric Biggers
@ 2023-06-28 17:44 ` Ard Biesheuvel
2023-06-28 17:55 ` Linus Torvalds
0 siblings, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2023-06-28 17:44 UTC (permalink / raw)
To: Eric Biggers
Cc: Herbert Xu, David Howells, Linus Torvalds, Roberto Sassu,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
keyrings, Linux Crypto Mailing List
On Wed, 28 Jun 2023 at 19:33, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Wed, Jun 28, 2023 at 06:58:58PM +0200, Ard Biesheuvel wrote:
> > On Wed, 28 Jun 2023 at 08:21, Eric Biggers <ebiggers@kernel.org> wrote:
> > >
> > > On Mon, Jun 26, 2023 at 06:13:44PM +0800, Herbert Xu wrote:
> > > > On Mon, Jun 26, 2023 at 12:03:04PM +0200, Ard Biesheuvel wrote:
> > > > >
> > > > > In any case, what I would like to see addressed is the horrid scomp to
> > > > > acomp layer that ties up megabytes of memory in scratch space, just to
> > > > > emulate the acomp interface on top of scomp drivers, while no code
> > > > > exists that makes use of the async nature. Do you have an idea on how
> > > > > we might address this particular issue?
> > > >
> > > > The whole reason why need to allocate megabytes of memory is because
> > > > of the lack of SG lists in the underlying algorithm. If they
> > > > actually used SG lists and allocated pages as they went during
> > > > decompression, then we wouldn't need to pre-allocate any memory
> > > > at all.
> > >
> > > I don't think that is a realistic expectation. Decompressors generally need a
> > > contiguous buffer for decompressed data anyway, up to a certain size which is
> > > 32KB for DEFLATE but can be much larger for the more modern algorithms. This is
> > > because they decode "matches" that refer to previously decompressed data by
> > > offset, and it has to be possible to index the data efficiently.
> > >
> > > (Some decompressors, e.g. zlib, provide "streaming" APIs where you can read
> > > arbitrary amounts. But that works by actually decompressing into an internal
> > > buffer that has sufficient size, then copying to the user provided buffer.)
> > >
> > > The same applies to compressors too, with regards to the original data.
> > >
> > > I think the "input/output is a list of pages" model just fundamentally does not
> > > work well for software compression and decompression. To support it, either
> > > large temporary buffers are needed (they might be hidden inside the
> > > (de)compressor, but they are there), or vmap() or vm_map_ram() is needed.
> > >
> > > FWIW, f2fs compression uses vm_map_ram() and skips the crypto API entirely...
> > >
> > > If acomp has to be kept for the hardware support, then maybe its scomp backend
> > > should use vm_map_ram() instead of scratch buffers?
> > >
> >
> > Yeah, but we'll run into similar issues related to the fact that
> > scatterlists can describe arbitrary sequences of sub-page size memory
> > chunks, which means vmap()ing the pages may not be sufficient to get a
> > virtual linear representation of the buffers.
>
> Yes, that is annoying... Maybe the acomp API should not support arbitrary
> scatterlists, but rather only ones that can be mapped contiguously?
>
Herbert seems to think that this could be useful for IPcomp as well,
so it would depend on whether that would also work under this
restriction.
> >
> > With zswap being the only current user, which uses a single contiguous
> > buffers for decompression out of place, and blocks on the completion,
> > the level of additional complexity we have in the acomp stack is mind
> > boggling. And the scomp-to-acomp adaptation layer, with its fixed size
> > per-CPU in and output buffer (implying that acomp in/output has a
> > hardcoded size limit) which are never freed makes it rather
> > unpalatable to me tbh.
>
> Either way, I think the way out may be that zcomp should support *both* the
> scomp and acomp APIs. It should use scomp by default, and if someone *really*
> wants to use a hardware (de)compression accelerator, they can configure it to
> use acomp.
>
Both the sole acomp h/w implementation and the zswap acomp conversion
were contributed by HiSilicon, and the code is synchronous.
So perhaps that driver should expose (blocking) scomp as well as
acomp, and zswap can just use scomp with virtual addresses.
> Also, I see that crypto/scompress.c currently allocates scratch buffers (256KB
> per CPU!) whenever any crypto_scomp tfm is initialized. That seems wrong. It
> should only happen when a crypto_acomp tfm that is backed by a crypto_scomp tfm
> is initialized. Then, if acomp is not used, all this craziness will be avoided.
>
Yeah, this is why I am trying to do something about this. I have a 32
core 4 way SMT ThunderX2 workstation, and so 32 MiB of physical memory
is permanently tied up this way, which is just silly (IIRC this is due
to pstore compression allocating a compression TFM upfront so it can
compress dmesg on a panic - I've already argued with Kees that
pstore's selection of compression algorithms is rather pointless in
any case, and it should just use the GZIP library)
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v2 PATCH 0/5] crypto: Add akcipher interface without SGs
2023-06-28 17:44 ` Ard Biesheuvel
@ 2023-06-28 17:55 ` Linus Torvalds
2023-06-28 18:34 ` David Howells
0 siblings, 1 reply; 28+ messages in thread
From: Linus Torvalds @ 2023-06-28 17:55 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Eric Biggers, Herbert Xu, David Howells, Roberto Sassu,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
keyrings, Linux Crypto Mailing List
On Wed, 28 Jun 2023 at 10:45, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Both the sole acomp h/w implementation and the zswap acomp conversion
> were contributed by HiSilicon, and the code is synchronous.
I really think people need to realize that the age of async external
accelerators is long long gone.
Yes, it was cool in the 80s.
But dammit, so was big hair, mullets, and Sony Walkmans.
Just give it up. It's a complete failure, and it is not making a come-back.
What is still relevant is:
(a) inline accelerators
Doing checksumming, big packets, encryption etc ON THE NETWORK
CARD as it is being sent out and received is still very much relevant.
But not this crazy "CPU gives data to external accelerator, does
something else, and is notified of the result and goes back to look at
it" is bogus and completely wrong.
(b) synchronous CPU accelerated routines
Whether this is using just vector instructions, or special
hardware, they are synchronous, and they use CPU virtual addresses
directly.
and nothing else matters.
Christ, people, Metallica even wrote one of the greatest song of all
time about it! How much more proof do you need?
I seriously believe that the crypto layer should basically aim to get
rid of the silly async interface entirely. It adds no real value, and
it has caused endless amounts of pain.
Linus
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [v2 PATCH 0/5] crypto: Add akcipher interface without SGs
2023-06-28 17:55 ` Linus Torvalds
@ 2023-06-28 18:34 ` David Howells
2023-06-28 20:10 ` Linus Torvalds
0 siblings, 1 reply; 28+ messages in thread
From: David Howells @ 2023-06-28 18:34 UTC (permalink / raw)
To: Linus Torvalds
Cc: dhowells, Ard Biesheuvel, Eric Biggers, Herbert Xu, Roberto Sassu,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
keyrings, Linux Crypto Mailing List
What about something like the Intel on-die accelerators (e.g. IAA and QAT)? I
think they can do async compression.
David
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [v2 PATCH 0/5] crypto: Add akcipher interface without SGs
2023-06-28 18:34 ` David Howells
@ 2023-06-28 20:10 ` Linus Torvalds
2023-06-29 4:49 ` Gao Xiang
0 siblings, 1 reply; 28+ messages in thread
From: Linus Torvalds @ 2023-06-28 20:10 UTC (permalink / raw)
To: David Howells
Cc: Ard Biesheuvel, Eric Biggers, Herbert Xu, Roberto Sassu,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
keyrings, Linux Crypto Mailing List
On Wed, 28 Jun 2023 at 11:34, David Howells <dhowells@redhat.com> wrote:
>
> What about something like the Intel on-die accelerators (e.g. IAA and QAT)? I
> think they can do async compression.
I'm sure they can. And for some made-up benchmark it might even help.
Do people use it in real life?
The *big* wins come from being able to do compression/encryption
inline, when you don't need to do double-buffering etc.
Anything else is completely broken, imnsho. Once you need to
double-buffer your IO, you've already lost the whole point.
Linus
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [v2 PATCH 0/5] crypto: Add akcipher interface without SGs
2023-06-28 20:10 ` Linus Torvalds
@ 2023-06-29 4:49 ` Gao Xiang
0 siblings, 0 replies; 28+ messages in thread
From: Gao Xiang @ 2023-06-29 4:49 UTC (permalink / raw)
To: Linus Torvalds, David Howells
Cc: Ard Biesheuvel, Eric Biggers, Herbert Xu, Roberto Sassu,
Stefan Berger, Mimi Zohar, dmitry.kasatkin, Jarkko Sakkinen,
keyrings, Linux Crypto Mailing List
Hi Linus,
On 2023/6/29 04:10, Linus Torvalds wrote:
> On Wed, 28 Jun 2023 at 11:34, David Howells <dhowells@redhat.com> wrote:
>>
>> What about something like the Intel on-die accelerators (e.g. IAA and QAT)? I
>> think they can do async compression.
>
> I'm sure they can. And for some made-up benchmark it might even help.
> Do people use it in real life?
>
> The *big* wins come from being able to do compression/encryption
> inline, when you don't need to do double-buffering etc.
>
> Anything else is completely broken, imnsho. Once you need to
> double-buffer your IO, you've already lost the whole point.
I'm not sure if I could say much about this for now, yet we're
slowly evaluating Intel IAA builtin DEFLATE engine for our
Cloud workloads and currently we don't have end-to-end numbers
yet.
Storage inline accelerators are great, especially
"do {en,de}cryption inline" since it consumes very little
on-chip memory, yet afaik "(de)compression" inline engine story
is different since it needs more SRAM space for their LZ sliding
windows for matching (e.g. 32kb for deflate each channel, 64kb
for LZ4 each channel, and much much more for Zstd, LZMA, etc.
I think those are quite expensive to integrate) in addition to
some additional memory for huffman/FSE tables.
So in production, inline "(de)compression" accelerators are
hardly seen as a part of storage at least in end consumer
markets.
Intel already has their on-die accelerators (IAA very recently),
yeah, it still needs double-buffer I/O, but we're considering
at least using in async writeback/readahead use cases as a
start for bulk async I/Os, which are not quite latency
sensitive. Intel also shows their Zswap work [1] , yet I don't
dive into that since I'm only focusing on storage use cases.
As for crypto current apis (no matter acomp or scomp), I'm not
sure I will say more too since I mostly agree with what Eric
said previously.
Thanks,
Gao Xiang
[1] https://lore.kernel.org/r/20230605201536.738396-1-tom.zanussi@linux.intel.com/
>
> Linus
>
^ permalink raw reply [flat|nested] 28+ messages in thread