stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Patch "libceph: introduce ceph_crypt() for in-place en/decryption" has been added to the 4.9-stable tree
@ 2017-01-20 14:53 gregkh
  2017-01-20 15:05 ` Ilya Dryomov
  0 siblings, 1 reply; 12+ messages in thread
From: gregkh @ 2017-01-20 14:53 UTC (permalink / raw)
  To: idryomov, gregkh, sage, spender; +Cc: stable, stable-commits


This is a note to let you know that I've just added the patch titled

    libceph: introduce ceph_crypt() for in-place en/decryption

to the 4.9-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     libceph-introduce-ceph_crypt-for-in-place-en-decryption.patch
and it can be found in the queue-4.9 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From a45f795c65b479b4ba107b6ccde29b896d51ee98 Mon Sep 17 00:00:00 2001
From: Ilya Dryomov <idryomov@gmail.com>
Date: Fri, 2 Dec 2016 16:35:07 +0100
Subject: libceph: introduce ceph_crypt() for in-place en/decryption

From: Ilya Dryomov <idryomov@gmail.com>

commit a45f795c65b479b4ba107b6ccde29b896d51ee98 upstream.

Starting with 4.9, kernel stacks may be vmalloced and therefore not
guaranteed to be physically contiguous; the new CONFIG_VMAP_STACK
option is enabled by default on x86.  This makes it invalid to use
on-stack buffers with the crypto scatterlist API, as sg_set_buf()
expects a logical address and won't work with vmalloced addresses.

There isn't a different (e.g. kvec-based) crypto API we could switch
net/ceph/crypto.c to and the current scatterlist.h API isn't getting
updated to accommodate this use case.  Allocating a new header and
padding for each operation is a non-starter, so do the en/decryption
in-place on a single pre-assembled (header + data + padding) heap
buffer.  This is explicitly supported by the crypto API:

    "... the caller may provide the same scatter/gather list for the
     plaintext and cipher text. After the completion of the cipher
     operation, the plaintext data is replaced with the ciphertext data
     in case of an encryption and vice versa for a decryption."

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Reviewed-by: Sage Weil <sage@redhat.com>
Cc: Brad Spengler <spender@grsecurity.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 net/ceph/crypto.c |   87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/ceph/crypto.h |    2 +
 2 files changed, 89 insertions(+)

--- a/net/ceph/crypto.c
+++ b/net/ceph/crypto.c
@@ -526,6 +526,93 @@ int ceph_encrypt2(struct ceph_crypto_key
 	}
 }
 
+static int ceph_aes_crypt(const struct ceph_crypto_key *key, bool encrypt,
+			  void *buf, int buf_len, int in_len, int *pout_len)
+{
+	struct crypto_skcipher *tfm = ceph_crypto_alloc_cipher();
+	SKCIPHER_REQUEST_ON_STACK(req, tfm);
+	struct sg_table sgt;
+	struct scatterlist prealloc_sg;
+	char iv[AES_BLOCK_SIZE];
+	int pad_byte = AES_BLOCK_SIZE - (in_len & (AES_BLOCK_SIZE - 1));
+	int crypt_len = encrypt ? in_len + pad_byte : in_len;
+	int ret;
+
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
+
+	WARN_ON(crypt_len > buf_len);
+	if (encrypt)
+		memset(buf + in_len, pad_byte, pad_byte);
+	ret = setup_sgtable(&sgt, &prealloc_sg, buf, crypt_len);
+	if (ret)
+		goto out_tfm;
+
+	crypto_skcipher_setkey((void *)tfm, key->key, key->len);
+	memcpy(iv, aes_iv, AES_BLOCK_SIZE);
+
+	skcipher_request_set_tfm(req, tfm);
+	skcipher_request_set_callback(req, 0, NULL, NULL);
+	skcipher_request_set_crypt(req, sgt.sgl, sgt.sgl, crypt_len, iv);
+
+	/*
+	print_hex_dump(KERN_ERR, "key: ", DUMP_PREFIX_NONE, 16, 1,
+		       key->key, key->len, 1);
+	print_hex_dump(KERN_ERR, " in: ", DUMP_PREFIX_NONE, 16, 1,
+		       buf, crypt_len, 1);
+	*/
+	if (encrypt)
+		ret = crypto_skcipher_encrypt(req);
+	else
+		ret = crypto_skcipher_decrypt(req);
+	skcipher_request_zero(req);
+	if (ret) {
+		pr_err("%s %scrypt failed: %d\n", __func__,
+		       encrypt ? "en" : "de", ret);
+		goto out_sgt;
+	}
+	/*
+	print_hex_dump(KERN_ERR, "out: ", DUMP_PREFIX_NONE, 16, 1,
+		       buf, crypt_len, 1);
+	*/
+
+	if (encrypt) {
+		*pout_len = crypt_len;
+	} else {
+		pad_byte = *(char *)(buf + in_len - 1);
+		if (pad_byte > 0 && pad_byte <= AES_BLOCK_SIZE &&
+		    in_len >= pad_byte) {
+			*pout_len = in_len - pad_byte;
+		} else {
+			pr_err("%s got bad padding %d on in_len %d\n",
+			       __func__, pad_byte, in_len);
+			ret = -EPERM;
+			goto out_sgt;
+		}
+	}
+
+out_sgt:
+	teardown_sgtable(&sgt);
+out_tfm:
+	crypto_free_skcipher(tfm);
+	return ret;
+}
+
+int ceph_crypt(const struct ceph_crypto_key *key, bool encrypt,
+	       void *buf, int buf_len, int in_len, int *pout_len)
+{
+	switch (key->type) {
+	case CEPH_CRYPTO_NONE:
+		*pout_len = in_len;
+		return 0;
+	case CEPH_CRYPTO_AES:
+		return ceph_aes_crypt(key, encrypt, buf, buf_len, in_len,
+				      pout_len);
+	default:
+		return -ENOTSUPP;
+	}
+}
+
 static int ceph_key_preparse(struct key_preparsed_payload *prep)
 {
 	struct ceph_crypto_key *ckey;
--- a/net/ceph/crypto.h
+++ b/net/ceph/crypto.h
@@ -43,6 +43,8 @@ int ceph_encrypt2(struct ceph_crypto_key
 		  void *dst, size_t *dst_len,
 		  const void *src1, size_t src1_len,
 		  const void *src2, size_t src2_len);
+int ceph_crypt(const struct ceph_crypto_key *key, bool encrypt,
+	       void *buf, int buf_len, int in_len, int *pout_len);
 int ceph_crypto_init(void);
 void ceph_crypto_shutdown(void);
 


Patches currently in stable-queue which might be from idryomov@gmail.com are

queue-4.9/libceph-introduce-ceph_crypt-for-in-place-en-decryption.patch

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch "libceph: introduce ceph_crypt() for in-place en/decryption" has been added to the 4.9-stable tree
  2017-01-20 14:53 Patch "libceph: introduce ceph_crypt() for in-place en/decryption" has been added to the 4.9-stable tree gregkh
@ 2017-01-20 15:05 ` Ilya Dryomov
  2017-01-20 15:08   ` Greg KH
  0 siblings, 1 reply; 12+ messages in thread
From: Ilya Dryomov @ 2017-01-20 15:05 UTC (permalink / raw)
  To: Greg KH; +Cc: Sage Weil, spender, stable, stable-commits

On Fri, Jan 20, 2017 at 3:53 PM,  <gregkh@linuxfoundation.org> wrote:
>
> This is a note to let you know that I've just added the patch titled
>
>     libceph: introduce ceph_crypt() for in-place en/decryption
>
> to the 4.9-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>
> The filename of the patch is:
>      libceph-introduce-ceph_crypt-for-in-place-en-decryption.patch
> and it can be found in the queue-4.9 subdirectory.
>
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@vger.kernel.org> know about it.
>
>
> From a45f795c65b479b4ba107b6ccde29b896d51ee98 Mon Sep 17 00:00:00 2001
> From: Ilya Dryomov <idryomov@gmail.com>
> Date: Fri, 2 Dec 2016 16:35:07 +0100
> Subject: libceph: introduce ceph_crypt() for in-place en/decryption
>
> From: Ilya Dryomov <idryomov@gmail.com>
>
> commit a45f795c65b479b4ba107b6ccde29b896d51ee98 upstream.
>
> Starting with 4.9, kernel stacks may be vmalloced and therefore not
> guaranteed to be physically contiguous; the new CONFIG_VMAP_STACK
> option is enabled by default on x86.  This makes it invalid to use
> on-stack buffers with the crypto scatterlist API, as sg_set_buf()
> expects a logical address and won't work with vmalloced addresses.
>
> There isn't a different (e.g. kvec-based) crypto API we could switch
> net/ceph/crypto.c to and the current scatterlist.h API isn't getting
> updated to accommodate this use case.  Allocating a new header and
> padding for each operation is a non-starter, so do the en/decryption
> in-place on a single pre-assembled (header + data + padding) heap
> buffer.  This is explicitly supported by the crypto API:
>
>     "... the caller may provide the same scatter/gather list for the
>      plaintext and cipher text. After the completion of the cipher
>      operation, the plaintext data is replaced with the ciphertext data
>      in case of an encryption and vice versa for a decryption."
>
> Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
> Reviewed-by: Sage Weil <sage@redhat.com>
> Cc: Brad Spengler <spender@grsecurity.net>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> ---
>  net/ceph/crypto.c |   87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  net/ceph/crypto.h |    2 +
>  2 files changed, 89 insertions(+)
>
> --- a/net/ceph/crypto.c
> +++ b/net/ceph/crypto.c
> @@ -526,6 +526,93 @@ int ceph_encrypt2(struct ceph_crypto_key
>         }
>  }
>
> +static int ceph_aes_crypt(const struct ceph_crypto_key *key, bool encrypt,
> +                         void *buf, int buf_len, int in_len, int *pout_len)
> +{
> +       struct crypto_skcipher *tfm = ceph_crypto_alloc_cipher();
> +       SKCIPHER_REQUEST_ON_STACK(req, tfm);
> +       struct sg_table sgt;
> +       struct scatterlist prealloc_sg;
> +       char iv[AES_BLOCK_SIZE];
> +       int pad_byte = AES_BLOCK_SIZE - (in_len & (AES_BLOCK_SIZE - 1));
> +       int crypt_len = encrypt ? in_len + pad_byte : in_len;
> +       int ret;
> +
> +       if (IS_ERR(tfm))
> +               return PTR_ERR(tfm);
> +
> +       WARN_ON(crypt_len > buf_len);
> +       if (encrypt)
> +               memset(buf + in_len, pad_byte, pad_byte);
> +       ret = setup_sgtable(&sgt, &prealloc_sg, buf, crypt_len);
> +       if (ret)
> +               goto out_tfm;
> +
> +       crypto_skcipher_setkey((void *)tfm, key->key, key->len);
> +       memcpy(iv, aes_iv, AES_BLOCK_SIZE);
> +
> +       skcipher_request_set_tfm(req, tfm);
> +       skcipher_request_set_callback(req, 0, NULL, NULL);
> +       skcipher_request_set_crypt(req, sgt.sgl, sgt.sgl, crypt_len, iv);
> +
> +       /*
> +       print_hex_dump(KERN_ERR, "key: ", DUMP_PREFIX_NONE, 16, 1,
> +                      key->key, key->len, 1);
> +       print_hex_dump(KERN_ERR, " in: ", DUMP_PREFIX_NONE, 16, 1,
> +                      buf, crypt_len, 1);
> +       */
> +       if (encrypt)
> +               ret = crypto_skcipher_encrypt(req);
> +       else
> +               ret = crypto_skcipher_decrypt(req);
> +       skcipher_request_zero(req);
> +       if (ret) {
> +               pr_err("%s %scrypt failed: %d\n", __func__,
> +                      encrypt ? "en" : "de", ret);
> +               goto out_sgt;
> +       }
> +       /*
> +       print_hex_dump(KERN_ERR, "out: ", DUMP_PREFIX_NONE, 16, 1,
> +                      buf, crypt_len, 1);
> +       */
> +
> +       if (encrypt) {
> +               *pout_len = crypt_len;
> +       } else {
> +               pad_byte = *(char *)(buf + in_len - 1);
> +               if (pad_byte > 0 && pad_byte <= AES_BLOCK_SIZE &&
> +                   in_len >= pad_byte) {
> +                       *pout_len = in_len - pad_byte;
> +               } else {
> +                       pr_err("%s got bad padding %d on in_len %d\n",
> +                              __func__, pad_byte, in_len);
> +                       ret = -EPERM;
> +                       goto out_sgt;
> +               }
> +       }
> +
> +out_sgt:
> +       teardown_sgtable(&sgt);
> +out_tfm:
> +       crypto_free_skcipher(tfm);
> +       return ret;
> +}
> +
> +int ceph_crypt(const struct ceph_crypto_key *key, bool encrypt,
> +              void *buf, int buf_len, int in_len, int *pout_len)
> +{
> +       switch (key->type) {
> +       case CEPH_CRYPTO_NONE:
> +               *pout_len = in_len;
> +               return 0;
> +       case CEPH_CRYPTO_AES:
> +               return ceph_aes_crypt(key, encrypt, buf, buf_len, in_len,
> +                                     pout_len);
> +       default:
> +               return -ENOTSUPP;
> +       }
> +}
> +
>  static int ceph_key_preparse(struct key_preparsed_payload *prep)
>  {
>         struct ceph_crypto_key *ckey;
> --- a/net/ceph/crypto.h
> +++ b/net/ceph/crypto.h
> @@ -43,6 +43,8 @@ int ceph_encrypt2(struct ceph_crypto_key
>                   void *dst, size_t *dst_len,
>                   const void *src1, size_t src1_len,
>                   const void *src2, size_t src2_len);
> +int ceph_crypt(const struct ceph_crypto_key *key, bool encrypt,
> +              void *buf, int buf_len, int in_len, int *pout_len);
>  int ceph_crypto_init(void);
>  void ceph_crypto_shutdown(void);
>
>
>
> Patches currently in stable-queue which might be from idryomov@gmail.com are
>
> queue-4.9/libceph-introduce-ceph_crypt-for-in-place-en-decryption.patch

Hi Greg,

Taking this commit by itself without the surrounding commits
(altogether about a dozen) doesn't make any sense.  It wasn't marked
for stable in any way and shouldn't be included.

Thanks,

                Ilya

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch "libceph: introduce ceph_crypt() for in-place en/decryption" has been added to the 4.9-stable tree
  2017-01-20 15:05 ` Ilya Dryomov
