diff mbox

[RFC,v1,1/1] crypto: algif_compression - User-space interface for compression

Message ID 1492108494-10694-2-git-send-email-akamaluddin@cavium.com (mailing list archive)
State RFC
Delegated to: Herbert Xu
Headers show

Commit Message

Abed Kamaluddin April 13, 2017, 6:34 p.m. UTC
crypto: algif_compression - User-space interface for compression

This patch adds af_alg plugin for compression algorithms of type scomp/acomp
registered to the kernel crypto layer.

The user needs to set operation (compression/decompression) as a control
message to sendmsg, identical to selecting the cipher operation type in case of
ciphers. Once a sendmsg call occurs, no further writes can be made to the
socket until all previous data has been processed and read. Therefore the
interface only supports one request at a time.

The interface is completely synchronous; all operations are carried out in
recvmsg and will complete prior to the system call returning.

The sendmsg and recvmsg interface supports directly reading/writing to 
user-space without additional copying, i.e., the kernel crypto interface will
receive the user-space address as its input/output SG list. The scomp interface
or crypto drivers may copy the data as required.

Signed-off-by: Abed Kamaluddin <akam...@cavium.com>
Signed-off-by: Mahipal Challa <mahipal.cha...@cavium.com>

---
 crypto/Kconfig              |  11 ++
 crypto/Makefile             |   1 +
 crypto/algif_compression.c  | 272 ++++++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/if_alg.h |   2 +
 4 files changed, 286 insertions(+)

Comments

PrasannaKumar Muralidharan April 14, 2017, 4:19 a.m. UTC | #1
On 14 April 2017 at 00:04, Abed Kamaluddin <abedamu@gmail.com> wrote:
> crypto: algif_compression - User-space interface for compression
>
> This patch adds af_alg plugin for compression algorithms of type scomp/acomp
> registered to the kernel crypto layer.
>
> The user needs to set operation (compression/decompression) as a control
> message to sendmsg, identical to selecting the cipher operation type in case of
> ciphers. Once a sendmsg call occurs, no further writes can be made to the
> socket until all previous data has been processed and read. Therefore the
> interface only supports one request at a time.
>
> The interface is completely synchronous; all operations are carried out in
> recvmsg and will complete prior to the system call returning.
>
> The sendmsg and recvmsg interface supports directly reading/writing to
> user-space without additional copying, i.e., the kernel crypto interface will
> receive the user-space address as its input/output SG list. The scomp interface
> or crypto drivers may copy the data as required.
>
> Signed-off-by: Abed Kamaluddin <akam...@cavium.com>
> Signed-off-by: Mahipal Challa <mahipal.cha...@cavium.com>
>
> ---
>  crypto/Kconfig              |  11 ++
>  crypto/Makefile             |   1 +
>  crypto/algif_compression.c  | 272 ++++++++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/if_alg.h |   2 +
>  4 files changed, 286 insertions(+)
>
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index f37e9cc..13b03ba 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -1741,6 +1741,17 @@ config CRYPTO_USER_API_AEAD
>           This option enables the user-spaces interface for AEAD
>           cipher algorithms.
>
> +config CRYPTO_USER_API_COMPRESSION
> +       tristate "User-space interface for compression algorithms"
> +       depends on NET
> +       select CRYPTO_ACOMP
> +       select CRYPTO_USER_API
> +       help
> +         This option enables the user-space interface for compression
> +         algorithms. Enable this option for access to compression algorithms
> +         of type scomp/acomp exported by the kernel crypto layer through
> +         AF_ALG interface.
> +
>  config CRYPTO_HASH_INFO
>         bool
>
> diff --git a/crypto/Makefile b/crypto/Makefile
> index 8a44057..1469e06 100644
> --- a/crypto/Makefile
> +++ b/crypto/Makefile
> @@ -134,6 +134,7 @@ obj-$(CONFIG_CRYPTO_TEST) += tcrypt.o
>  obj-$(CONFIG_CRYPTO_GHASH) += ghash-generic.o
>  obj-$(CONFIG_CRYPTO_USER_API) += af_alg.o
>  obj-$(CONFIG_CRYPTO_USER_API_HASH) += algif_hash.o
> +obj-$(CONFIG_CRYPTO_USER_API_COMPRESSION) += algif_compression.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
> diff --git a/crypto/algif_compression.c b/crypto/algif_compression.c
> new file mode 100644
> index 0000000..0ba6d1e
> --- /dev/null
> +++ b/crypto/algif_compression.c
> @@ -0,0 +1,272 @@
> +/*
> + * algif_compression: User-space interface for COMPRESSION algorithms
> + *
> + * This file provides user-space API support for compression algorithms
> + * registered through the kernel crypto layer.
> + *
> + * Copyright (C) 2017 Cavium, Inc.
> + *
> + * Original Authors: Abed Kamaluddin <akamaluddin@cavium.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + */
> +
> +#include <crypto/acompress.h>
> +#include <crypto/if_alg.h>
> +#include <linux/crypto.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>
> +#include <linux/scatterlist.h>
> +
> +/* scomp scratch is currently 128KB */
> +#define COMP_BUFFER_SIZE       65535
> +
> +struct comp_ctx {
> +       struct af_alg_sgl tsgl;
> +       struct af_alg_sgl rsgl;
> +       struct af_alg_completion completion;
> +       unsigned int clen;
> +       unsigned int slen;
> +       unsigned int dlen;
> +       bool comp;
> +       bool used;
> +       struct acomp_req *acomp_req;
> +};

