If an AF_ALG socket bound to a hashing algorithm is sent a zero-length message with MSG_MORE set and then recvmsg() is called without first sending another message without MSG_MORE set to end the operation, an oops will occur because the crypto context and result doesn't now get set up in advance because hash_sendmsg() now defers that as long as possible in the hope that it can use crypto_ahash_digest() - and then because the message is zero-length, it the data wrangling loop is skipped. Fix this by always making a pass of the loop, even in the case that no data is provided to the sendmsg(). Fix also extract_iter_to_sg() to handle a zero-length iterator by returning 0 immediately. Whilst we're at it, remove the code to create a kvmalloc'd scatterlist if we get more than ALG_MAX_PAGES - this shouldn't happen. Fixes: c662b043cdca ("crypto: af_alg/hash: Support MSG_SPLICE_PAGES") Reported-by: syzbot+13a08c0bf4d212766c3c@xxxxxxxxxxxxxxxxxxxxxxxxx Link: https://lore.kernel.org/r/000000000000b928f705fdeb873a@xxxxxxxxxx/ Reported-by: syzbot+14234ccf6d0ef629ec1a@xxxxxxxxxxxxxxxxxxxxxxxxx Link: https://lore.kernel.org/r/000000000000c047db05fdeb8790@xxxxxxxxxx/ Reported-by: syzbot+4e2e47f32607d0f72d43@xxxxxxxxxxxxxxxxxxxxxxxxx Link: https://lore.kernel.org/r/000000000000bcca3205fdeb87fb@xxxxxxxxxx/ Reported-by: syzbot+472626bb5e7c59fb768f@xxxxxxxxxxxxxxxxxxxxxxxxx Link: https://lore.kernel.org/r/000000000000b55d8805fdeb8385@xxxxxxxxxx/ Signed-off-by: David Howells <dhowells@xxxxxxxxxx> Tested-by: syzbot+13a08c0bf4d212766c3c@xxxxxxxxxxxxxxxxxxxxxxxxx Tested-by: syzbot+14234ccf6d0ef629ec1a@xxxxxxxxxxxxxxxxxxxxxxxxx Tested-by: syzbot+4e2e47f32607d0f72d43@xxxxxxxxxxxxxxxxxxxxxxxxx Tested-by: syzbot+472626bb5e7c59fb768f@xxxxxxxxxxxxxxxxxxxxxxxxx cc: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> cc: "David S. Miller" <davem@xxxxxxxxxxxxx> cc: Eric Dumazet <edumazet@xxxxxxxxxx> cc: Jakub Kicinski <kuba@xxxxxxxxxx> cc: Paolo Abeni <pabeni@xxxxxxxxxx> cc: Jens Axboe <axboe@xxxxxxxxx> cc: Matthew Wilcox <willy@xxxxxxxxxxxxx> cc: linux-crypto@xxxxxxxxxxxxxxx cc: netdev@xxxxxxxxxxxxxxx --- crypto/algif_hash.c | 21 +++++---------------- lib/scatterlist.c | 2 +- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c index dfb048cefb60..1176533a55c9 100644 --- a/crypto/algif_hash.c +++ b/crypto/algif_hash.c @@ -83,26 +83,14 @@ static int hash_sendmsg(struct socket *sock, struct msghdr *msg, ctx->more = false; - while (msg_data_left(msg)) { + do { ctx->sgl.sgt.sgl = ctx->sgl.sgl; ctx->sgl.sgt.nents = 0; ctx->sgl.sgt.orig_nents = 0; err = -EIO; npages = iov_iter_npages(&msg->msg_iter, max_pages); - if (npages == 0) - goto unlock_free; - - if (npages > ARRAY_SIZE(ctx->sgl.sgl)) { - err = -ENOMEM; - ctx->sgl.sgt.sgl = - kvmalloc(array_size(npages, - sizeof(*ctx->sgl.sgt.sgl)), - GFP_KERNEL); - if (!ctx->sgl.sgt.sgl) - goto unlock_free; - } - sg_init_table(ctx->sgl.sgl, npages); + sg_init_table(ctx->sgl.sgl, max_t(size_t, npages, 1)); ctx->sgl.need_unpin = iov_iter_extract_will_pin(&msg->msg_iter); @@ -111,7 +99,8 @@ static int hash_sendmsg(struct socket *sock, struct msghdr *msg, if (err < 0) goto unlock_free; len = err; - sg_mark_end(ctx->sgl.sgt.sgl + ctx->sgl.sgt.nents - 1); + if (len > 0) + sg_mark_end(ctx->sgl.sgt.sgl + ctx->sgl.sgt.nents - 1); if (!msg_data_left(msg)) { err = hash_alloc_result(sk, ctx); @@ -148,7 +137,7 @@ static int hash_sendmsg(struct socket *sock, struct msghdr *msg, copied += len; af_alg_free_sg(&ctx->sgl); - } + } while (msg_data_left(msg)); ctx->more = msg->msg_flags & MSG_MORE; err = 0; diff --git a/lib/scatterlist.c b/lib/scatterlist.c index e97d7060329e..77a7b18ee751 100644 --- a/lib/scatterlist.c +++ b/lib/scatterlist.c @@ -1340,7 +1340,7 @@ ssize_t extract_iter_to_sg(struct iov_iter *iter, size_t maxsize, struct sg_table *sgtable, unsigned int sg_max, iov_iter_extraction_t extraction_flags) { - if (maxsize == 0) + if (!maxsize || !iter->count) return 0; switch (iov_iter_type(iter)) {