@ 2017-01-20 15:08   ` Greg KH
  2017-01-20 15:12     ` Greg KH
  2017-01-20 15:26     ` Ilya Dryomov
  0 siblings, 2 replies; 12+ messages in thread
From: Greg KH @ 2017-01-20 15:08 UTC (permalink / raw)
  To: Ilya Dryomov; +Cc: Sage Weil, spender, stable, stable-commits

On Fri, Jan 20, 2017 at 04:05:04PM +0100, Ilya Dryomov wrote:
> On Fri, Jan 20, 2017 at 3:53 PM,  <gregkh@linuxfoundation.org> wrote:
> >
> > This is a note to let you know that I've just added the patch titled
> >
> >     libceph: introduce ceph_crypt() for in-place en/decryption
> >
> > to the 4.9-stable tree which can be found at:
> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> >
> > The filename of the patch is:
> >      libceph-introduce-ceph_crypt-for-in-place-en-decryption.patch
> > and it can be found in the queue-4.9 subdirectory.
> >
> > If you, or anyone else, feels it should not be added to the stable tree,
> > please let <stable@vger.kernel.org> know about it.
> >
> >
> > From a45f795c65b479b4ba107b6ccde29b896d51ee98 Mon Sep 17 00:00:00 2001
> > From: Ilya Dryomov <idryomov@gmail.com>
> > Date: Fri, 2 Dec 2016 16:35:07 +0100
> > Subject: libceph: introduce ceph_crypt() for in-place en/decryption
> >
> > From: Ilya Dryomov <idryomov@gmail.com>
> >
> > commit a45f795c65b479b4ba107b6ccde29b896d51ee98 upstream.
> >
> > Starting with 4.9, kernel stacks may be vmalloced and therefore not
> > guaranteed to be physically contiguous; the new CONFIG_VMAP_STACK
> > option is enabled by default on x86.  This makes it invalid to use
> > on-stack buffers with the crypto scatterlist API, as sg_set_buf()
> > expects a logical address and won't work with vmalloced addresses.
> >
> > There isn't a different (e.g. kvec-based) crypto API we could switch
> > net/ceph/crypto.c to and the current scatterlist.h API isn't getting
> > updated to accommodate this use case.  Allocating a new header and
> > padding for each operation is a non-starter, so do the en/decryption
> > in-place on a single pre-assembled (header + data + padding) heap
> > buffer.  This is explicitly supported by the crypto API:
> >
> >     "... the caller may provide the same scatter/gather list for the
> >      plaintext and cipher text. After the completion of the cipher
> >      operation, the plaintext data is replaced with the ciphertext data
> >      in case of an encryption and vice versa for a decryption."
> >
> > Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
> > Reviewed-by: Sage Weil <sage@redhat.com>
> > Cc: Brad Spengler <spender@grsecurity.net>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >
> > ---
> >  net/ceph/crypto.c |   87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  net/ceph/crypto.h |    2 +
> >  2 files changed, 89 insertions(+)
> >
> > --- a/net/ceph/crypto.c
> > +++ b/net/ceph/crypto.c
> > @@ -526,6 +526,93 @@ int ceph_encrypt2(struct ceph_crypto_key
> >         }
> >  }
> >
> > +static int ceph_aes_crypt(const struct ceph_crypto_key *key, bool encrypt,
> > +                         void *buf, int buf_len, int in_len, int *pout_len)
> > +{
> > +       struct crypto_skcipher *tfm = ceph_crypto_alloc_cipher();
> > +       SKCIPHER_REQUEST_ON_STACK(req, tfm);
> > +       struct sg_table sgt;
> > +       struct scatterlist prealloc_sg;
> > +       char iv[AES_BLOCK_SIZE];
> > +       int pad_byte = AES_BLOCK_SIZE - (in_len & (AES_BLOCK_SIZE - 1));
> > +       int crypt_len = encrypt ? in_len + pad_byte : in_len;
> > +       int ret;
> > +
> > +       if (IS_ERR(tfm))
> > +               return PTR_ERR(tfm);
> > +
> > +       WARN_ON(crypt_len > buf_len);
> > +       if (encrypt)
> > +               memset(buf + in_len, pad_byte, pad_byte);
> > +       ret = setup_sgtable(&sgt, &prealloc_sg, buf, crypt_len);
> > +       if (ret)
> > +               goto out_tfm;
> > +
> > +       crypto_skcipher_setkey((void *)tfm, key->key, key->len);
> > +       memcpy(iv, aes_iv, AES_BLOCK_SIZE);
> > +
> > +       skcipher_request_set_tfm(req, tfm);
> > +       skcipher_request_set_callback(req, 0, NULL, NULL);
> > +       skcipher_request_set_crypt(req, sgt.sgl, sgt.sgl, crypt_len, iv);
> > +
> > +       /*
> > +       print_hex_dump(KERN_ERR, "key: ", DUMP_PREFIX_NONE, 16, 1,
> > +                      key->key, key->len, 1);
> > +       print_hex_dump(KERN_ERR, " in: ", DUMP_PREFIX_NONE, 16, 1,
> > +                      buf, crypt_len, 1);
> > +       */
> > +       if (encrypt)
> > +               ret = crypto_skcipher_encrypt(req);
> > +       else
> > +               ret = crypto_skcipher_decrypt(req);
> > +       skcipher_request_zero(req);
> > +       if (ret) {
> > +               pr_err("%s %scrypt failed: %d\n", __func__,
> > +                      encrypt ? "en" : "de", ret);
> > +               goto out_sgt;
> > +       }
> > +       /*
> > +       print_hex_dump(KERN_ERR, "out: ", DUMP_PREFIX_NONE, 16, 1,
> > +                      buf, crypt_len, 1);
> > +       */
> > +
> > +       if (encrypt) {
> > +               *pout_len = crypt_len;
> > +       } else {
> > +               pad_byte = *(char *)(buf + in_len - 1);
> > +               if (pad_byte > 0 && pad_byte <= AES_BLOCK_SIZE &&
> > +                   in_len >= pad_byte) {
> > +                       *pout_len = in_len - pad_byte;
> > +               } else {
> > +                       pr_err("%s got bad padding %d on in_len %d\n",
> > +                              __func__, pad_byte, in_len);
> > +                       ret = -EPERM;
> > +                       goto out_sgt;
> > +               }
> > +       }
> > +
> > +out_sgt:
> > +       teardown_sgtable(&sgt);
> > +out_tfm:
> > +       crypto_free_skcipher(tfm);
> > +       return ret;
> > +}
> > +
> > +int ceph_crypt(const struct ceph_crypto_key *key, bool encrypt,
> > +              void *buf, int buf_len, int in_len, int *pout_len)
> > +{
> > +       switch (key->type) {
> > +       case CEPH_CRYPTO_NONE:
> > +               *pout_len = in_len;
> > +               return 0;
> > +       case CEPH_CRYPTO_AES:
> > +               return ceph_aes_crypt(key, encrypt, buf, buf_len, in_len,
> > +                                     pout_len);
> > +       default:
> > +               return -ENOTSUPP;
> > +       }
> > +}
> > +
> >  static int ceph_key_preparse(struct key_preparsed_payload *prep)
> >  {
> >         struct ceph_crypto_key *ckey;
> > --- a/net/ceph/crypto.h
> > +++ b/net/ceph/crypto.h
> > @@ -43,6 +43,8 @@ int ceph_encrypt2(struct ceph_crypto_key
> >                   void *dst, size_t *dst_len,
> >                   const void *src1, size_t src1_len,
> >                   const void *src2, size_t src2_len);
> > +int ceph_crypt(const struct ceph_crypto_key *key, bool encrypt,
> > +              void *buf, int buf_len, int in_len, int *pout_len);
> >  int ceph_crypto_init(void);
> >  void ceph_crypto_shutdown(void);
> >
> >
> >
> > Patches currently in stable-queue which might be from idryomov@gmail.com are
> >
> > queue-4.9/libceph-introduce-ceph_crypt-for-in-place-en-decryption.patch
> 
> Hi Greg,
> 
> Taking this commit by itself without the surrounding commits
> (altogether about a dozen) doesn't make any sense.  It wasn't marked
> for stable in any way and shouldn't be included.

Ok, what were the surrounding commits?  Don't we need this to handle the
vmalloced stack issue in 4.9?  If not, that's fine, I'll drop this,
otherwise it would be good to fix that up, right?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch "libceph: introduce ceph_crypt() for in-place en/decryption" has been added to the 4.9-stable tree
  2017-01-20 15:08   ` Greg KH