Is it necessary to have 3 len fields viz clen, slen, dlen? Please add
a comment indicating their purpose.

> +struct comp_tfm {
> +       struct crypto_acomp *acomp;
> +};
> +
> +static int comp_sendmsg(struct socket *sock, struct msghdr *msg,
> +                       size_t ignored)
> +{
> +       struct sock *sk = sock->sk;
> +       struct alg_sock *ask = alg_sk(sk);
> +       struct comp_ctx *ctx = ask->private;
> +       struct af_alg_control con = {};
> +       int limit = COMP_BUFFER_SIZE;
> +       int len;
> +       int err = -EINVAL;
> +
> +       if (msg->msg_controllen) {
> +               err = af_alg_cmsg_send(msg, &con);
> +               if (err)
> +                       return err;
> +
> +               switch (con.op) {
> +               case ALG_OP_COMPRESS:
> +                       ctx->comp = 1;
> +                       break;
> +
> +               case ALG_OP_DECOMPRESS:
> +                       ctx->comp = 0;
> +                       break;
> +
> +               default:
> +                       return -EINVAL;
> +               }
> +       }
> +
> +       lock_sock(sk);
> +
> +       /* One request at a time supported, data submitted for comp/decomp will
> +        * be processed at subsequent recvmsg
> +        */
> +       if (ctx->used) {
> +               err = -EAGAIN;
> +               goto unlock;
> +       }
> +
> +       len = msg_data_left(msg);
> +
> +       if (len > limit)
> +               len = limit;
> +
> +       len = af_alg_make_sg(&ctx->tsgl, &msg->msg_iter, len);
> +
> +       if (len < 0) {
> +               err = len;
> +               goto unlock;
> +       }
> +
> +       ctx->slen = len;
> +       ctx->used = 1;
> +
> +unlock:
> +       release_sock(sk);
> +
> +       return err ?: len;
> +}
> +
> +static int comp_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
> +                       int flags)
> +{
> +       struct sock *sk = sock->sk;
> +       struct alg_sock *ask = alg_sk(sk);
> +       struct comp_ctx *ctx = ask->private;
> +       int rlen = ctx->dlen;
> +       int err;
> +
> +       if (len > rlen)
> +               len = rlen;
> +
> +       lock_sock(sk);
> +
> +       if (!ctx->used) {
> +               err = -EAGAIN;
> +               goto unlock;
> +       }
> +
> +       len = af_alg_make_sg(&ctx->rsgl, &msg->msg_iter, len);
> +       if (len < 0) {
> +               err = len;
> +               goto unlock;
> +       }
> +
> +       acomp_request_set_params(ctx->acomp_req, ctx->tsgl.sg, ctx->rsgl.sg,
> +                                ctx->slen, len);
> +
> +       /* Synchronous completion of comp/decomp requests */
> +       err = af_alg_wait_for_completion(
> +                       ctx->comp ?
> +                               crypto_acomp_compress(ctx->acomp_req) :
> +                               crypto_acomp_decompress(ctx->acomp_req),
> +                               &ctx->completion);
> +
> +       /* Add acomp req wrapper for dlen */
> +       len = (ctx->acomp_req)->dlen;
> +
> +       af_alg_free_sg(&ctx->tsgl);
> +       af_alg_free_sg(&ctx->rsgl);
> +
> +unlock:
> +       ctx->used = 0;
> +       release_sock(sk);
> +
> +       return err ?: len;
> +}
> +
> +static struct proto_ops algif_comp_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,
> +       .setsockopt     =       sock_no_setsockopt,
> +       .poll           =       sock_no_poll,
> +
> +       .release        =       af_alg_release,
> +       .sendmsg        =       comp_sendmsg,
> +       .recvmsg        =       comp_recvmsg,
> +       .sendpage       =       sock_no_sendpage,
> +       .accept         =       sock_no_accept,
> +};
> +
> +static void *comp_bind(const char *name, u32 type, u32 mask)
> +{
> +       struct comp_tfm *tfm;
> +       struct crypto_acomp *acomp;
> +
> +       tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
> +       if (!tfm)
> +               return ERR_PTR(-ENOMEM);
> +
> +       acomp = crypto_alloc_acomp(name, type, mask);
> +       if (IS_ERR_OR_NULL(acomp)) {
> +               kfree(tfm);
> +               return ERR_PTR(-ENOMEM);
> +       }
> +       tfm->acomp = acomp;
> +
> +       return tfm;
> +}
> +
> +static void comp_release(void *private)
> +{
> +       struct comp_tfm *tfm = private;
> +
> +       crypto_free_acomp(tfm->acomp);
> +       kfree(tfm);
> +}
> +
> +static void comp_sock_destruct(struct sock *sk)
> +{
> +       struct alg_sock *ask = alg_sk(sk);
> +       struct comp_ctx *ctx = ask->private;
> +
> +       acomp_request_free(ctx->acomp_req);
> +       sock_kfree_s(sk, ctx, ctx->clen);
> +       af_alg_release_parent(sk);
> +}
> +
> +static int comp_accept_parent(void *private, struct sock *sk)
> +{
> +       struct comp_ctx *ctx;
> +       struct alg_sock *ask = alg_sk(sk);
> +       struct comp_tfm *tfm = private;
> +       struct crypto_acomp *acomp = tfm->acomp;
> +       unsigned int len = sizeof(*ctx);
> +
> +       ctx = sock_kmalloc(sk, len, GFP_KERNEL);
> +       if (!ctx)
> +               return -ENOMEM;
> +
> +       ctx->used = 0;
> +       ctx->clen = len;
> +       ctx->dlen = COMP_BUFFER_SIZE;
> +       ctx->slen = COMP_BUFFER_SIZE;
> +
> +       af_alg_init_completion(&ctx->completion);
> +
> +       ctx->acomp_req = acomp_request_alloc(acomp);
> +       if (!ctx->acomp_req) {
> +               sock_kfree_s(sk, ctx, ctx->clen);
> +               return -ENOMEM;
> +       }
> +
> +       acomp_request_set_callback(ctx->acomp_req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> +                                  af_alg_complete, &ctx->completion);
> +
> +       ask->private = ctx;
> +       sk->sk_destruct = comp_sock_destruct;
> +
> +       return 0;
> +}
> +
> +static const struct af_alg_type algif_type_comp = {
> +       .bind           =       comp_bind,
> +       .release        =       comp_release,
> +       .accept         =       comp_accept_parent,
> +       .ops            =       &algif_comp_ops,
> +       .name           =       "compression",
> +       .owner          =       THIS_MODULE
> +};
> +
> +static int __init algif_comp_init(void)
> +{
> +       return af_alg_register_type(&algif_type_comp);
> +}
> +
> +static void __exit algif_comp_exit(void)
> +{
> +       int err = af_alg_unregister_type(&algif_type_comp);
> +
> +       BUG_ON(err);
> +}
> +
> +module_init(algif_comp_init);
> +module_exit(algif_comp_exit);
> +MODULE_LICENSE("GPL");
> diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
> index f2acd2f..5cca9eb 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_DECOMPRESS              0
> +#define ALG_OP_COMPRESS                1
>
>  #endif /* _LINUX_IF_ALG_H */
> --
> 2.7.4
>

