LinuxLists.cc - [PATCH 0/3] AF_ALG interface for akcipher

[permalink] [raw]

Subject: [PATCH 3/3] crypto: algif_akcipher - enable compilation

Add the Makefile and Kconfig updates to allow algif_akcipher to be
compiled.

CC: Tadeusz Struk <[email protected]>
Signed-off-by: Stephan Mueller <[email protected]>
---
crypto/Kconfig | 9 +++++++++
crypto/Makefile | 1 +
2 files changed, 10 insertions(+)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 354bb69..d12ecc5 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1623,6 +1623,15 @@ config CRYPTO_USER_API_AEAD
This option enables the user-spaces interface for AEAD
cipher algorithms.

+config CRYPTO_USER_API_AKCIPHER
+ tristate "User-space interface for asymmetric key cipher algorithms"
+ depends on NET
+ select CRYPTO_AKCIPHER2
+ select CRYPTO_USER_API
+ help
+ This option enables the user-spaces interface for asymmetric
+ key cipher algorithms.
+
config CRYPTO_HASH_INFO
bool

diff --git a/crypto/Makefile b/crypto/Makefile
index a16a7e7..64823ab 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -116,6 +116,7 @@ obj-$(CONFIG_CRYPTO_USER_API_HASH) += algif_hash.o
obj-$(CONFIG_CRYPTO_USER_API_SKCIPHER) += algif_skcipher.o
obj-$(CONFIG_CRYPTO_USER_API_RNG) += algif_rng.o
obj-$(CONFIG_CRYPTO_USER_API_AEAD) += algif_aead.o
+obj-$(CONFIG_CRYPTO_USER_API_AKCIPHER) += algif_akcipher.o

#
# generic algorithms and the async_tx api
--
2.4.3

2015-07-21 22:14:35

[permalink] [raw]

Subject: [PATCH 2/3] crypto: algif_akcipher user space interface

This patch adds the AF_ALG interface to user space.

The akcipher kernel crypto API uses linear buffers (instead of the
scatter lists for the other AF_ALG interfaces). To handle such linear
buffer, the interface handler uses an internal buffer to operate on
data. The buffer has the size required by the asymmetric cipher
implementation and usually is identical to the modulus.

The algif_akcipher interface provides a sendmsg as well as a sendpage
interface.

As requested by RFC3447, the maximum buffer size for input data to
either encryption, decryption, sig gen, sig ver is equal to the size
of the modulus. The buffer size has an absolute limit of PAGE_SIZE
to catch errors in an underlying asym cipher implementation.

CC: Tadeusz Struk <[email protected]>
Signed-off-by: Stephan Mueller <[email protected]>
---
crypto/algif_akcipher.c | 480 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 480 insertions(+)
create mode 100644 crypto/algif_akcipher.c