@ 2017-01-20 15:12     ` Greg KH
  2017-01-20 15:26     ` Ilya Dryomov
  1 sibling, 0 replies; 12+ messages in thread
From: Greg KH @ 2017-01-20 15:12 UTC (permalink / raw)
  To: Ilya Dryomov; +Cc: Sage Weil, spender, stable, stable-commits

On Fri, Jan 20, 2017 at 04:08:05PM +0100, Greg KH wrote:
> On Fri, Jan 20, 2017 at 04:05:04PM +0100, Ilya Dryomov wrote:
> > On Fri, Jan 20, 2017 at 3:53 PM,  <gregkh@linuxfoundation.org> wrote:
> > >
> > > This is a note to let you know that I've just added the patch titled
> > >
> > >     libceph: introduce ceph_crypt() for in-place en/decryption
> > >
> > > to the 4.9-stable tree which can be found at:
> > >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > >
> > > The filename of the patch is:
> > >      libceph-introduce-ceph_crypt-for-in-place-en-decryption.patch
> > > and it can be found in the queue-4.9 subdirectory.
> > >
> > > If you, or anyone else, feels it should not be added to the stable tree,
> > > please let <stable@vger.kernel.org> know about it.
> > >
> > >
> > > From a45f795c65b479b4ba107b6ccde29b896d51ee98 Mon Sep 17 00:00:00 2001
> > > From: Ilya Dryomov <idryomov@gmail.com>
> > > Date: Fri, 2 Dec 2016 16:35:07 +0100
> > > Subject: libceph: introduce ceph_crypt() for in-place en/decryption
> > >
> > > From: Ilya Dryomov <idryomov@gmail.com>
> > >
> > > commit a45f795c65b479b4ba107b6ccde29b896d51ee98 upstream.
> > >
> > > Starting with 4.9, kernel stacks may be vmalloced and therefore not
> > > guaranteed to be physically contiguous; the new CONFIG_VMAP_STACK
> > > option is enabled by default on x86.  This makes it invalid to use
> > > on-stack buffers with the crypto scatterlist API, as sg_set_buf()
> > > expects a logical address and won't work with vmalloced addresses.
> > >
> > > There isn't a different (e.g. kvec-based) crypto API we could switch
> > > net/ceph/crypto.c to and the current scatterlist.h API isn't getting
> > > updated to accommodate this use case.  Allocating a new header and
> > > padding for each operation is a non-starter, so do the en/decryption
> > > in-place on a single pre-assembled (header + data + padding) heap
> > > buffer.  This is explicitly supported by the crypto API:
> > >
> > >     "... the caller may provide the same scatter/gather list for the
> > >      plaintext and cipher text. After the completion of the cipher
> > >      operation, the plaintext data is replaced with the ciphertext data
> > >      in case of an encryption and vice versa for a decryption."
> > >
> > > Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
> > > Reviewed-by: Sage Weil <sage@redhat.com>
> > > Cc: Brad Spengler <spender@grsecurity.net>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > >
> > > ---
> > >  net/ceph/crypto.c |   87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  net/ceph/crypto.h |    2 +
> > >  2 files changed, 89 insertions(+)
> > >
> > > --- a/net/ceph/crypto.c
> > > +++ b/net/ceph/crypto.c
> > > @@ -526,6 +526,93 @@ int ceph_encrypt2(struct ceph_crypto_key
> > >         }
> > >  }
> > >
> > > +static int ceph_aes_crypt(const struct ceph_crypto_key *key, bool encrypt,
> > > +                         void *buf, int buf_len, int in_len, int *pout_len)
> > > +{
> > > +       struct crypto_skcipher *tfm = ceph_crypto_alloc_cipher();
> > > +       SKCIPHER_REQUEST_ON_STACK(req, tfm);
> > > +       struct sg_table sgt;
> > > +       struct scatterlist prealloc_sg;
> > > +       char iv[AES_BLOCK_SIZE];
> > > +       int pad_byte = AES_BLOCK_SIZE - (in_len & (AES_BLOCK_SIZE - 1));
> > > +       int crypt_len = encrypt ? in_len + pad_byte : in_len;
> > > +       int ret;
> > > +
> > > +       if (IS_ERR(tfm))
> > > +               return PTR_ERR(tfm);
> > > +
> > > +       WARN_ON(crypt_len > buf_len);
> > > +       if (encrypt)
> > > +               memset(buf + in_len, pad_byte, pad_byte);
> > > +       ret = setup_sgtable(&sgt, &prealloc_sg, buf, crypt_len);
> > > +       if (ret)
> > > +               goto out_tfm;
> > > +
> > > +       crypto_skcipher_setkey((void *)tfm, key->key, key->len);
> > > +       memcpy(iv, aes_iv, AES_BLOCK_SIZE);
> > > +
> > > +       skcipher_request_set_tfm(req, tfm);
> > > +       skcipher_request_set_callback(req, 0, NULL, NULL);
> > > +       skcipher_request_set_crypt(req, sgt.sgl, sgt.sgl, crypt_len, iv);
> > > +
> > > +       /*
> > > +       print_hex_dump(KERN_ERR, "key: ", DUMP_PREFIX_NONE, 16, 1,
> > > +                      key->key, key->len, 1);
> > > +       print_hex_dump(KERN_ERR, " in: ", DUMP_PREFIX_NONE, 16, 1,
> > > +                      buf, crypt_len, 1);
> > > +       */
> > > +       if (encrypt)
> > > +               ret = crypto_skcipher_encrypt(req);
> > > +       else
> > > +               ret = crypto_skcipher_decrypt(req);
> > > +       skcipher_request_zero(req);
> > > +       if (ret) {
> > > +               pr_err("%s %scrypt failed: %d\n", __func__,
> > > +                      encrypt ? "en" : "de", ret);
> > > +               goto out_sgt;
> > > +       }
> > > +       /*
> > > +       print_hex_dump(KERN_ERR, "out: ", DUMP_PREFIX_NONE, 16, 1,
> > > +                      buf, crypt_len, 1);
> > > +       */
> > > +
> > > +       if (encrypt) {
> > > +               *pout_len = crypt_len;
> > > +       } else {
> > > +               pad_byte = *(char *)(buf + in_len - 1);
> > > +               if (pad_byte > 0 && pad_byte <= AES_BLOCK_SIZE &&
> > > +                   in_len >= pad_byte) {
> > > +                       *pout_len = in_len - pad_byte;
> > > +               } else {
> > > +                       pr_err("%s got bad padding %d on in_len %d\n",
> > > +                              __func__, pad_byte, in_len);
> > > +                       ret = -EPERM;
> > > +                       goto out_sgt;
> > > +               }
> > > +       }
> > > +
> > > +out_sgt:
> > > +       teardown_sgtable(&sgt);
> > > +out_tfm:
> > > +       crypto_free_skcipher(tfm);
> > > +       return ret;
> > > +}
> > > +
> > > +int ceph_crypt(const struct ceph_crypto_key *key, bool encrypt,
> > > +              void *buf, int buf_len, int in_len, int *pout_len)
> > > +{
> > > +       switch (key->type) {
> > > +       case CEPH_CRYPTO_NONE:
> > > +               *pout_len = in_len;
> > > +               return 0;
> > > +       case CEPH_CRYPTO_AES:
> > > +               return ceph_aes_crypt(key, encrypt, buf, buf_len, in_len,
> > > +                                     pout_len);
> > > +       default:
> > > +               return -ENOTSUPP;
> > > +       }
> > > +}
> > > +
> > >  static int ceph_key_preparse(struct key_preparsed_payload *prep)
> > >  {
> > >         struct ceph_crypto_key *ckey;
> > > --- a/net/ceph/crypto.h
> > > +++ b/net/ceph/crypto.h
> > > @@ -43,6 +43,8 @@ int ceph_encrypt2(struct ceph_crypto_key
> > >                   void *dst, size_t *dst_len,
> > >                   const void *src1, size_t src1_len,
> > >                   const void *src2, size_t src2_len);
> > > +int ceph_crypt(const struct ceph_crypto_key *key, bool encrypt,
> > > +              void *buf, int buf_len, int in_len, int *pout_len);
> > >  int ceph_crypto_init(void);
> > >  void ceph_crypto_shutdown(void);
> > >
> > >
> > >
> > > Patches currently in stable-queue which might be from idryomov@gmail.com are
> > >
> > > queue-4.9/libceph-introduce-ceph_crypt-for-in-place-en-decryption.patch
> > 
> > Hi Greg,
> > 
> > Taking this commit by itself without the surrounding commits
> > (altogether about a dozen) doesn't make any sense.  It wasn't marked
> > for stable in any way and shouldn't be included.
> 
> Ok, what were the surrounding commits?  Don't we need this to handle the
> vmalloced stack issue in 4.9?  If not, that's fine, I'll drop this,
> otherwise it would be good to fix that up, right?

And yes, you are correct in that this doesn't do anything on it's own, I
haven't made it through my patches today, I shouldn't have pushed this
one until the rest of the tree was done, sorry about that.

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch "libceph: introduce ceph_crypt() for in-place en/decryption" has been added to the 4.9-stable tree
  2017-01-20 15:08   ` Greg KH
  2017-01-20 15:12     ` Greg KH
@ 2017-01-20 15:26     ` Ilya Dryomov
  2017-01-20 15:34       ` Greg KH
  1 sibling, 1 reply; 12+ messages in thread
From: Ilya Dryomov @ 2017-01-20 15:26 UTC (permalink / raw)
  To: Greg KH; +Cc: Sage Weil, spender, stable, stable-commits

On Fri, Jan 20, 2017 at 4:08 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Jan 20, 2017 at 04:05:04PM +0100, Ilya Dryomov wrote:
>> On Fri, Jan 20, 2017 at 3:53 PM,  <gregkh@linuxfoundation.org> wrote:
>> >
>> > This is a note to let you know that I've just added the patch titled
>> >
>> >     libceph: introduce ceph_crypt() for in-place en/decryption
>> >
>> > to the 4.9-stable tree which can be found at:
>> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>> >
>> > The filename of the patch is:
>> >      libceph-introduce-ceph_crypt-for-in-place-en-decryption.patch
>> > and it can be found in the queue-4.9 subdirectory.
>> >
>> > If you, or anyone else, feels it should not be added to the stable tree,
>> > please let <stable@vger.kernel.org> know about it.
>> >
>> >
>> > From a45f795c65b479b4ba107b6ccde29b896d51ee98 Mon Sep 17 00:00:00 2001
>> > From: Ilya Dryomov <idryomov@gmail.com>
>> > Date: Fri, 2 Dec 2016 16:35:07 +0100
>> > Subject: libceph: introduce ceph_crypt() for in-place en/decryption
>> >
>> > From: Ilya Dryomov <idryomov@gmail.com>
>> >
>> > commit a45f795c65b479b4ba107b6ccde29b896d51ee98 upstream.
>> >
>> > Starting with 4.9, kernel stacks may be vmalloced and therefore not
>> > guaranteed to be physically contiguous; the new CONFIG_VMAP_STACK
>> > option is enabled by default on x86.  This makes it invalid to use
>> > on-stack buffers with the crypto scatterlist API, as sg_set_buf()
>> > expects a logical address and won't work with vmalloced addresses.
>> >
>> > There isn't a different (e.g. kvec-based) crypto API we could switch
>> > net/ceph/crypto.c to and the current scatterlist.h API isn't getting
>> > updated to accommodate this use case.  Allocating a new header and
>> > padding for each operation is a non-starter, so do the en/decryption
>> > in-place on a single pre-assembled (header + data + padding) heap
>> > buffer.  This is explicitly supported by the crypto API:
>> >
>> >     "... the caller may provide the same scatter/gather list for the
>> >      plaintext and cipher text. After the completion of the cipher
>> >      operation, the plaintext data is replaced with the ciphertext data
>> >      in case of an encryption and vice versa for a decryption."
>> >
>> > Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
>> > Reviewed-by: Sage Weil <sage@redhat.com>
>> > Cc: Brad Spengler <spender@grsecurity.net>
>> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> >
>> > ---
>> >  net/ceph/crypto.c |   87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >  net/ceph/crypto.h |    2 +
>> >  2 files changed, 89 insertions(+)
>> >
>> > --- a/net/ceph/crypto.c
>> > +++ b/net/ceph/crypto.c
>> > @@ -526,6 +526,93 @@ int ceph_encrypt2(struct ceph_crypto_key
>> >         }
>> >  }
>> >
>> > +static int ceph_aes_crypt(const struct ceph_crypto_key *key, bool encrypt,
>> > +                         void *buf, int buf_len, int in_len, int *pout_len)
>> > +{
>> > +       struct crypto_skcipher *tfm = ceph_crypto_alloc_cipher();
>> > +       SKCIPHER_REQUEST_ON_STACK(req, tfm);
>> > +       struct sg_table sgt;
>> > +       struct scatterlist prealloc_sg;
>> > +       char iv[AES_BLOCK_SIZE];
>> > +       int pad_byte = AES_BLOCK_SIZE - (in_len & (AES_BLOCK_SIZE - 1));
>> > +       int crypt_len = encrypt ? in_len + pad_byte : in_len;
>> > +       int ret;
>> > +
>> > +       if (IS_ERR(tfm))
>> > +               return PTR_ERR(tfm);
>> > +
>> > +       WARN_ON(crypt_len > buf_len);
>> > +       if (encrypt)
>> > +               memset(buf + in_len, pad_byte, pad_byte);
>> > +       ret = setup_sgtable(&sgt, &prealloc_sg, buf, crypt_len);
>> > +       if (ret)
>> > +               goto out_tfm;
>> > +
>> > +       crypto_skcipher_setkey((void *)tfm, key->key, key->len);
>> > +       memcpy(iv, aes_iv, AES_BLOCK_SIZE);
>> > +
>> > +       skcipher_request_set_tfm(req, tfm);
>> > +       skcipher_request_set_callback(req, 0, NULL, NULL);
>> > +       skcipher_request_set_crypt(req, sgt.sgl, sgt.sgl, crypt_len, iv);
>> > +
>> > +       /*
>> > +       print_hex_dump(KERN_ERR, "key: ", DUMP_PREFIX_NONE, 16, 1,
>> > +                      key->key, key->len, 1);
>> > +       print_hex_dump(KERN_ERR, " in: ", DUMP_PREFIX_NONE, 16, 1,
>> > +                      buf, crypt_len, 1);
>> > +       */
>> > +       if (encrypt)
>> > +               ret = crypto_skcipher_encrypt(req);
>> > +       else
>> > +               ret = crypto_skcipher_decrypt(req);
>> > +       skcipher_request_zero(req);
>> > +       if (ret) {
>> > +               pr_err("%s %scrypt failed: %d\n", __func__,
>> > +                      encrypt ? "en" : "de", ret);
>> > +               goto out_sgt;
>> > +       }
>> > +       /*
>> > +       print_hex_dump(KERN_ERR, "out: ", DUMP_PREFIX_NONE, 16, 1,
>> > +                      buf, crypt_len, 1);
>> > +       */
>> > +
>> > +       if (encrypt) {
>> > +               *pout_len = crypt_len;
>> > +       } else {
>> > +               pad_byte = *(char *)(buf + in_len - 1);
>> > +               if (pad_byte > 0 && pad_byte <= AES_BLOCK_SIZE &&
>> > +                   in_len >= pad_byte) {
>> > +                       *pout_len = in_len - pad_byte;
>> > +               } else {
>> > +                       pr_err("%s got bad padding %d on in_len %d\n",
>> > +                              __func__, pad_byte, in_len);
>> > +                       ret = -EPERM;
>> > +                       goto out_sgt;
>> > +               }
>> > +       }
>> > +
>> > +out_sgt:
>> > +       teardown_sgtable(&sgt);
>> > +out_tfm:
>> > +       crypto_free_skcipher(tfm);
>> > +       return ret;
>> > +}
>> > +
>> > +int ceph_crypt(const struct ceph_crypto_key *key, bool encrypt,
>> > +              void *buf, int buf_len, int in_len, int *pout_len)
>> > +{
>> > +       switch (key->type) {
>> > +       case CEPH_CRYPTO_NONE:
>> > +               *pout_len = in_len;
>> > +               return 0;
>> > +       case CEPH_CRYPTO_AES:
>> > +               return ceph_aes_crypt(key, encrypt, buf, buf_len, in_len,
>> > +                                     pout_len);
>> > +       default:
>> > +               return -ENOTSUPP;
>> > +       }
>> > +}
>> > +
>> >  static int ceph_key_preparse(struct key_preparsed_payload *prep)
>> >  {
>> >         struct ceph_crypto_key *ckey;
>> > --- a/net/ceph/crypto.h
>> > +++ b/net/ceph/crypto.h
>> > @@ -43,6 +43,8 @@ int ceph_encrypt2(struct ceph_crypto_key
>> >                   void *dst, size_t *dst_len,
>> >                   const void *src1, size_t src1_len,
>> >                   const void *src2, size_t src2_len);
>> > +int ceph_crypt(const struct ceph_crypto_key *key, bool encrypt,
>> > +              void *buf, int buf_len, int in_len, int *pout_len);
>> >  int ceph_crypto_init(void);
>> >  void ceph_crypto_shutdown(void);
>> >
>> >
>> >
>> > Patches currently in stable-queue which might be from idryomov@gmail.com are
>> >
>> > queue-4.9/libceph-introduce-ceph_crypt-for-in-place-en-decryption.patch
>>
>> Hi Greg,
>>
>> Taking this commit by itself without the surrounding commits
>> (altogether about a dozen) doesn't make any sense.  It wasn't marked
>> for stable in any way and shouldn't be included.
>
> Ok, what were the surrounding commits?  Don't we need this to handle the
> vmalloced stack issue in 4.9?  If not, that's fine, I'll drop this,
> otherwise it would be good to fix that up, right?

Yes, but it's pretty large in terms of diffstat.  Honestly, I'm not
worried -- our on-stack buffers are really small and the chances that
any of them will straddle a page should be tiny.

Here is the list (bottom to top):

2b1e1a7cd0a6 libceph: remove now unused ceph_*{en,de}crypt*() functions
e15fd0a11db0 libceph: switch ceph_x_decrypt() to ceph_crypt()
d03857c63bb0 libceph: switch ceph_x_encrypt() to ceph_crypt()
4eb4517ce7c9 libceph: tweak calcu_signature() a little
7882a26d2e2e libceph: rename and align ceph_x_authorizer::reply_buf
a45f795c65b4 libceph: introduce ceph_crypt() for in-place en/decryption
55d9cc834f93 libceph: introduce ceph_x_encrypt_offset()
462e650451c5 libceph: old_key in process_one_ticket() is redundant
36721ece1e84 libceph: ceph_x_encrypt_buflen() takes in_len

Can probably drop one or two, but you want to take these, I'd rather
you take them all.

Thanks,

                Ilya

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch "libceph: introduce ceph_crypt() for in-place en/decryption" has been added to the 4.9-stable tree
  2017-01-20 15:26     ` Ilya Dryomov
@ 2017-01-20 15:34       ` Greg KH
  2017-01-20 15:57         ` Ilya Dryomov
  0 siblings, 1 reply; 12+ messages in thread
From: Greg KH @ 2017-01-20 15:34 UTC (permalink / raw)
  To: Ilya Dryomov; +Cc: Sage Weil, spender, stable, stable-commits

On Fri, Jan 20, 2017 at 04:26:16PM +0100, Ilya Dryomov wrote:
> On Fri, Jan 20, 2017 at 4:08 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > Ok, what were the surrounding commits?  Don't we need this to handle the
> > vmalloced stack issue in 4.9?  If not, that's fine, I'll drop this,
> > otherwise it would be good to fix that up, right?
> 
> Yes, but it's pretty large in terms of diffstat.  Honestly, I'm not
> worried -- our on-stack buffers are really small and the chances that
> any of them will straddle a page should be tiny.
> 
> Here is the list (bottom to top):
> 
> 2b1e1a7cd0a6 libceph: remove now unused ceph_*{en,de}crypt*() functions
> e15fd0a11db0 libceph: switch ceph_x_decrypt() to ceph_crypt()
> d03857c63bb0 libceph: switch ceph_x_encrypt() to ceph_crypt()
> 4eb4517ce7c9 libceph: tweak calcu_signature() a little
> 7882a26d2e2e libceph: rename and align ceph_x_authorizer::reply_buf
> a45f795c65b4 libceph: introduce ceph_crypt() for in-place en/decryption
> 55d9cc834f93 libceph: introduce ceph_x_encrypt_offset()
> 462e650451c5 libceph: old_key in process_one_ticket() is redundant
> 36721ece1e84 libceph: ceph_x_encrypt_buflen() takes in_len
> 
> Can probably drop one or two, but you want to take these, I'd rather
> you take them all.

Given the length that 4.9 is going to be around, I'd prefer to have this
work correctly.  I'll drop this single patch now, but will queue this
larger list up later when I get a chance to do more testing and review.

Brad, thanks again for pointing this issue out, much appreciated for the
heads up, I had missed this one in my review.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch "libceph: introduce ceph_crypt() for in-place en/decryption" has been added to the 4.9-stable tree
  2017-01-20 15:34       ` Greg KH
@ 2017-01-20 15:57         ` Ilya Dryomov
  2017-01-20 17:38           ` Greg KH
  0 siblings, 1 reply; 12+ messages in thread
From: Ilya Dryomov @ 2017-01-20 15:57 UTC (permalink / raw)
  To: Greg KH; +Cc: Sage Weil, spender, stable, stable-commits

On Fri, Jan 20, 2017 at 4:34 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Jan 20, 2017 at 04:26:16PM +0100, Ilya Dryomov wrote:
>> On Fri, Jan 20, 2017 at 4:08 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > Ok, what were the surrounding commits?  Don't we need this to handle the
>> > vmalloced stack issue in 4.9?  If not, that's fine, I'll drop this,
>> > otherwise it would be good to fix that up, right?
>>
>> Yes, but it's pretty large in terms of diffstat.  Honestly, I'm not
>> worried -- our on-stack buffers are really small and the chances that
>> any of them will straddle a page should be tiny.
>>
>> Here is the list (bottom to top):
>>
>> 2b1e1a7cd0a6 libceph: remove now unused ceph_*{en,de}crypt*() functions
>> e15fd0a11db0 libceph: switch ceph_x_decrypt() to ceph_crypt()
>> d03857c63bb0 libceph: switch ceph_x_encrypt() to ceph_crypt()
>> 4eb4517ce7c9 libceph: tweak calcu_signature() a little
>> 7882a26d2e2e libceph: rename and align ceph_x_authorizer::reply_buf
>> a45f795c65b4 libceph: introduce ceph_crypt() for in-place en/decryption
>> 55d9cc834f93 libceph: introduce ceph_x_encrypt_offset()
>> 462e650451c5 libceph: old_key in process_one_ticket() is redundant
>> 36721ece1e84 libceph: ceph_x_encrypt_buflen() takes in_len
>>
>> Can probably drop one or two, but you want to take these, I'd rather
>> you take them all.
>
> Given the length that 4.9 is going to be around, I'd prefer to have this
> work correctly.  I'll drop this single patch now, but will queue this
> larger list up later when I get a chance to do more testing and review.

Any chance you can also take

7af3ea189a9a libceph: stop allocating a new cipher on every crypto request
6db2304aabb0 libceph: uninline ceph_crypto_key_destroy()

to make 4.9.z even more awesome? ;)

