On Fri, Jan 20, 2017 at 4:08 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > On Fri, Jan 20, 2017 at 04:05:04PM +0100, Ilya Dryomov wrote: >> On Fri, Jan 20, 2017 at 3:53 PM, <gregkh@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx> know about it. >> > >> > >> > From a45f795c65b479b4ba107b6ccde29b896d51ee98 Mon Sep 17 00:00:00 2001 >> > From: Ilya Dryomov <idryomov@xxxxxxxxx> >> > Date: Fri, 2 Dec 2016 16:35:07 +0100 >> > Subject: libceph: introduce ceph_crypt() for in-place en/decryption >> > >> > From: Ilya Dryomov <idryomov@xxxxxxxxx> >> > >> > 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@xxxxxxxxx> >> > Reviewed-by: Sage Weil <sage@xxxxxxxxxx> >> > Cc: Brad Spengler <spender@xxxxxxxxxxxxxx> >> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> >> > >> > --- >> > 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@xxxxxxxxx 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 -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html