Regards,
PrasannaKumar
Eric Biggers April 17, 2017, 1:31 a.m. UTC | #2
On Fri, Apr 14, 2017 at 12:04:54AM +0530, Abed Kamaluddin wrote:
> crypto: algif_compression - User-space interface for compression
> 
> This patch adds af_alg plugin for compression algorithms of type scomp/acomp
> registered to the kernel crypto layer.
> 
> The user needs to set operation (compression/decompression) as a control
> message to sendmsg, identical to selecting the cipher operation type in case of
> ciphers. Once a sendmsg call occurs, no further writes can be made to the
> socket until all previous data has been processed and read. Therefore the
> interface only supports one request at a time.
> 
> The interface is completely synchronous; all operations are carried out in
> recvmsg and will complete prior to the system call returning.
> 
> The sendmsg and recvmsg interface supports directly reading/writing to 
> user-space without additional copying, i.e., the kernel crypto interface will
> receive the user-space address as its input/output SG list. The scomp interface
> or crypto drivers may copy the data as required.

Fun, so unprivileged users will be able to feed arbitrary data into the kernel's
zlib, LZ4, LZO, etc. compressors and decompressors.  Including zlib which is 12
years out of date from the upstream version.  Moreover, if anyone decides to
optimize these to directly support the new "acomp" (page-based) API, e.g. for
zlib by using its streaming API, then the algorithms will be passed the actual
userspace memory which can be modified by userspace concurrently.  When people
write compression algorithms usually it's assumed that's not possible.  At the
very least, it's unlikely to have been covered by fuzz testing that's been done
on the original userspace versions of these algorithms.  They might be safe by
chance, but I don't know.

Why does userspace need to be able to call the in-kernel zlib, LZ4, LZO, etc.
anyway?  At the very least, how about limiting the new attack surface by only
exposing algorithms provided by hardware drivers?

Eric
Stephan Mueller April 18, 2017, 10:12 a.m. UTC | #3
Am Donnerstag, 13. April 2017, 20:34:54 CEST schrieb Abed Kamaluddin:

Hi Abed,

> crypto: algif_compression - User-space interface for compression
> 
> This patch adds af_alg plugin for compression algorithms of type scomp/acomp
> registered to the kernel crypto layer.
> 
> The user needs to set operation (compression/decompression) as a control
> message to sendmsg, identical to selecting the cipher operation type in case
> of ciphers. Once a sendmsg call occurs, no further writes can be made to
> the socket until all previous data has been processed and read. Therefore
> the interface only supports one request at a time.
> 
> The interface is completely synchronous; all operations are carried out in
> recvmsg and will complete prior to the system call returning.
> 
> The sendmsg and recvmsg interface supports directly reading/writing to
> user-space without additional copying, i.e., the kernel crypto interface
> will receive the user-space address as its input/output SG list. The scomp
> interface or crypto drivers may copy the data as required.
> 
> Signed-off-by: Abed Kamaluddin <akam...@cavium.com>
> Signed-off-by: Mahipal Challa <mahipal.cha...@cavium.com>
> 
> ---
>  crypto/Kconfig              |  11 ++
>  crypto/Makefile             |   1 +
>  crypto/algif_compression.c  | 272
> ++++++++++++++++++++++++++++++++++++++++++++ include/uapi/linux/if_alg.h | 
>  2 +
>  4 files changed, 286 insertions(+)
> 
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index f37e9cc..13b03ba 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -1741,6 +1741,17 @@ config CRYPTO_USER_API_AEAD
>  	  This option enables the user-spaces interface for AEAD
>  	  cipher algorithms.
> 
> +config CRYPTO_USER_API_COMPRESSION
> +	tristate "User-space interface for compression algorithms"
> +	depends on NET
> +	select CRYPTO_ACOMP
> +	select CRYPTO_USER_API
> +	help
> +	  This option enables the user-space interface for compression
> +	  algorithms. Enable this option for access to compression algorithms
> +	  of type scomp/acomp exported by the kernel crypto layer through
> +	  AF_ALG interface.
> +
>  config CRYPTO_HASH_INFO
>  	bool
> 
> diff --git a/crypto/Makefile b/crypto/Makefile
> index 8a44057..1469e06 100644
> --- a/crypto/Makefile
> +++ b/crypto/Makefile
> @@ -134,6 +134,7 @@ obj-$(CONFIG_CRYPTO_TEST) += tcrypt.o
>  obj-$(CONFIG_CRYPTO_GHASH) += ghash-generic.o
>  obj-$(CONFIG_CRYPTO_USER_API) += af_alg.o
>  obj-$(CONFIG_CRYPTO_USER_API_HASH) += algif_hash.o
> +obj-$(CONFIG_CRYPTO_USER_API_COMPRESSION) += algif_compression.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
> diff --git a/crypto/algif_compression.c b/crypto/algif_compression.c
> new file mode 100644
> index 0000000..0ba6d1e
> --- /dev/null
> +++ b/crypto/algif_compression.c
> @@ -0,0 +1,272 @@
> +/*
> + * algif_compression: User-space interface for COMPRESSION algorithms
> + *
> + * This file provides user-space API support for compression algorithms
> + * registered through the kernel crypto layer.
> + *
> + * Copyright (C) 2017 Cavium, Inc.
> + *
> + * Original Authors: Abed Kamaluddin <akamaluddin@cavium.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> Free + * Software Foundation; either version 2 of the License, or (at your
> option) + * any later version.
> + */
> +
> +#include <crypto/acompress.h>
> +#include <crypto/if_alg.h>
> +#include <linux/crypto.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>
> +#include <linux/scatterlist.h>
> +
> +/* scomp scratch is currently 128KB */
> +#define COMP_BUFFER_SIZE	65535
> +
> +struct comp_ctx {
> +	struct af_alg_sgl tsgl;
> +	struct af_alg_sgl rsgl;
> +	struct af_alg_completion completion;
> +	unsigned int clen;
> +	unsigned int slen;
> +	unsigned int dlen;
> +	bool comp;
> +	bool used;
> +	struct acomp_req *acomp_req;
> +};
> +
> +struct comp_tfm {
> +	struct crypto_acomp *acomp;
> +};
> +
> +static int comp_sendmsg(struct socket *sock, struct msghdr *msg,
> +			size_t ignored)
> +{
> +	struct sock *sk = sock->sk;
> +	struct alg_sock *ask = alg_sk(sk);
> +	struct comp_ctx *ctx = ask->private;
> +	struct af_alg_control con = {};
> +	int limit = COMP_BUFFER_SIZE;
> +	int len;
> +	int err = -EINVAL;
> +
> +	if (msg->msg_controllen) {
> +		err = af_alg_cmsg_send(msg, &con);
> +		if (err)
> +			return err;
> +
> +		switch (con.op) {
> +		case ALG_OP_COMPRESS:
> +			ctx->comp = 1;
> +			break;
> +
> +		case ALG_OP_DECOMPRESS:
> +			ctx->comp = 0;

You set the ctx without locking. I guess you want to move the ctx->comp 
setting below the lock.

> +			break;
> +
> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +
> +	lock_sock(sk);
> +
> +	/* One request at a time supported, data submitted for comp/decomp will
> +	 * be processed at subsequent recvmsg

I recommend to lift that limitation.

May I propose that you look into the patches for skcipher and aead regarding 
memory handling updates that are currently discussed. There you will find the 
sendmsg code with two parts:

1. checking of the input data of cmsg and setting the ctx respectively.

2. the big while loop for getting all user space data

I guess you have seen that I would like to consolidate the algif 
implementations once the memory handling patch is in and agreed on. My plan 
is: Bullet 1 will be private to the algif implementation, so leave your code. 
Bullet 2 will be moved into a common service function. Thus, may I propose 
that you simply copy the entire while loop with the same TX data structures 
into your sendmsg code. This way you do not have the mentioned limit.

Also, simply copy the sendpage code from the patch set to support splice/
vmsplice.



> +	 */
> +	if (ctx->used) {
> +		err = -EAGAIN;
> +		goto unlock;
> +	}
> +
> +	len = msg_data_left(msg);
> +
> +	if (len > limit)
> +		len = limit;
> +
> +	len = af_alg_make_sg(&ctx->tsgl, &msg->msg_iter, len);
> +
> +	if (len < 0) {
> +		err = len;
> +		goto unlock;
> +	}
> +
> +	ctx->slen = len;
> +	ctx->used = 1;
> +
> +unlock:
> +	release_sock(sk);
> +
> +	return err ?: len;
> +}
> +
> +static int comp_recvmsg(struct socket *sock, struct msghdr *msg, size_t
> len, +			int flags)
> +{
> +	struct sock *sk = sock->sk;
> +	struct alg_sock *ask = alg_sk(sk);
> +	struct comp_ctx *ctx = ask->private;
> +	int rlen = ctx->dlen;
> +	int err;
> +
> +	if (len > rlen)
> +		len = rlen;
> +
> +	lock_sock(sk);
> +
> +	if (!ctx->used) {
> +		err = -EAGAIN;
> +		goto unlock;
> +	}
> +
> +	len = af_alg_make_sg(&ctx->rsgl, &msg->msg_iter, len);
> +	if (len < 0) {
> +		err = len;
> +		goto unlock;
> +	}

As mentioned above, I recommend to change this code here too. Please copy the 
while loop from the skcipher_recvmsg and the associated data structures. Also, 
copy the areq allocation and handling code to support synchronous and async 
operations.

Note, the code below the while loop in the recvmsg is private to your 
implementation.
> +
> +	acomp_request_set_params(ctx->acomp_req, ctx->tsgl.sg, ctx->rsgl.sg,
> +				 ctx->slen, len);
> +
> +	/* Synchronous completion of comp/decomp requests */
> +	err = af_alg_wait_for_completion(
> +			ctx->comp ?
> +				crypto_acomp_compress(ctx->acomp_req) :
> +				crypto_acomp_decompress(ctx->acomp_req),
> +				&ctx->completion);
> +
> +	/* Add acomp req wrapper for dlen */
> +	len = (ctx->acomp_req)->dlen;
> +
> +	af_alg_free_sg(&ctx->tsgl);
> +	af_alg_free_sg(&ctx->rsgl);
> +
> +unlock:
> +	ctx->used = 0;
> +	release_sock(sk);
> +
> +	return err ?: len;
> +}
> +
> +static struct proto_ops algif_comp_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,
> +	.setsockopt	=	sock_no_setsockopt,
> +	.poll		=	sock_no_poll,
> +
> +	.release	=	af_alg_release,
> +	.sendmsg	=	comp_sendmsg,
> +	.recvmsg	=	comp_recvmsg,
> +	.sendpage	=	sock_no_sendpage,
> +	.accept		=	sock_no_accept,
> +};
> +
> +static void *comp_bind(const char *name, u32 type, u32 mask)
> +{
> +	struct comp_tfm *tfm;
> +	struct crypto_acomp *acomp;
> +
> +	tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
> +	if (!tfm)
> +		return ERR_PTR(-ENOMEM);
> +
> +	acomp = crypto_alloc_acomp(name, type, mask);
> +	if (IS_ERR_OR_NULL(acomp)) {
> +		kfree(tfm);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +	tfm->acomp = acomp;
> +
> +	return tfm;
> +}
> +
> +static void comp_release(void *private)
> +{
> +	struct comp_tfm *tfm = private;
> +
> +	crypto_free_acomp(tfm->acomp);
> +	kfree(tfm);
> +}
> +
> +static void comp_sock_destruct(struct sock *sk)
> +{
> +	struct alg_sock *ask = alg_sk(sk);
> +	struct comp_ctx *ctx = ask->private;
> +
> +	acomp_request_free(ctx->acomp_req);
> +	sock_kfree_s(sk, ctx, ctx->clen);
> +	af_alg_release_parent(sk);
> +}
> +
> +static int comp_accept_parent(void *private, struct sock *sk)
> +{
> +	struct comp_ctx *ctx;
> +	struct alg_sock *ask = alg_sk(sk);
> +	struct comp_tfm *tfm = private;
> +	struct crypto_acomp *acomp = tfm->acomp;
> +	unsigned int len = sizeof(*ctx);
> +
> +	ctx = sock_kmalloc(sk, len, GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	ctx->used = 0;
> +	ctx->clen = len;
> +	ctx->dlen = COMP_BUFFER_SIZE;
> +	ctx->slen = COMP_BUFFER_SIZE;
> +
> +	af_alg_init_completion(&ctx->completion);
> +
> +	ctx->acomp_req = acomp_request_alloc(acomp);
> +	if (!ctx->acomp_req) {
> +		sock_kfree_s(sk, ctx, ctx->clen);
> +		return -ENOMEM;
> +	}
> +
> +	acomp_request_set_callback(ctx->acomp_req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> +				   af_alg_complete, &ctx->completion);
> +
> +	ask->private = ctx;
> +	sk->sk_destruct = comp_sock_destruct;
> +
> +	return 0;
> +}
> +
> +static const struct af_alg_type algif_type_comp = {
> +	.bind		=	comp_bind,
> +	.release	=	comp_release,
> +	.accept		=	comp_accept_parent,
> +	.ops		=	&algif_comp_ops,
> +	.name		=	"compression",
> +	.owner		=	THIS_MODULE
> +};
> +
> +static int __init algif_comp_init(void)
> +{
> +	return af_alg_register_type(&algif_type_comp);
> +}
> +
> +static void __exit algif_comp_exit(void)
> +{
> +	int err = af_alg_unregister_type(&algif_type_comp);
> +
> +	BUG_ON(err);
> +}
> +
> +module_init(algif_comp_init);
> +module_exit(algif_comp_exit);
> +MODULE_LICENSE("GPL");
> diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
> index f2acd2f..5cca9eb 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_DECOMPRESS		0
> +#define ALG_OP_COMPRESS		1
> 
>  #endif	/* _LINUX_IF_ALG_H */


Ciao
Stephan
Abed Kamaluddin April 21, 2017, 3:42 p.m. UTC | #4
Hi Stephan,

On Tue, Apr 18, 2017 at 3:42 PM, Stephan Müller <smueller@chronox.de> wrote:
> Am Donnerstag, 13. April 2017, 20:34:54 CEST schrieb Abed Kamaluddin:
>
> May I propose that you look into the patches for skcipher and aead regarding
> memory handling updates that are currently discussed. There you will find the
> sendmsg code with two parts:
>
> 1. checking of the input data of cmsg and setting the ctx respectively.
>
> 2. the big while loop for getting all user space data
>
> I guess you have seen that I would like to consolidate the algif
> implementations once the memory handling patch is in and agreed on. My plan
> is: Bullet 1 will be private to the algif implementation, so leave your code.
> Bullet 2 will be moved into a common service function. Thus, may I propose
> that you simply copy the entire while loop with the same TX data structures
> into your sendmsg code. This way you do not have the mentioned limit.
>
> Also, simply copy the sendpage code from the patch set to support splice/
> vmsplice.
>

Thanks for the suggestions and helpful pointers, I will rework the
patch and incorporate these, hoping that the compression interface
will be exported. These changes would also remain unaffected by any
changes to the acomp interface.

Thanks
Abed (Cavium)
Regards,
Stephan Mueller April 21, 2017, 3:50 p.m. UTC | #5
Am Freitag, 21. April 2017, 17:42:10 CEST schrieb abed mohammad kamaluddin:

Hi abed,

> Hi Stephan,
> 
> On Tue, Apr 18, 2017 at 3:42 PM, Stephan Müller <smueller@chronox.de> wrote:
> > Am Donnerstag, 13. April 2017, 20:34:54 CEST schrieb Abed Kamaluddin:
> > 
> > May I propose that you look into the patches for skcipher and aead
> > regarding memory handling updates that are currently discussed. There you
> > will find the sendmsg code with two parts:
> > 
> > 1. checking of the input data of cmsg and setting the ctx respectively.
> > 
> > 2. the big while loop for getting all user space data
> > 
> > I guess you have seen that I would like to consolidate the algif
> > implementations once the memory handling patch is in and agreed on. My
> > plan
> > is: Bullet 1 will be private to the algif implementation, so leave your
> > code. Bullet 2 will be moved into a common service function. Thus, may I
> > propose that you simply copy the entire while loop with the same TX data
> > structures into your sendmsg code. This way you do not have the mentioned
> > limit.
> > 
> > Also, simply copy the sendpage code from the patch set to support splice/
> > vmsplice.
> 
> Thanks for the suggestions and helpful pointers, I will rework the
> patch and incorporate these, hoping that the compression interface
> will be exported. These changes would also remain unaffected by any
> changes to the acomp interface.

Just diff the just RFCed algif_kpp with the proposed patch set for 
algif_skcipher and algif_aead. There you will see that 80% of all code is 
identical (if you disregard the different namespace). And that is the code I 
am referring to.
> 
> Thanks
> Abed (Cavium)
> Regards,



Ciao
Stephan
Abed Kamaluddin April 21, 2017, 4:07 p.m. UTC | #6
On Fri, Apr 21, 2017 at 9:20 PM, Stephan Müller <smueller@chronox.de> wrote:
> Am Freitag, 21. April 2017, 17:42:10 CEST schrieb abed mohammad kamaluddin:
>
> Just diff the just RFCed algif_kpp with the proposed patch set for
> algif_skcipher and algif_aead. There you will see that 80% of all code is
> identical (if you disregard the different namespace). And that is the code I
> am referring to.
>
> Ciao
> Stephan

Thanks, I will use the pointer. Considering all the identical code,
the proposal to consolidate would definitely help!

Thanks,
Abed
diff mbox

Patch

diff --git a/crypto/Kconfig b/crypto/Kconfig
index f37e9cc..13b03ba 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1741,6 +1741,17 @@  config CRYPTO_USER_API_AEAD
 	  This option enables the user-spaces interface for AEAD
 	  cipher algorithms.
 
+config CRYPTO_USER_API_COMPRESSION
+	tristate "User-space interface for compression algorithms"
+	depends on NET
+	select CRYPTO_ACOMP
+	select CRYPTO_USER_API
+	help
+	  This option enables the user-space interface for compression
+	  algorithms. Enable this option for access to compression algorithms
+	  of type scomp/acomp exported by the kernel crypto layer through
+	  AF_ALG interface.
+
 config CRYPTO_HASH_INFO
 	bool
 
diff --git a/crypto/Makefile b/crypto/Makefile
index 8a44057..1469e06 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -134,6 +134,7 @@  obj-$(CONFIG_CRYPTO_TEST) += tcrypt.o
 obj-$(CONFIG_CRYPTO_GHASH) += ghash-generic.o
 obj-$(CONFIG_CRYPTO_USER_API) += af_alg.o
 obj-$(CONFIG_CRYPTO_USER_API_HASH) += algif_hash.o
+obj-$(CONFIG_CRYPTO_USER_API_COMPRESSION) += algif_compression.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
diff --git a/crypto/algif_compression.c b/crypto/algif_compression.c
new file mode 100644
index 0000000..0ba6d1e
--- /dev/null
+++ b/crypto/algif_compression.c
@@ -0,0 +1,272 @@ 
+/*
+ * algif_compression: User-space interface for COMPRESSION algorithms
+ *
+ * This file provides user-space API support for compression algorithms
+ * registered through the kernel crypto layer.
+ *
+ * Copyright (C) 2017 Cavium, Inc.
+ *
+ * Original Authors: Abed Kamaluddin <akamaluddin@cavium.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#include <crypto/acompress.h>
+#include <crypto/if_alg.h>
+#include <linux/crypto.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>
+#include <linux/scatterlist.h>
+
+/* scomp scratch is currently 128KB */
+#define COMP_BUFFER_SIZE	65535
+
+struct comp_ctx {
+	struct af_alg_sgl tsgl;
+	struct af_alg_sgl rsgl;
+	struct af_alg_completion completion;
+	unsigned int clen;
+	unsigned int slen;
+	unsigned int dlen;
+	bool comp;
+	bool used;
+	struct acomp_req *acomp_req;
+};
+
+struct comp_tfm {
+	struct crypto_acomp *acomp;
+};
+
+static int comp_sendmsg(struct socket *sock, struct msghdr *msg,
+			size_t ignored)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct comp_ctx *ctx = ask->private;
+	struct af_alg_control con = {};
+	int limit = COMP_BUFFER_SIZE;
+	int len;
+	int err = -EINVAL;
+
+	if (msg->msg_controllen) {
+		err = af_alg_cmsg_send(msg, &con);
+		if (err)
+			return err;
+
+		switch (con.op) {
+		case ALG_OP_COMPRESS:
+			ctx->comp = 1;
+			break;
+
+		case ALG_OP_DECOMPRESS:
+			ctx->comp = 0;
+			break;
+
+		default:
+			return -EINVAL;
+		}
+	}
+
+	lock_sock(sk);
+
+	/* One request at a time supported, data submitted for comp/decomp will
+	 * be processed at subsequent recvmsg
+	 */
+	if (ctx->used) {
+		err = -EAGAIN;
+		goto unlock;
+	}
+
+	len = msg_data_left(msg);
+
+	if (len > limit)
+		len = limit;
+
+	len = af_alg_make_sg(&ctx->tsgl, &msg->msg_iter, len);
+
+	if (len < 0) {
+		err = len;
+		goto unlock;
+	}
+
+	ctx->slen = len;
+	ctx->used = 1;
+
+unlock:
+	release_sock(sk);
+
+	return err ?: len;
+}
+
+static int comp_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
+			int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct comp_ctx *ctx = ask->private;
+	int rlen = ctx->dlen;
+	int err;
+
+	if (len > rlen)
+		len = rlen;
+
+	lock_sock(sk);
+
+	if (!ctx->used) {
+		err = -EAGAIN;
+		goto unlock;
+	}
+
+	len = af_alg_make_sg(&ctx->rsgl, &msg->msg_iter, len);
+	if (len < 0) {
+		err = len;
+		goto unlock;
+	}
+
+	acomp_request_set_params(ctx->acomp_req, ctx->tsgl.sg, ctx->rsgl.sg,
+				 ctx->slen, len);
+
+	/* Synchronous completion of comp/decomp requests */
+	err = af_alg_wait_for_completion(
+			ctx->comp ?
+				crypto_acomp_compress(ctx->acomp_req) :
+				crypto_acomp_decompress(ctx->acomp_req),
+				&ctx->completion);
+
+	/* Add acomp req wrapper for dlen */
+	len = (ctx->acomp_req)->dlen;
+
+	af_alg_free_sg(&ctx->tsgl);
+	af_alg_free_sg(&ctx->rsgl);
+
+unlock:
+	ctx->used = 0;
+	release_sock(sk);
+
+	return err ?: len;
+}
+
+static struct proto_ops algif_comp_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,
+	.setsockopt	=	sock_no_setsockopt,
+	.poll		=	sock_no_poll,
+
+	.release	=	af_alg_release,
+	.sendmsg	=	comp_sendmsg,
+	.recvmsg	=	comp_recvmsg,
+	.sendpage	=	sock_no_sendpage,
+	.accept		=	sock_no_accept,
+};
+
+static void *comp_bind(const char *name, u32 type, u32 mask)
+{
+	struct comp_tfm *tfm;
+	struct crypto_acomp *acomp;
+
+	tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
+	if (!tfm)
+		return ERR_PTR(-ENOMEM);
+
+	acomp = crypto_alloc_acomp(name, type, mask);
+	if (IS_ERR_OR_NULL(acomp)) {
+		kfree(tfm);
+		return ERR_PTR(-ENOMEM);
+	}
+	tfm->acomp = acomp;
+
+	return tfm;
+}
+
+static void comp_release(void *private)
+{
+	struct comp_tfm *tfm = private;
+
+	crypto_free_acomp(tfm->acomp);
+	kfree(tfm);
+}
+
+static void comp_sock_destruct(struct sock *sk)
+{
+	struct alg_sock *ask = alg_sk(sk);
+	struct comp_ctx *ctx = ask->private;
+
+	acomp_request_free(ctx->acomp_req);
+	sock_kfree_s(sk, ctx, ctx->clen);
+	af_alg_release_parent(sk);
+}
+
+static int comp_accept_parent(void *private, struct sock *sk)
+{
+	struct comp_ctx *ctx;
+	struct alg_sock *ask = alg_sk(sk);
+	struct comp_tfm *tfm = private;
+	struct crypto_acomp *acomp = tfm->acomp;
+	unsigned int len = sizeof(*ctx);
+
+	ctx = sock_kmalloc(sk, len, GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->used = 0;
+	ctx->clen = len;
+	ctx->dlen = COMP_BUFFER_SIZE;
+	ctx->slen = COMP_BUFFER_SIZE;
+
+	af_alg_init_completion(&ctx->completion);
+
+	ctx->acomp_req = acomp_request_alloc(acomp);
+	if (!ctx->acomp_req) {
+		sock_kfree_s(sk, ctx, ctx->clen);
+		return -ENOMEM;
+	}
+
+	acomp_request_set_callback(ctx->acomp_req, CRYPTO_TFM_REQ_MAY_BACKLOG,
+				   af_alg_complete, &ctx->completion);
+
+	ask->private = ctx;
+	sk->sk_destruct = comp_sock_destruct;
+
+	return 0;
+}
+
+static const struct af_alg_type algif_type_comp = {
+	.bind		=	comp_bind,
+	.release	=	comp_release,
+	.accept		=	comp_accept_parent,
+	.ops		=	&algif_comp_ops,
+	.name		=	"compression",
+	.owner		=	THIS_MODULE
+};
+
+static int __init algif_comp_init(void)
+{
+	return af_alg_register_type(&algif_type_comp);
+}
+
+static void __exit algif_comp_exit(void)
+{
+	int err = af_alg_unregister_type(&algif_type_comp);
+
+	BUG_ON(err);
+}
+
+module_init(algif_comp_init);
+module_exit(algif_comp_exit);
+MODULE_LICENSE("GPL");
diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
index f2acd2f..5cca9eb 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_DECOMPRESS		0
+#define ALG_OP_COMPRESS		1
 
 #endif	/* _LINUX_IF_ALG_H */