These depend on the ceph_crypt() bunch and fix a writeback deadlock
that has been there forever but started showing up only recently.  The
only reason I didn't mark it for stable was this dependency.

All of the above patches were developed and tested on 4.9, so there
shouldn't be any issues.

Thanks,

                Ilya

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch "libceph: introduce ceph_crypt() for in-place en/decryption" has been added to the 4.9-stable tree
  2017-01-20 15:57         ` Ilya Dryomov
@ 2017-01-20 17:38           ` Greg KH
  2017-01-23 15:41             ` Greg KH
  0 siblings, 1 reply; 12+ messages in thread
From: Greg KH @ 2017-01-20 17:38 UTC (permalink / raw)
  To: Ilya Dryomov; +Cc: Sage Weil, spender, stable, stable-commits

On Fri, Jan 20, 2017 at 04:57:37PM +0100, Ilya Dryomov wrote:
> On Fri, Jan 20, 2017 at 4:34 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Fri, Jan 20, 2017 at 04:26:16PM +0100, Ilya Dryomov wrote:
> >> On Fri, Jan 20, 2017 at 4:08 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> > Ok, what were the surrounding commits?  Don't we need this to handle the
> >> > vmalloced stack issue in 4.9?  If not, that's fine, I'll drop this,
> >> > otherwise it would be good to fix that up, right?
> >>
> >> Yes, but it's pretty large in terms of diffstat.  Honestly, I'm not
> >> worried -- our on-stack buffers are really small and the chances that
> >> any of them will straddle a page should be tiny.
> >>
> >> Here is the list (bottom to top):
> >>
> >> 2b1e1a7cd0a6 libceph: remove now unused ceph_*{en,de}crypt*() functions
> >> e15fd0a11db0 libceph: switch ceph_x_decrypt() to ceph_crypt()
> >> d03857c63bb0 libceph: switch ceph_x_encrypt() to ceph_crypt()
> >> 4eb4517ce7c9 libceph: tweak calcu_signature() a little
> >> 7882a26d2e2e libceph: rename and align ceph_x_authorizer::reply_buf
> >> a45f795c65b4 libceph: introduce ceph_crypt() for in-place en/decryption
> >> 55d9cc834f93 libceph: introduce ceph_x_encrypt_offset()
> >> 462e650451c5 libceph: old_key in process_one_ticket() is redundant
> >> 36721ece1e84 libceph: ceph_x_encrypt_buflen() takes in_len
> >>
> >> Can probably drop one or two, but you want to take these, I'd rather
> >> you take them all.
> >
> > Given the length that 4.9 is going to be around, I'd prefer to have this
> > work correctly.  I'll drop this single patch now, but will queue this
> > larger list up later when I get a chance to do more testing and review.
> 
> Any chance you can also take
> 
> 7af3ea189a9a libceph: stop allocating a new cipher on every crypto request
> 6db2304aabb0 libceph: uninline ceph_crypto_key_destroy()
> 
> to make 4.9.z even more awesome? ;)
> 
> These depend on the ceph_crypt() bunch and fix a writeback deadlock
> that has been there forever but started showing up only recently.  The
> only reason I didn't mark it for stable was this dependency.
> 
> All of the above patches were developed and tested on 4.9, so there
> shouldn't be any issues.

Sure, I'll work to queue these all up for the next round of kernels,
thanks for the git commit ids.

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Patch "libceph: introduce ceph_crypt() for in-place en/decryption" has been added to the 4.9-stable tree
@ 2017-01-23 15:35 gregkh
  0 siblings, 0 replies; 12+ messages in thread
From: gregkh @ 2017-01-23 15:35 UTC (permalink / raw)
  To: idryomov, gregkh, sage; +Cc: stable, stable-commits


This is a note to let you know that I've just added the patch titled

    libceph: introduce ceph_crypt() for in-place en/decryption

to the 4.9-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     libceph-introduce-ceph_crypt-for-in-place-en-decryption.patch
and it can be found in the queue-4.9 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From a45f795c65b479b4ba107b6ccde29b896d51ee98 Mon Sep 17 00:00:00 2001
From: Ilya Dryomov <idryomov@gmail.com>
Date: Fri, 2 Dec 2016 16:35:07 +0100
Subject: libceph: introduce ceph_crypt() for in-place en/decryption

From: Ilya Dryomov <idryomov@gmail.com>

commit a45f795c65b479b4ba107b6ccde29b896d51ee98 upstream.

Starting with 4.9, kernel stacks may be vmalloced and therefore not
guaranteed to be physically contiguous; the new CONFIG_VMAP_STACK
option is enabled by default on x86.  This makes it invalid to use
on-stack buffers with the crypto scatterlist API, as sg_set_buf()
expects a logical address and won't work with vmalloced addresses.

There isn't a different (e.g. kvec-based) crypto API we could switch
net/ceph/crypto.c to and the current scatterlist.h API isn't getting
updated to accommodate this use case.  Allocating a new header and
padding for each operation is a non-starter, so do the en/decryption
in-place on a single pre-assembled (header + data + padding) heap
buffer.  This is explicitly supported by the crypto API:

    "... the caller may provide the same scatter/gather list for the
     plaintext and cipher text. After the completion of the cipher
     operation, the plaintext data is replaced with the ciphertext data
     in case of an encryption and vice versa for a decryption."

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Reviewed-by: Sage Weil <sage@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 net/ceph/crypto.c |   87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/ceph/crypto.h |    2 +
 2 files changed, 89 insertions(+)

--- a/net/ceph/crypto.c
+++ b/net/ceph/crypto.c
@@ -526,6 +526,93 @@ int ceph_encrypt2(struct ceph_crypto_key
 	}
 }
 
+static int ceph_aes_crypt(const struct ceph_crypto_key *key, bool encrypt,
+			  void *buf, int buf_len, int in_len, int *pout_len)
+{
+	struct crypto_skcipher *tfm = ceph_crypto_alloc_cipher();
+	SKCIPHER_REQUEST_ON_STACK(req, tfm);
+	struct sg_table sgt;
+	struct scatterlist prealloc_sg;
+	char iv[AES_BLOCK_SIZE];
+	int pad_byte = AES_BLOCK_SIZE - (in_len & (AES_BLOCK_SIZE - 1));
+	int crypt_len = encrypt ? in_len + pad_byte : in_len;
+	int ret;
+
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
+
+	WARN_ON(crypt_len > buf_len);
+	if (encrypt)
+		memset(buf + in_len, pad_byte, pad_byte);
+	ret = setup_sgtable(&sgt, &prealloc_sg, buf, crypt_len);
+	if (ret)
+		goto out_tfm;
+
+	crypto_skcipher_setkey((void *)tfm, key->key, key->len);
+	memcpy(iv, aes_iv, AES_BLOCK_SIZE);
+
+	skcipher_request_set_tfm(req, tfm);
+	skcipher_request_set_callback(req, 0, NULL, NULL);
+	skcipher_request_set_crypt(req, sgt.sgl, sgt.sgl, crypt_len, iv);
+
+	/*
+	print_hex_dump(KERN_ERR, "key: ", DUMP_PREFIX_NONE, 16, 1,
+		       key->key, key->len, 1);
+	print_hex_dump(KERN_ERR, " in: ", DUMP_PREFIX_NONE, 16, 1,
+		       buf, crypt_len, 1);
+	*/
+	if (encrypt)
+		ret = crypto_skcipher_encrypt(req);
+	else
+		ret = crypto_skcipher_decrypt(req);
+	skcipher_request_zero(req);
+	if (ret) {
+		pr_err("%s %scrypt failed: %d\n", __func__,
+		       encrypt ? "en" : "de", ret);
+		goto out_sgt;
+	}
+	/*
+	print_hex_dump(KERN_ERR, "out: ", DUMP_PREFIX_NONE, 16, 1,
+		       buf, crypt_len, 1);
+	*/
+
+	if (encrypt) {
+		*pout_len = crypt_len;
+	} else {
+		pad_byte = *(char *)(buf + in_len - 1);
+		if (pad_byte > 0 && pad_byte <= AES_BLOCK_SIZE &&
+		    in_len >= pad_byte) {
+			*pout_len = in_len - pad_byte;
+		} else {
+			pr_err("%s got bad padding %d on in_len %d\n",
+			       __func__, pad_byte, in_len);
+			ret = -EPERM;
+			goto out_sgt;
+		}
+	}
+
+out_sgt:
+	teardown_sgtable(&sgt);
+out_tfm:
+	crypto_free_skcipher(tfm);
+	return ret;
+}
+
+int ceph_crypt(const struct ceph_crypto_key *key, bool encrypt,
+	       void *buf, int buf_len, int in_len, int *pout_len)
+{
+	switch (key->type) {
+	case CEPH_CRYPTO_NONE:
+		*pout_len = in_len;
+		return 0;
+	case CEPH_CRYPTO_AES:
+		return ceph_aes_crypt(key, encrypt, buf, buf_len, in_len,
+				      pout_len);
+	default:
+		return -ENOTSUPP;
+	}
+}
+
 static int ceph_key_preparse(struct key_preparsed_payload *prep)
 {
 	struct ceph_crypto_key *ckey;
--- a/net/ceph/crypto.h
+++ b/net/ceph/crypto.h
@@ -43,6 +43,8 @@ int ceph_encrypt2(struct ceph_crypto_key
 		  void *dst, size_t *dst_len,
 		  const void *src1, size_t src1_len,
 		  const void *src2, size_t src2_len);
+int ceph_crypt(const struct ceph_crypto_key *key, bool encrypt,
+	       void *buf, int buf_len, int in_len, int *pout_len);
 int ceph_crypto_init(void);
 void ceph_crypto_shutdown(void);
 


Patches currently in stable-queue which might be from idryomov@gmail.com are

queue-4.9/libceph-switch-ceph_x_decrypt-to-ceph_crypt.patch
queue-4.9/libceph-introduce-ceph_x_encrypt_offset.patch
queue-4.9/libceph-rename-and-align-ceph_x_authorizer-reply_buf.patch
queue-4.9/libceph-ceph_x_encrypt_buflen-takes-in_len.patch
queue-4.9/libceph-introduce-ceph_crypt-for-in-place-en-decryption.patch
queue-4.9/libceph-old_key-in-process_one_ticket-is-redundant.patch
queue-4.9/libceph-tweak-calcu_signature-a-little.patch
queue-4.9/libceph-remove-now-unused-ceph_-en-de-crypt-functions.patch
queue-4.9/ceph-fix-bad-endianness-handling-in-parse_reply_info_extra.patch
queue-4.9/libceph-switch-ceph_x_encrypt-to-ceph_crypt.patch

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch "libceph: introduce ceph_crypt() for in-place en/decryption" has been added to the 4.9-stable tree
  2017-01-20 17:38           ` Greg KH
