Commit b6d972f6 authored Jun 16, 2023 by David Howells Committed by Jakub Kicinski Jun 20, 2023

crypto: af_alg/hash: Fix recvmsg() after sendmsg(MSG_MORE)

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 handling zero-length sends at the top of the hash_sendmsg()
function. If we're not continuing the previous sendmsg(), then just ignore
the send (hash_recvmsg() will invent something when called); if we are
continuing, then we finalise the request at this point if MSG_MORE is not
set to get any error here, otherwise the send is of no effect and can be
ignored.

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: c662b043 ("crypto: af_alg/hash: Support MSG_SPLICE_PAGES")
Reported-by: <syzbot+13a08c0bf4d212766c3c@syzkaller.appspotmail.com>
Link: https://lore.kernel.org/r/000000000000b928f705fdeb873a@google.com/

Reported-by: <syzbot+14234ccf6d0ef629ec1a@syzkaller.appspotmail.com>
Link: https://lore.kernel.org/r/000000000000c047db05fdeb8790@google.com/

Reported-by: <syzbot+4e2e47f32607d0f72d43@syzkaller.appspotmail.com>
Link: https://lore.kernel.org/r/000000000000bcca3205fdeb87fb@google.com/

Reported-by: <syzbot+472626bb5e7c59fb768f@syzkaller.appspotmail.com>
Link: https://lore.kernel.org/r/000000000000b55d8805fdeb8385@google.com/

Signed-off-by: David Howells <dhowells@redhat.com>
Reported-and-tested-by: <syzbot+6efc50cc1f8d718d6cb7@syzkaller.appspotmail.com>
cc: Jens Axboe <axboe@kernel.dk>
cc: Matthew Wilcox <willy@infradead.org>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Link: https://lore.kernel.org/r/427646.1686913832@warthog.procyon.org.uk

Signed-off-by: Jakub Kicinski <kuba@kernel.org>

parent 462a3daa

crypto/algif_hash.c

+25 −13

Original line number	Diff line number	Diff line
		@@ -76,12 +76,29 @@ static int hash_sendmsg(struct socket sock, struct msghdr msg,

		lock_sock(sk);
		if (!continuing) {
		if ((msg->msg_flags & MSG_MORE))
		/* Discard a previous request that wasn't marked MSG_MORE. */
		hash_free_result(sk, ctx);
		if (!msg_data_left(msg))
		goto done; /* Zero-length; don't start new req */
		need_init = true;
		} else if (!msg_data_left(msg)) {
		/*
		* No data - finalise the prev req if MSG_MORE so any error
		* comes out here.
		*/
		if (!(msg->msg_flags & MSG_MORE)) {
		err = hash_alloc_result(sk, ctx);
		if (err)
		goto unlock_free;
		ahash_request_set_crypt(&ctx->req, NULL,
		ctx->result, 0);
		err = crypto_wait_req(crypto_ahash_final(&ctx->req),
		&ctx->wait);
		if (err)
		goto unlock_free;
		}
		goto done_more;
		}

		ctx->more = false;

		while (msg_data_left(msg)) {
		ctx->sgl.sgt.sgl = ctx->sgl.sgl;
		@@ -93,15 +110,6 @@ static int hash_sendmsg(struct socket sock, struct msghdr msg,
		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);

		ctx->sgl.need_unpin = iov_iter_extract_will_pin(&msg->msg_iter);
		@@ -150,7 +158,9 @@ static int hash_sendmsg(struct socket sock, struct msghdr msg,
		af_alg_free_sg(&ctx->sgl);
		}

		done_more:
		ctx->more = msg->msg_flags & MSG_MORE;
		done:
		err = 0;
		unlock:
		release_sock(sk);
		@@ -158,6 +168,8 @@ static int hash_sendmsg(struct socket sock, struct msghdr msg,

		unlock_free:
		af_alg_free_sg(&ctx->sgl);
		hash_free_result(sk, ctx);
		ctx->more = false;
		goto unlock;
		}