Hi Herbert, I am sorry to interrupt your merge window, but may I ask to consider this patch for the current development cycle as well as for stable back to v4.1 where the algif_skcipher AIO support was added? It fixes the two bug reports which I reported back in September that allow crashing the kernel from user space as an unprivileged user. I think that this patch now fixes the real issue and not just papers things over. The fix can be validated using the following invocation from [1]. test/kcapi -d 2 -x 9 -e -c "cbc(aes)" -k 8d7dd9b0170ce0b5f2f8e1aa768e01e91da8bfc67fd486d081b28254c99eb423 -i 7fbc02ebf5b93322329df9bfccb635af -p 48981da18e4bb9ef7e2e3162d16b1910 Without the patch, the kernel crashes. With the patch, the kernel works. The test duplicates the plaintext for supplying two IOCBs, expecting the two identical blocks of ciphertext. When changing the test such that both input data blocks are different, the resulting cipher text blocks are different, as expected. [1] http://www.chronox.de/libkcapi.html ---8<--- When submitting multiple IOCBs to be processed with one AIO invocation, the initially supplied input data is processed with with each AIO operation. For example, a simplified AIO operation may look like the following: 1. sendmsg(32 bytes) 2. io_submit which defines 2 IOCBs (i.e. 2 operations providing 16 bytes buffer each to invoke an ecb(aes) operation) The io_submit call is processed by the skcipher_recvmsg_async AF_ALG handler. io_submit invokes skcipher_recvmsg_async once for each IOCB. skcipher_recvmsg_async processes the ecb(aes) operation request, taking the first 16 bytes from the input. When finishing the skcipher_recvmsg_async operation, the page holding the 32 bytes of input data from sendmsg cannot be released yet, but the scatter/gather list pointing into the page needs to be advanced to point to the second 16 bytes. Only when all data is used up, the page is released. Signed-off-by: Stephan Mueller <smueller@xxxxxxxxxx> --- crypto/algif_skcipher.c | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c index 1e38aaa..68bde92 100644 --- a/crypto/algif_skcipher.c +++ b/crypto/algif_skcipher.c @@ -72,7 +72,8 @@ struct skcipher_async_req { #define MAX_SGL_ENTS ((4096 - sizeof(struct skcipher_sg_list)) / \ sizeof(struct scatterlist) - 1) -static void skcipher_free_async_sgls(struct skcipher_async_req *sreq) +static void skcipher_free_async_sgls(struct skcipher_async_req *sreq, + unsigned int len) { struct skcipher_async_rsgl *rsgl, *tmp; struct scatterlist *sgl; @@ -86,8 +87,31 @@ static void skcipher_free_async_sgls(struct skcipher_async_req *sreq) } sgl = sreq->tsg; n = sg_nents(sgl); - for_each_sg(sgl, sg, n, i) - put_page(sg_page(sg)); + for_each_sg(sgl, sg, n, i) { + struct page *page = sg_page(sg); + + if (!page) + continue; + + /* + * The async operation may have processed only a subset of + * the data that was initially received from the caller. + * Thus, we only can release the data that a cipher operation + * processed. + */ + if (len < sg->length) { + /* ensure that empty SGLs are not referenced any more */ + sreq->tsg = sg; + + /* advance the buffers to the unprocessed data */ + sg->length -= len; + sg->offset += len; + return; + } + + len -= sg->length; + put_page(page); + } kfree(sreq->tsg); } @@ -95,10 +119,11 @@ static void skcipher_free_async_sgls(struct skcipher_async_req *sreq) static void skcipher_async_cb(struct crypto_async_request *req, int err) { struct skcipher_async_req *sreq = req->data; + struct skcipher_request *sk_req = &sreq->req; struct kiocb *iocb = sreq->iocb; atomic_dec(sreq->inflight); - skcipher_free_async_sgls(sreq); + skcipher_free_async_sgls(sreq, err ? 0 : sk_req->cryptlen); kzfree(sreq); iocb->ki_complete(iocb, err, err); } @@ -623,7 +648,7 @@ static int skcipher_recvmsg_async(struct socket *sock, struct msghdr *msg, goto unlock; } free: - skcipher_free_async_sgls(sreq); + skcipher_free_async_sgls(sreq, err ? 0 : len); unlock: skcipher_wmem_wakeup(sk); release_sock(sk); -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