@ 2017-01-23 15:41             ` Greg KH
  2017-01-23 15:47               ` Ilya Dryomov
  0 siblings, 1 reply; 12+ messages in thread
From: Greg KH @ 2017-01-23 15:41 UTC (permalink / raw)
  To: Ilya Dryomov; +Cc: Sage Weil, spender, stable, stable-commits

On Fri, Jan 20, 2017 at 06:38:18PM +0100, Greg KH wrote:
> On Fri, Jan 20, 2017 at 04:57:37PM +0100, Ilya Dryomov wrote:
> > On Fri, Jan 20, 2017 at 4:34 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > > On Fri, Jan 20, 2017 at 04:26:16PM +0100, Ilya Dryomov wrote:
> > >> On Fri, Jan 20, 2017 at 4:08 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > >> > Ok, what were the surrounding commits?  Don't we need this to handle the
> > >> > vmalloced stack issue in 4.9?  If not, that's fine, I'll drop this,
> > >> > otherwise it would be good to fix that up, right?
> > >>
> > >> Yes, but it's pretty large in terms of diffstat.  Honestly, I'm not
> > >> worried -- our on-stack buffers are really small and the chances that
> > >> any of them will straddle a page should be tiny.
> > >>
> > >> Here is the list (bottom to top):
> > >>
> > >> 2b1e1a7cd0a6 libceph: remove now unused ceph_*{en,de}crypt*() functions
> > >> e15fd0a11db0 libceph: switch ceph_x_decrypt() to ceph_crypt()
> > >> d03857c63bb0 libceph: switch ceph_x_encrypt() to ceph_crypt()
> > >> 4eb4517ce7c9 libceph: tweak calcu_signature() a little
> > >> 7882a26d2e2e libceph: rename and align ceph_x_authorizer::reply_buf
> > >> a45f795c65b4 libceph: introduce ceph_crypt() for in-place en/decryption
> > >> 55d9cc834f93 libceph: introduce ceph_x_encrypt_offset()
> > >> 462e650451c5 libceph: old_key in process_one_ticket() is redundant
> > >> 36721ece1e84 libceph: ceph_x_encrypt_buflen() takes in_len
> > >>
> > >> Can probably drop one or two, but you want to take these, I'd rather
> > >> you take them all.
> > >
> > > Given the length that 4.9 is going to be around, I'd prefer to have this
> > > work correctly.  I'll drop this single patch now, but will queue this
> > > larger list up later when I get a chance to do more testing and review.
> > 
> > Any chance you can also take
> > 
> > 7af3ea189a9a libceph: stop allocating a new cipher on every crypto request
> > 6db2304aabb0 libceph: uninline ceph_crypto_key_destroy()
> > 
> > to make 4.9.z even more awesome? ;)
> > 
> > These depend on the ceph_crypt() bunch and fix a writeback deadlock
> > that has been there forever but started showing up only recently.  The
> > only reason I didn't mark it for stable was this dependency.
> > 
> > All of the above patches were developed and tested on 4.9, so there
> > shouldn't be any issues.
> 
> Sure, I'll work to queue these all up for the next round of kernels,
> thanks for the git commit ids.

