Re: [PATCH v2] crypto: AF_ALG - consolidation of duplicate code
Am Mittwoch, 16. August 2017, 11:37:31 CEST schrieb Herbert Xu: Hi Herbert, > On Wed, Aug 16, 2017 at 10:39:43AM +0200, Stephan Mueller wrote: > > Are you feeding that patch to the Linus 4.13-rc tree via your crypto-2.6 > > tree? > Good point. Why don't you repost that patch and I'll push it > to 4.13 along with stable. Will do. Ciao Stephan
Re: [PATCH v2] crypto: AF_ALG - consolidation of duplicate code
On Wed, Aug 16, 2017 at 10:39:43AM +0200, Stephan Mueller wrote: > > Are you feeding that patch to the Linus 4.13-rc tree via your crypto-2.6 tree? Good point. Why don't you repost that patch and I'll push it to 4.13 along with stable. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2] crypto: AF_ALG - consolidation of duplicate code
Am Mittwoch, 16. August 2017, 10:29:18 CEST schrieb Herbert Xu: Hi Herbert, > On Thu, Aug 10, 2017 at 10:25:58AM +0200, Stephan Mueller wrote: > > I think there is such patch already, see [1]. > > > > Your comment to that patch triggered my rewrite of the memory managment > > code. > > > > [1] https://www.spinics.net/lists/linux-crypto/msg21618.html > > Could you please post that patch to sta...@vger.kernel.org with > the appropriate tags? Sure, will do. Are you feeding that patch to the Linus 4.13-rc tree via your crypto-2.6 tree? Ciao Stephan
Re: [PATCH v2] crypto: AF_ALG - consolidation of duplicate code
On Thu, Aug 10, 2017 at 10:25:58AM +0200, Stephan Mueller wrote: > > I think there is such patch already, see [1]. > > Your comment to that patch triggered my rewrite of the memory managment code. > > [1] https://www.spinics.net/lists/linux-crypto/msg21618.html Could you please post that patch to sta...@vger.kernel.org with the appropriate tags? Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2] crypto: AF_ALG - consolidation of duplicate code
Am Donnerstag, 10. August 2017, 10:21:53 CEST schrieb Herbert Xu: Hi Herbert, > On Thu, Aug 10, 2017 at 10:16:48AM +0200, Stephan Mueller wrote: > > As now the AIO code path is updated, the bug that I was reporting last > > September allowing to crash the kernel via AF_ALG is fixed. > > > > As the patch is very invasive, I am not sure that patch set should be sent > > to stable. How do you propose we fix the crash bug in older kernels that > > are due to memory management problems in the AIO code path? > > Is it possible to create a minimal fix for the stable kernels? I think there is such patch already, see [1]. Your comment to that patch triggered my rewrite of the memory managment code. [1] https://www.spinics.net/lists/linux-crypto/msg21618.html Ciao Stephan
Re: [PATCH v2] crypto: AF_ALG - consolidation of duplicate code
On Thu, Aug 10, 2017 at 10:16:48AM +0200, Stephan Mueller wrote: > > As now the AIO code path is updated, the bug that I was reporting last > September allowing to crash the kernel via AF_ALG is fixed. > > As the patch is very invasive, I am not sure that patch set should be sent to > stable. How do you propose we fix the crash bug in older kernels that are due > to memory management problems in the AIO code path? Is it possible to create a minimal fix for the stable kernels? Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2] crypto: AF_ALG - consolidation of duplicate code
Am Mittwoch, 9. August 2017, 15:57:34 CEST schrieb Herbert Xu: Hi Herbert, > > Patch applied. Thanks. Thanks. As now the AIO code path is updated, the bug that I was reporting last September allowing to crash the kernel via AF_ALG is fixed. As the patch is very invasive, I am not sure that patch set should be sent to stable. How do you propose we fix the crash bug in older kernels that are due to memory management problems in the AIO code path? Ciao Stephan
Re: [PATCH v2] crypto: AF_ALG - consolidation of duplicate code
On Wed, Aug 02, 2017 at 07:56:19AM +0200, Stephan Müller wrote: > Hi Herbert, > > as agreed, the individual patches from the first submission are now changed. > > After review of the changes I had to apply to algif_aead and algif_skcipher, > I saw that they are all in the category that you agreed that can be rolled > into this patch. Though, I documented the changes so that a review should > be easier. > > Ciao > Stephan > > ---8<--- > > Consolidate following data structures: > > skcipher_async_req, aead_async_req -> af_alg_async_req > skcipher_rsgl, aead_rsql -> af_alg_rsgl > skcipher_tsgl, aead_tsql -> af_alg_tsgl > skcipher_ctx, aead_ctx -> af_alg_ctx > > Consolidate following functions: > > skcipher_sndbuf, aead_sndbuf -> af_alg_sndbuf > skcipher_writable, aead_writable -> af_alg_writable > skcipher_rcvbuf, aead_rcvbuf -> af_alg_rcvbuf > skcipher_readable, aead_readable -> af_alg_readable > aead_alloc_tsgl, skcipher_alloc_tsgl -> af_alg_alloc_tsgl > aead_count_tsgl, skcipher_count_tsgl -> af_alg_count_tsgl > aead_pull_tsgl, skcipher_pull_tsgl -> af_alg_pull_tsgl > aead_free_areq_sgls, skcipher_free_areq_sgls -> af_alg_free_areq_sgls > aead_wait_for_wmem, skcipher_wait_for_wmem -> af_alg_wait_for_wmem > aead_wmem_wakeup, skcipher_wmem_wakeup -> af_alg_wmem_wakeup > aead_wait_for_data, skcipher_wait_for_data -> af_alg_wait_for_data > aead_data_wakeup, skcipher_data_wakeup -> af_alg_data_wakeup > aead_sendmsg, skcipher_sendmsg -> af_alg_sendmsg > aead_sendpage, skcipher_sendpage -> af_alg_sendpage > aead_async_cb, skcipher_async_cb -> af_alg_async_cb > aead_poll, skcipher_poll -> af_alg_poll > > Split out the following common code from recvmsg: > > af_alg_alloc_areq: allocation of the request data structure for the > cipher operation > > af_alg_get_rsgl: creation of the RX SGL anchored in the request data > structure > > The following changes to the implementation without affecting the > functionality have been applied to synchronize slightly different code > bases in algif_skcipher and algif_aead: > > The wakeup in af_alg_wait_for_data is triggered when either more data > is received or the indicator that more data is to be expected is > released. The first is triggered by user space, the second is > triggered by the kernel upon finishing the processing of data > (i.e. the kernel is ready for more). > > af_alg_sendmsg uses size_t in min_t calculation for obtaining len. > Return code determination is consistent with algif_skcipher. The > scope of the variable i is reduced to match algif_aead. The type of the > variable i is switched from int to unsigned int to match algif_aead. > > af_alg_sendpage does not contain the superfluous err = 0 from > aead_sendpage. > > af_alg_async_cb requires to store the number of output bytes in > areq->outlen before the AIO callback is triggered. > > The POLLIN / POLLRDNORM is now set when either not more data is given or > the kernel is supplied with data. This is consistent to the wakeup from > sleep when the kernel waits for data. > > The request data structure is extended by the field last_rsgl which > points to the last RX SGL list entry. This shall help recvmsg > implementation to chain the RX SGL to other SG(L)s if needed. It is > currently used by algif_aead which chains the tag SGL to the RX SGL > during decryption. > > Signed-off-by: Stephan Mueller Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[PATCH v2] crypto: AF_ALG - consolidation of duplicate code
Hi Herbert, as agreed, the individual patches from the first submission are now changed. After review of the changes I had to apply to algif_aead and algif_skcipher, I saw that they are all in the category that you agreed that can be rolled into this patch. Though, I documented the changes so that a review should be easier. Ciao Stephan ---8<--- Consolidate following data structures: skcipher_async_req, aead_async_req -> af_alg_async_req skcipher_rsgl, aead_rsql -> af_alg_rsgl skcipher_tsgl, aead_tsql -> af_alg_tsgl skcipher_ctx, aead_ctx -> af_alg_ctx Consolidate following functions: skcipher_sndbuf, aead_sndbuf -> af_alg_sndbuf skcipher_writable, aead_writable -> af_alg_writable skcipher_rcvbuf, aead_rcvbuf -> af_alg_rcvbuf skcipher_readable, aead_readable -> af_alg_readable aead_alloc_tsgl, skcipher_alloc_tsgl -> af_alg_alloc_tsgl aead_count_tsgl, skcipher_count_tsgl -> af_alg_count_tsgl aead_pull_tsgl, skcipher_pull_tsgl -> af_alg_pull_tsgl aead_free_areq_sgls, skcipher_free_areq_sgls -> af_alg_free_areq_sgls aead_wait_for_wmem, skcipher_wait_for_wmem -> af_alg_wait_for_wmem aead_wmem_wakeup, skcipher_wmem_wakeup -> af_alg_wmem_wakeup aead_wait_for_data, skcipher_wait_for_data -> af_alg_wait_for_data aead_data_wakeup, skcipher_data_wakeup -> af_alg_data_wakeup aead_sendmsg, skcipher_sendmsg -> af_alg_sendmsg aead_sendpage, skcipher_sendpage -> af_alg_sendpage aead_async_cb, skcipher_async_cb -> af_alg_async_cb aead_poll, skcipher_poll -> af_alg_poll Split out the following common code from recvmsg: af_alg_alloc_areq: allocation of the request data structure for the cipher operation af_alg_get_rsgl: creation of the RX SGL anchored in the request data structure The following changes to the implementation without affecting the functionality have been applied to synchronize slightly different code bases in algif_skcipher and algif_aead: The wakeup in af_alg_wait_for_data is triggered when either more data is received or the indicator that more data is to be expected is released. The first is triggered by user space, the second is triggered by the kernel upon finishing the processing of data (i.e. the kernel is ready for more). af_alg_sendmsg uses size_t in min_t calculation for obtaining len. Return code determination is consistent with algif_skcipher. The scope of the variable i is reduced to match algif_aead. The type of the variable i is switched from int to unsigned int to match algif_aead. af_alg_sendpage does not contain the superfluous err = 0 from aead_sendpage. af_alg_async_cb requires to store the number of output bytes in areq->outlen before the AIO callback is triggered. The POLLIN / POLLRDNORM is now set when either not more data is given or the kernel is supplied with data. This is consistent to the wakeup from sleep when the kernel waits for data. The request data structure is extended by the field last_rsgl which points to the last RX SGL list entry. This shall help recvmsg implementation to chain the RX SGL to other SG(L)s if needed. It is currently used by algif_aead which chains the tag SGL to the RX SGL during decryption. Signed-off-by: Stephan Mueller --- crypto/af_alg.c | 693 +++ crypto/algif_aead.c | 701 +++- crypto/algif_skcipher.c | 638 +++ include/crypto/if_alg.h | 170 4 files changed, 940 insertions(+), 1262 deletions(-) diff --git a/crypto/af_alg.c b/crypto/af_alg.c index 92a3d540d920..d6936c0e08d9 100644 --- a/crypto/af_alg.c +++ b/crypto/af_alg.c @@ -21,6 +21,7 @@ #include #include #include +#include #include struct alg_type_list { @@ -507,6 +508,698 @@ void af_alg_complete(struct crypto_async_request *req, int err) } EXPORT_SYMBOL_GPL(af_alg_complete); +/** + * af_alg_alloc_tsgl - allocate the TX SGL + * + * @sk socket of connection to user space + * @return: 0 upon success, < 0 upon error + */ +int af_alg_alloc_tsgl(struct sock *sk) +{ + struct alg_sock *ask = alg_sk(sk); + struct af_alg_ctx *ctx = ask->private; + struct af_alg_tsgl *sgl; + struct scatterlist *sg = NULL; + + sgl = list_entry(ctx->tsgl_list.prev, struct af_alg_tsgl, list); + if (!list_empty(&ctx->tsgl_list)) + sg = sgl->sg; + + if (!sg || sgl->cur >= MAX_SGL_ENTS) { + sgl = sock_kmalloc(sk, sizeof(*sgl) + + sizeof(sgl->sg[0]) * (MAX_SGL_ENTS + 1), + GFP_KERNEL); + if (!sgl) + return -ENOMEM; + + sg_init_table(sgl->sg, MAX_SGL_ENTS + 1); + sgl->cur = 0; + + if (sg) + sg_chain(sg, MAX_SGL_ENTS + 1, sgl->sg); + + list_add_tail(&sgl->list, &ctx->tsgl_list); + } + + return 0; +} +EXPORT_SYMBOL_GPL(af_alg_alloc_tsgl); + +/** + * aead_count_tsgl - Count number of