diff --git a/crypto/algif_akcipher.c b/crypto/algif_akcipher.c
new file mode 100644
index 0000000..2275fb1
--- /dev/null
+++ b/crypto/algif_akcipher.c
@@ -0,0 +1,480 @@
+/*
+ * algif_aead: User-space interface for asymmetric key algorithms
+ *
+ * Copyright (C) 2015, Stephan Mueller <[email protected]>
+ *
+ * This file provides the user-space API for akciphers.
+ *
+ * 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.
+ */
+
+#include <crypto/akcipher.h>
+#include <crypto/if_alg.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/net.h>
+#include <net/sock.h>
+
+struct akcipher_ctx {
+ u8 *req_data_ptr; /* pointer to buffer to be processed */
+ struct page *page_ptr;
+ u8 *req_data; /* buffer used for sendmsg requests */
+ size_t req_size; /* minimum buffer size */
+
+ struct af_alg_completion completion;
+
+ size_t used;
+
+ unsigned int len;
+ bool more;
+ int op;
+
+ struct akcipher_request req;
+};
+
+static inline bool akcipher_sufficient_data(struct akcipher_ctx *ctx)
+{
+ return ctx->used >= ctx->req_size;
+}
+
+static void akcipher_put_sgl(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+
+ ctx->used = 0;
+ ctx->more = 0;
+
+ ctx->req_data_ptr = NULL;
+
+ if (ctx->page_ptr)
+ put_page(ctx->page_ptr);
+ ctx->page_ptr = NULL;
+}
+
+static void akcipher_wmem_wakeup(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+ struct socket_wq *wq;
+
+ if (akcipher_sufficient_data(ctx))
+ return;
+
+ rcu_read_lock();
+ wq = rcu_dereference(sk->sk_wq);
+ if (wq_has_sleeper(wq))
+ wake_up_interruptible_sync_poll(&wq->wait, POLLIN |
+ POLLRDNORM |
+ POLLRDBAND);
+ sk_wake_async(sk, SOCK_WAKE_WAITD, POLL_IN);
+ rcu_read_unlock();
+}
+
+static int akcipher_wait_for_data(struct sock *sk, unsigned flags)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+ long timeout;
+ DEFINE_WAIT(wait);
+ int err = -ERESTARTSYS;
+
+ if (flags & MSG_DONTWAIT)
+ return -EAGAIN;
+
+ set_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
+
+ for (;;) {
+ if (signal_pending(current))
+ break;
+ prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
+ timeout = MAX_SCHEDULE_TIMEOUT;
+ if (sk_wait_event(sk, &timeout, !ctx->more)) {
+ err = 0;
+ break;
+ }
+ }
+ finish_wait(sk_sleep(sk), &wait);
+
+ clear_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
+
+ return err;
+}
+
+static void akcipher_data_wakeup(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+ struct socket_wq *wq;
+
+ if (ctx->more)
+ return;
+ if (!ctx->used)
+ return;
+
+ rcu_read_lock();
+ wq = rcu_dereference(sk->sk_wq);
+ if (wq_has_sleeper(wq))
+ wake_up_interruptible_sync_poll(&wq->wait, POLLOUT |
+ POLLRDNORM |
+ POLLRDBAND);
+ sk_wake_async(sk, SOCK_WAKE_SPACE, POLL_OUT);
+ rcu_read_unlock();
+}
+
+static int akcipher_get_reqsize(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+
+ akcipher_request_set_crypt(&ctx->req, NULL, NULL, 0, 0);
+ /* expect this to fail, and update the required buf len */
+ crypto_akcipher_encrypt(&ctx->req);
+
+ if (!ctx->req.dst_len)
+
+ BUG_ON(ctx->req.dst_len > PAGE_SIZE);
+
+ /* Existing memory size matches required size. */
+ if (ctx->req_size == ctx->req.dst_len)
+ return 0;
+
+ if (ctx->req_data) {
+ sock_kzfree_s(sk, ctx->req_data, ctx->req_size);
+ ctx->req_data = NULL;
+ }
+
+ ctx->req_size = ctx->req.dst_len;
+ ctx->req_data = sock_kmalloc(sk, ctx->req_size, GFP_KERNEL);
+ if (!ctx->req_data) {
+ ctx->req_size = 0;
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+static int akcipher_sendmsg(struct socket *sock, struct msghdr *msg,
+ size_t size)
+{
+ struct sock *sk = sock->sk;
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+ struct af_alg_control con = {};
+ long copied = 0;
+ int op = 0;
+ bool init = 0;
+ int err = -EINVAL;
+
+ if (msg->msg_controllen) {
+ err = af_alg_cmsg_send(msg, &con);
+ if (err)
+ return err;
+
+ init = 1;
+ switch (con.op) {
+ case ALG_OP_VERIFY:
+ break;
+ case ALG_OP_SIGN:
+ break;
+ case ALG_OP_ENCRYPT:
+ break;
+ case ALG_OP_DECRYPT:
+ break;
+ default:
+ return -EINVAL;
+ }
+ op = con.op;
+ }
+
+ lock_sock(sk);
+ if (!ctx->more && ctx->used)
+ goto unlock;
+
+ if (init) {
+ ctx->op = op;
+
+ err = akcipher_get_reqsize(sk);
+ if (err)
+ goto unlock;
+ }
+
+ if (!size)
+ goto done;
+
+
+ /*
+ * We do not allow mixing of sendmsg and sendpage calls as this would
+ * require a hairy memory management.
+ */
+ if (ctx->page_ptr)
+ goto unlock;
+
+ ctx->req_data_ptr = ctx->req_data;
+
+ if (ctx->req_size > ctx->used) {
+ size_t len = min_t(size_t, size, ctx->req_size - ctx->used);
+
+ err = memcpy_from_msg(ctx->req_data + ctx->used, msg, len);
+ if (err)
+ goto unlock;
+
+ ctx->used += len;
+ copied += len;
+ size -= len;
+ }
+
+ err = 0;
+
+done:
+ ctx->more = msg->msg_flags & MSG_MORE;
+
+unlock:
+ akcipher_data_wakeup(sk);
+ release_sock(sk);
+
+ return err ?: copied;
+}
+
+static ssize_t akcipher_sendpage(struct socket *sock, struct page *page,
+ int offset, size_t size, int flags)
+{
+ struct sock *sk = sock->sk;
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+ int err = -EINVAL;
+
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
+ lock_sock(sk);
+
+ /*
+ * We do not allow mixing of sendmsg and sendpage calls as this would
+ * require a hairy memory management.
+ *
+ * This check also guards against double call of sendpage.
+ * We require that the output buffer size must be provided with one
+ * sendpage request as otherwise we cannot have a linear buffer required
+ * by the akcipher API.
+ */
+ if (ctx->req_data_ptr)
+ goto unlock;
+
+ if (akcipher_sufficient_data(ctx)) {
+ err = -EAGAIN;
+ goto done;
+ }
+
+ if (!size)
+ goto done;
+
+ get_page(page);
+ ctx->req_data_ptr = page_address(page) + offset;
+ ctx->page_ptr = page;
+ ctx->used = size;
+ if (ctx->used > ctx->req_size)
+ ctx->used = ctx->req_size;
+
+ err = 0;
+
+done:
+ ctx->more = flags & MSG_MORE;
+
+unlock:
+ akcipher_data_wakeup(sk);
+ release_sock(sk);
+
+ return err ?: size;
+}
+
+static int akcipher_recvmsg(struct socket *sock, struct msghdr *msg,
+ size_t ignored, int flags)
+{
+ struct sock *sk = sock->sk;
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+ int err = -EINVAL;
+ size_t outlen = 0;
+
+ lock_sock(sk);
+
+ if (ctx->more && !akcipher_sufficient_data(ctx)) {
+ err = akcipher_wait_for_data(sk, flags);
+ if (err)
+ goto unlock;
+ }
+
+ BUG_ON(!ctx->req_data_ptr);
+ akcipher_request_set_crypt(&ctx->req, ctx->req_data_ptr,
+ ctx->req_data, ctx->used, ctx->req_size);
+ switch(ctx->op) {
+ case ALG_OP_VERIFY:
+ err = crypto_akcipher_verify(&ctx->req);
+ break;
+ case ALG_OP_SIGN:
+ err = crypto_akcipher_sign(&ctx->req);
+ break;
+ case ALG_OP_ENCRYPT:
+ err = crypto_akcipher_encrypt(&ctx->req);
+ break;
+ case ALG_OP_DECRYPT:
+ err = crypto_akcipher_decrypt(&ctx->req);
+ break;
+ default:
+ BUG();
+ }
+
+ err = af_alg_wait_for_completion(err, &ctx->completion);
+
+ /* EBADMSG implies a valid cipher operation took place */
+ if (err) {
+ if (err == -EBADMSG)
+ akcipher_put_sgl(sk);
+ goto unlock;
+ }
+
+ /* Only copy the size of the calculated value to user space. */
+ outlen = ctx->req.dst_len;
+ err = memcpy_to_msg(msg, ctx->req_data, outlen);
+ if (err)
+ goto unlock;
+
+ akcipher_put_sgl(sk);
+
+unlock:
+ akcipher_wmem_wakeup(sk);
+ release_sock(sk);
+
+ return err ? err : outlen;
+}
+
+static unsigned int akcipher_poll(struct file *file, struct socket *sock,
+ poll_table *wait)
+{
+ struct sock *sk = sock->sk;
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+ unsigned int mask;
+
+ sock_poll_wait(file, sk_sleep(sk), wait);
+ mask = 0;
+
+ if (!ctx->more)
+ mask |= POLLIN | POLLRDNORM;
+
+ if (!akcipher_sufficient_data(ctx))
+ mask |= POLLOUT | POLLWRNORM | POLLWRBAND;
+
+ return mask;
+}
+
+static struct proto_ops algif_akcipher_ops = {
+ .family = PF_ALG,
+
+ .connect = sock_no_connect,
+ .socketpair = sock_no_socketpair,
+ .getname = sock_no_getname,
+ .ioctl = sock_no_ioctl,
+ .listen = sock_no_listen,
+ .shutdown = sock_no_shutdown,
+ .getsockopt = sock_no_getsockopt,
+ .mmap = sock_no_mmap,
+ .bind = sock_no_bind,
+ .accept = sock_no_accept,
+ .setsockopt = sock_no_setsockopt,
+
+ .release = af_alg_release,
+ .sendmsg = akcipher_sendmsg,
+ .sendpage = akcipher_sendpage,
+ .recvmsg = akcipher_recvmsg,
+ .poll = akcipher_poll,
+};
+
+static void *akcipher_bind(const char *name, u32 type, u32 mask)
+{
+ return crypto_alloc_akcipher(name, type, mask);
+}
+
+static void akcipher_release(void *private)
+{
+ crypto_free_akcipher(private);
+}
+
+static int akcipher_setkey(void *private, const u8 *key, unsigned int keylen)
+{
+ return crypto_akcipher_setkey(private, (u8 *)key, keylen);
+}
+
+static void akcipher_sock_destruct(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+
+ akcipher_put_sgl(sk);
+ if (ctx->req_data)
+ sock_kzfree_s(sk, ctx->req_data, ctx->req_size);
+ sock_kfree_s(sk, ctx, ctx->len);
+ af_alg_release_parent(sk);
+}
+
+static int akcipher_accept_parent(void *private, struct sock *sk)
+{
+ struct akcipher_ctx *ctx;
+ struct alg_sock *ask = alg_sk(sk);
+ unsigned int len = sizeof(*ctx) + crypto_akcipher_reqsize(private);
+
+ ctx = sock_kmalloc(sk, len, GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+ memset(ctx, 0, len);
+
+ ctx->len = len;
+ ctx->used = 0;
+ ctx->more = 0;
+ ctx->op = 0;
+ ctx->req_size = 0;
+ af_alg_init_completion(&ctx->completion);
+
+ ask->private = ctx;
+
+ akcipher_request_set_tfm(&ctx->req, private);
+ akcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
+ af_alg_complete, &ctx->completion);
+
+ sk->sk_destruct = akcipher_sock_destruct;
+
+ return 0;
+}
+
+static const struct af_alg_type algif_type_aead = {
+ .bind = akcipher_bind,
+ .release = akcipher_release,
+ .setkey = akcipher_setkey,
+ .accept = akcipher_accept_parent,
+ .ops = &algif_akcipher_ops,
+ .name = "akcipher",
+ .owner = THIS_MODULE
+};
+
+static int __init algif_akcipher_init(void)
+{
+ return af_alg_register_type(&algif_type_aead);
+}
+
+static void __exit algif_akcipher_exit(void)
+{
+ int err = af_alg_unregister_type(&algif_type_aead);
+ BUG_ON(err);
+}
+
+module_init(algif_akcipher_init);
+module_exit(algif_akcipher_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Stephan Mueller <[email protected]>");
+MODULE_DESCRIPTION("Asymmetric key kernel crypto API user space interface");
--
2.4.3

2015-07-21 22:14:35

[permalink] [raw]

Subject: [PATCH 1/3] crypto: af_alg - add sig gen / verify API

As a prerequisite to add the asymmetric cipher AF_ALG API, this patch
adds the user space interface identifier to tell the kernel about
performing a signature generation and verification operation in addition
to the existing encryption / decryption operation.

CC: Tadeusz Struk <[email protected]>
Signed-off-by: Stephan Mueller <[email protected]>
---
include/uapi/linux/if_alg.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
index f2acd2f..d81dcca 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -38,5 +38,7 @@ struct af_alg_iv {
/* Operations */
#define ALG_OP_DECRYPT 0
#define ALG_OP_ENCRYPT 1
+#define ALG_OP_SIGN 2
+#define ALG_OP_VERIFY 3

#endif /* _LINUX_IF_ALG_H */
--
2.4.3

2015-07-22 01:32:52

[permalink] [raw]

Subject: Re: [PATCH 0/3] AF_ALG interface for akcipher

On Wed, Jul 22, 2015 at 12:12:29AM +0200, Stephan Mueller wrote:
>
> The patchset provides the implementation of the AF_ALG userspace interface
> for the asymmetric cipher API of the kernel crypto API.

I think we should finish the conversion of the only in-kernel
user of RSA before we add the user-space interface. Otherwise
this unnecessarily ties our hands to the current API.

For example, do we want an SG interface for the input and output?
As it stands even if a hardware device came with SG support we
would not be able to use it.

Tadeusz, can you confirm again that qat does not support SG with
respect to RSA input and output?

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2015-07-22 04:20:03

[permalink] [raw]

Subject: Re: [PATCH 0/3] AF_ALG interface for akcipher

On 07/21/2015 06:32 PM, Herbert Xu wrote:
> I think we should finish the conversion of the only in-kernel
> user of RSA before we add the user-space interface. Otherwise
> this unnecessarily ties our hands to the current API.
>
> For example, do we want an SG interface for the input and output?
> As it stands even if a hardware device came with SG support we
> would not be able to use it.
>
> Tadeusz, can you confirm again that qat does not support SG with
> respect to RSA input and output?

No, it doesn't. Even if we would add SG support on the API it will
need to be copied into contiguous buffers before it is sent to the HW.

Is there any other in-kernel user of RSA other than the module verifier?
If that's the only one I don't think we need any API changes.
I can refresh the old patches that converted it.

2015-07-22 04:21:42

[permalink] [raw]

Subject: Re: [PATCH 0/3] AF_ALG interface for akcipher

On Tue, Jul 21, 2015 at 09:19:28PM -0700, Tadeusz Struk wrote:
>
> Is there any other in-kernel user of RSA other than the module verifier?

Not that I know of.

> If that's the only one I don't think we need any API changes.
> I can refresh the old patches that converted it.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2015-07-22 07:15:00

[permalink] [raw]

Subject: Re: [PATCH 0/3] AF_ALG interface for akcipher

Am Mittwoch, 22. Juli 2015, 09:32:41 schrieb Herbert Xu:

Hi Herbert,

>
> I think we should finish the conversion of the only in-kernel
> user of RSA before we add the user-space interface. Otherwise
> this unnecessarily ties our hands to the current API.

Agreed.

After my question around the SGL vs linear buffers I was under the impression
that it was decided.

So I will wait until the discussion is completed.

>
> For example, do we want an SG interface for the input and output?

The question is whether the SGL overhead is still acceptable for the small
buffer size we have to consider when talking about raw asymmetric ciphers. For
RSA, it seems that the max size 4096 bits for the generic implementation. For
other implementations, I would not expect much larger sizes. Thus, we talk
about 512 bytes or one 1kbytes maximum operation size.

When ECC comes into picture, the sizes are even significanly smaller.

If I understood correctly, we do not want to support full hybrid asymmetric
operations (like PKCS1, OAEP and the like) where large buffers are to be
handled.

On the other hand, what about using struct scatterlist as the API input
parameter. In addition, we allow cipher implementations to traverse only one
SG entry which implies a linear buffer operation. But the API is open for full
SG operation if needed in the future. To inform the caller about the minimum
size of an SG entry, we may use another integer value similar to alignmask.
So, if Tadeusz's hardware wants one linear buffer, the integer is 256 or 512
where the caller must create the buffers of at least this size. As all input
data would fit into that one buffer, there the caller only needs to allocate
one SG.

--
Ciao
Stephan

2015-07-22 12:32:18

by Horia Geantă

[permalink] [raw]

Subject: Re: [PATCH 0/3] AF_ALG interface for akcipher

On 7/22/2015 7:19 AM, Tadeusz Struk wrote:
> On 07/21/2015 06:32 PM, Herbert Xu wrote:
>> I think we should finish the conversion of the only in-kernel
>> user of RSA before we add the user-space interface. Otherwise
>> this unnecessarily ties our hands to the current API.
>>
>> For example, do we want an SG interface for the input and output?
>> As it stands even if a hardware device came with SG support we
>> would not be able to use it.
>>
>> Tadeusz, can you confirm again that qat does not support SG with
>> respect to RSA input and output?
>
> No, it doesn't. Even if we would add SG support on the API it will
> need to be copied into contiguous buffers before it is sent to the HW.

OTOH, caam has SG support for all PK operations, including rsa-encrypt,
rsa-decrypt primitives.
We are working at upstreaming - aligning our internal caam-pkc with
akcipher.

Please consider designing a flexible AF_ALG interface, as Stephan suggested.

Thanks,
Horia

2015-07-22 16:01:51

[permalink] [raw]

Subject: Re: [PATCH 2/3] crypto: algif_akcipher user space interface

On 07/21/2015 03:13 PM, Stephan Mueller wrote:
> +static ssize_t akcipher_sendpage(struct socket *sock, struct page *page,
> + int offset, size_t size, int flags)
> +{
> + struct sock *sk = sock->sk;
> + struct alg_sock *ask = alg_sk(sk);
> + struct akcipher_ctx *ctx = ask->private;
> + int err = -EINVAL;
> +
> + if (flags & MSG_SENDPAGE_NOTLAST)
> + flags |= MSG_MORE;
> +
> + lock_sock(sk);
> +
> + /*
> + * We do not allow mixing of sendmsg and sendpage calls as this would
> + * require a hairy memory management.
> + *
> + * This check also guards against double call of sendpage.
> + * We require that the output buffer size must be provided with one
> + * sendpage request as otherwise we cannot have a linear buffer required
> + * by the akcipher API.
> + */
> + if (ctx->req_data_ptr)
> + goto unlock;

Shouldn't we be more flexible and copy the data if it comes in chunks here too.
The user doesn't really have control over this and it would look bad if splice
would randomly fail for a valid buffer.

2015-07-22 18:55:28

[permalink] [raw]

Subject: Re: [PATCH 2/3] crypto: algif_akcipher user space interface

Am Mittwoch, 22. Juli 2015, 09:01:15 schrieb Tadeusz Struk:

Hi Tadeusz,

> On 07/21/2015 03:13 PM, Stephan Mueller wrote:
> > +static ssize_t akcipher_sendpage(struct socket *sock, struct page *page,
> > + int offset, size_t size, int flags)
> > +{
> > + struct sock *sk = sock->sk;
> > + struct alg_sock *ask = alg_sk(sk);
> > + struct akcipher_ctx *ctx = ask->private;
> > + int err = -EINVAL;
> > +
> > + if (flags & MSG_SENDPAGE_NOTLAST)
> > + flags |= MSG_MORE;
> > +
> > + lock_sock(sk);
> > +
> > + /*
> > + * We do not allow mixing of sendmsg and sendpage calls as this would
> > + * require a hairy memory management.
> > + *
> > + * This check also guards against double call of sendpage.
> > + * We require that the output buffer size must be provided with one
> > + * sendpage request as otherwise we cannot have a linear buffer
required
> > + * by the akcipher API.
> > + */
> > + if (ctx->req_data_ptr)
> > + goto unlock;
>
> Shouldn't we be more flexible and copy the data if it comes in chunks here
> too. The user doesn't really have control over this and it would look bad
> if splice would randomly fail for a valid buffer.

I concur with you. But we have only two options:

- either use SGLs which the current akcipher API does not do

- or do a memcpy of the sendpage data into the internal buffer

As the sendpage already has a speed penalty, I did not like the latter one.
Based on my measurements for AEAD, Hash and skicpher, sendpage starts to
become faster than sendmsg with input buffers > 8 to 16 kBytes (sendpage as at
least 4 syscalls where sendmsg uses only two).

As our current akcipher API does not reach the mentioned limit, I opted to
require one sendpage call.

But if we change the akcipher API to SGLs, I will lift that limit.

Thanks

--
Ciao
Stephan

2015-07-22 22:05:13

[permalink] [raw]

Subject: Re: [PATCH 2/3] crypto: algif_akcipher user space interface

On 07/22/2015 11:55 AM, Stephan Mueller wrote:
> I concur with you. But we have only two options:
>
> - either use SGLs which the current akcipher API does not do
>
> - or do a memcpy of the sendpage data into the internal buffer

I would memcpy it here. Both the current software and QAT implementations
do not support SGL so we'll end up memcpy it anyway.

>
> As the sendpage already has a speed penalty, I did not like the latter one.
> Based on my measurements for AEAD, Hash and skicpher, sendpage starts to
> become faster than sendmsg with input buffers > 8 to 16 kBytes (sendpage as at
> least 4 syscalls where sendmsg uses only two).
>
> As our current akcipher API does not reach the mentioned limit, I opted to
> require one sendpage call.
>
> But if we change the akcipher API to SGLs, I will lift that limit.

As you said - the preferred way of invoking this would be sendmsg with data instead
of splice. This is only to make splice happy in case the data is not in a contiguous
buffer.
Thanks,
T

2015-07-22 22:30:00

[permalink] [raw]

Subject: Re: [PATCH 2/3] crypto: algif_akcipher user space interface

Am Mittwoch, 22. Juli 2015, 15:04:37 schrieb Tadeusz Struk:

Hi Tadeusz,

> On 07/22/2015 11:55 AM, Stephan Mueller wrote:
> > I concur with you. But we have only two options:
> >
> > - either use SGLs which the current akcipher API does not do
> >
> > - or do a memcpy of the sendpage data into the internal buffer
>
> I would memcpy it here. Both the current software and QAT implementations
> do not support SGL so we'll end up memcpy it anyway.
>
> > As the sendpage already has a speed penalty, I did not like the latter
> > one.
> > Based on my measurements for AEAD, Hash and skicpher, sendpage starts to
> > become faster than sendmsg with input buffers > 8 to 16 kBytes (sendpage
> > as at least 4 syscalls where sendmsg uses only two).
> >
> > As our current akcipher API does not reach the mentioned limit, I opted to
> > require one sendpage call.
> >
> > But if we change the akcipher API to SGLs, I will lift that limit.
>
> As you said - the preferred way of invoking this would be sendmsg with data
> instead of splice. This is only to make splice happy in case the data is
> not in a contiguous buffer.

Good point.

I am waiting for Herbert for the decision on changing the akcipher to SGL.
Based on that decision I either implement your suggestion (if we leave linear
buffers) or I will go with preparing the SGL setup.

--
Ciao
Stephan

2015-07-23 01:53:15

[permalink] [raw]

Subject: Re: [PATCH 0/3] AF_ALG interface for akcipher

On Wed, Jul 22, 2015 at 02:58:18PM +0300, Horia Geantă wrote:
>
> OTOH, caam has SG support for all PK operations, including rsa-encrypt,
> rsa-decrypt primitives.
> We are working at upstreaming - aligning our internal caam-pkc with
> akcipher.

OK. Then we should tweak our interface to allow SGs. Tadeusz?

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2015-07-23 05:15:06

[permalink] [raw]

Subject: Re: [PATCH 0/3] AF_ALG interface for akcipher

On 07/22/2015 06:53 PM, Herbert Xu wrote:
> On Wed, Jul 22, 2015 at 02:58:18PM +0300, Horia Geantă wrote:
>>
>> OTOH, caam has SG support for all PK operations, including rsa-encrypt,
>> rsa-decrypt primitives.
>> We are working at upstreaming - aligning our internal caam-pkc with
>> akcipher.
>
> OK. Then we should tweak our interface to allow SGs. Tadeusz?
>

We can add a flag to akcipher_request to say if src/dst are SGs or buffers, but
is this really necessary?
What we have now is two implementations that don't support SGs.
In terms of users we have one in kernel i.e module verifier which doesn't need SGs
and proposed user space interface, which is going to copy data from/to user because
the buffers are so small that it is cheaper to copy than to use the splice/vmsplice
calls. To be honest I don't see any benefit in adding SG support.

2015-07-23 05:19:17

[permalink] [raw]

Subject: Re: [PATCH 0/3] AF_ALG interface for akcipher

On Wed, Jul 22, 2015 at 10:14:30PM -0700, Tadeusz Struk wrote:
>
> We can add a flag to akcipher_request to say if src/dst are SGs or buffers, but
> is this really necessary?

No it shouldn't be done as a flag. If we are going to do SGs then
it needs to be part of the API. That means replacing src/dst
pointers with SG lists.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2015-07-23 10:16:11