Ok, I've queued these all up, along with a few other minor ones I found
when digging through the logs.  If any of them look incorrect, please
let me know.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch "libceph: introduce ceph_crypt() for in-place en/decryption" has been added to the 4.9-stable tree
  2017-01-23 15:41             ` Greg KH
@ 2017-01-23 15:47               ` Ilya Dryomov
  2017-01-23 16:05                 ` Greg KH
  0 siblings, 1 reply; 12+ messages in thread
From: Ilya Dryomov @ 2017-01-23 15:47 UTC (permalink / raw)
  To: Greg KH; +Cc: Sage Weil, spender, stable, stable-commits

On Mon, Jan 23, 2017 at 4:41 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Jan 20, 2017 at 06:38:18PM +0100, Greg KH wrote:
>> On Fri, Jan 20, 2017 at 04:57:37PM +0100, Ilya Dryomov wrote:
>> > On Fri, Jan 20, 2017 at 4:34 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > > On Fri, Jan 20, 2017 at 04:26:16PM +0100, Ilya Dryomov wrote:
>> > >> On Fri, Jan 20, 2017 at 4:08 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > >> > Ok, what were the surrounding commits?  Don't we need this to handle the
>> > >> > vmalloced stack issue in 4.9?  If not, that's fine, I'll drop this,
>> > >> > otherwise it would be good to fix that up, right?
>> > >>
>> > >> Yes, but it's pretty large in terms of diffstat.  Honestly, I'm not
>> > >> worried -- our on-stack buffers are really small and the chances that
>> > >> any of them will straddle a page should be tiny.
>> > >>
>> > >> Here is the list (bottom to top):
>> > >>
>> > >> 2b1e1a7cd0a6 libceph: remove now unused ceph_*{en,de}crypt*() functions
>> > >> e15fd0a11db0 libceph: switch ceph_x_decrypt() to ceph_crypt()
>> > >> d03857c63bb0 libceph: switch ceph_x_encrypt() to ceph_crypt()
>> > >> 4eb4517ce7c9 libceph: tweak calcu_signature() a little
>> > >> 7882a26d2e2e libceph: rename and align ceph_x_authorizer::reply_buf
>> > >> a45f795c65b4 libceph: introduce ceph_crypt() for in-place en/decryption
>> > >> 55d9cc834f93 libceph: introduce ceph_x_encrypt_offset()
>> > >> 462e650451c5 libceph: old_key in process_one_ticket() is redundant
>> > >> 36721ece1e84 libceph: ceph_x_encrypt_buflen() takes in_len
>> > >>
>> > >> Can probably drop one or two, but you want to take these, I'd rather
>> > >> you take them all.
>> > >
>> > > Given the length that 4.9 is going to be around, I'd prefer to have this
>> > > work correctly.  I'll drop this single patch now, but will queue this
>> > > larger list up later when I get a chance to do more testing and review.
>> >
>> > Any chance you can also take
>> >
>> > 7af3ea189a9a libceph: stop allocating a new cipher on every crypto request
>> > 6db2304aabb0 libceph: uninline ceph_crypto_key_destroy()
>> >
>> > to make 4.9.z even more awesome? ;)
>> >
>> > These depend on the ceph_crypt() bunch and fix a writeback deadlock
>> > that has been there forever but started showing up only recently.  The
>> > only reason I didn't mark it for stable was this dependency.
>> >
>> > All of the above patches were developed and tested on 4.9, so there
>> > shouldn't be any issues.
>>
>> Sure, I'll work to queue these all up for the next round of kernels,
>> thanks for the git commit ids.
>
> Ok, I've queued these all up, along with a few other minor ones I found
> when digging through the logs.  If any of them look incorrect, please
> let me know.

All 9 commits from my original list are there and look good.  Not sure
which are those other minor ones you are talking about, though...

Could you also add 6db2304aabb0 and 7af3ea189a9a, as discussed
above?

Thanks,

                Ilya

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch "libceph: introduce ceph_crypt() for in-place en/decryption" has been added to the 4.9-stable tree
  2017-01-23 15:47               ` Ilya Dryomov
@ 2017-01-23 16:05                 ` Greg KH
  0 siblings, 0 replies; 12+ messages in thread
From: Greg KH @ 2017-01-23 16:05 UTC (permalink / raw)
  To: Ilya Dryomov; +Cc: Sage Weil, spender, stable, stable-commits

On Mon, Jan 23, 2017 at 04:47:50PM +0100, Ilya Dryomov wrote:
> On Mon, Jan 23, 2017 at 4:41 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Fri, Jan 20, 2017 at 06:38:18PM +0100, Greg KH wrote:
> >> On Fri, Jan 20, 2017 at 04:57:37PM +0100, Ilya Dryomov wrote:
> >> > On Fri, Jan 20, 2017 at 4:34 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> > > On Fri, Jan 20, 2017 at 04:26:16PM +0100, Ilya Dryomov wrote:
> >> > >> On Fri, Jan 20, 2017 at 4:08 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> > >> > Ok, what were the surrounding commits?  Don't we need this to handle the
> >> > >> > vmalloced stack issue in 4.9?  If not, that's fine, I'll drop this,
> >> > >> > otherwise it would be good to fix that up, right?
> >> > >>
> >> > >> Yes, but it's pretty large in terms of diffstat.  Honestly, I'm not
> >> > >> worried -- our on-stack buffers are really small and the chances that
> >> > >> any of them will straddle a page should be tiny.
> >> > >>
> >> > >> Here is the list (bottom to top):
> >> > >>
> >> > >> 2b1e1a7cd0a6 libceph: remove now unused ceph_*{en,de}crypt*() functions
> >> > >> e15fd0a11db0 libceph: switch ceph_x_decrypt() to ceph_crypt()
> >> > >> d03857c63bb0 libceph: switch ceph_x_encrypt() to ceph_crypt()
> >> > >> 4eb4517ce7c9 libceph: tweak calcu_signature() a little
> >> > >> 7882a26d2e2e libceph: rename and align ceph_x_authorizer::reply_buf
> >> > >> a45f795c65b4 libceph: introduce ceph_crypt() for in-place en/decryption
> >> > >> 55d9cc834f93 libceph: introduce ceph_x_encrypt_offset()
> >> > >> 462e650451c5 libceph: old_key in process_one_ticket() is redundant
> >> > >> 36721ece1e84 libceph: ceph_x_encrypt_buflen() takes in_len
> >> > >>
> >> > >> Can probably drop one or two, but you want to take these, I'd rather
> >> > >> you take them all.
> >> > >
> >> > > Given the length that 4.9 is going to be around, I'd prefer to have this
> >> > > work correctly.  I'll drop this single patch now, but will queue this
> >> > > larger list up later when I get a chance to do more testing and review.
> >> >
> >> > Any chance you can also take
> >> >
> >> > 7af3ea189a9a libceph: stop allocating a new cipher on every crypto request
> >> > 6db2304aabb0 libceph: uninline ceph_crypto_key_destroy()
> >> >
> >> > to make 4.9.z even more awesome? ;)
> >> >
> >> > These depend on the ceph_crypt() bunch and fix a writeback deadlock
> >> > that has been there forever but started showing up only recently.  The
> >> > only reason I didn't mark it for stable was this dependency.
> >> >
> >> > All of the above patches were developed and tested on 4.9, so there
> >> > shouldn't be any issues.
> >>
> >> Sure, I'll work to queue these all up for the next round of kernels,
> >> thanks for the git commit ids.
> >
> > Ok, I've queued these all up, along with a few other minor ones I found
> > when digging through the logs.  If any of them look incorrect, please
> > let me know.
> 
> All 9 commits from my original list are there and look good.  Not sure
> which are those other minor ones you are talking about, though...

They should show up now, took a bit to push out.

> Could you also add 6db2304aabb0 and 7af3ea189a9a, as discussed
> above?

Oops, missed that, now queued up as well, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-01-23 16:05 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-20 14:53 Patch "libceph: introduce ceph_crypt() for in-place en/decryption" has been added to the 4.9-stable tree gregkh
2017-01-20 15:05 ` Ilya Dryomov
2017-01-20 15:08   ` Greg KH
2017-01-20 15:12     ` Greg KH
2017-01-20 15:26     ` Ilya Dryomov
2017-01-20 15:34       ` Greg KH
2017-01-20 15:57         ` Ilya Dryomov
2017-01-20 17:38           ` Greg KH
2017-01-23 15:41             ` Greg KH
2017-01-23 15:47               ` Ilya Dryomov
2017-01-23 16:05                 ` Greg KH
  -- strict thread matches above, loose matches on Subject: below --
2017-01-23 15:35 gregkh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).