crypto: algif_skcipher - force synchronous processing

The AIO/async path in skcipher_recvmsg() passes the socket-wide ctx->iv
directly into the skcipher request. After io_submit() the socket lock is
dropped and the request is processed asynchronously by a worker (e.g.
cryptd), which dereferences ctx->iv only later.

A concurrent sendmsg(ALG_SET_IV) on the same socket can overwrite ctx->iv
inside this window, so the in-flight request runs under an
attacker-controlled IV. For CTR and other stream modes this causes
IV/keystream reuse and allows an unprivileged user to recover the
plaintext of a concurrent operation.

Snapshotting ctx->iv into per-request storage for the async path is not
sufficient here. For ciphers with statesize == 0 - which includes cbc
and ctr - skcipher_prepare_alg() installs skcipher_noimport()/
skcipher_noexport(), so ctx->state carries nothing and the MSG_MORE
inter-chunk IV chaining is carried solely by the in-place req->iv
writeback. A snapshot redirects that writeback into per-request memory
that af_alg_free_resources() releases on completion, so AIO + MSG_MORE
with cbc/ctr would silently produce wrong output. Writing the IV back
from the completion callback instead is not possible either: that would
require lock_sock() there, but the callback can run in softirq/atomic
context, so it must not sleep.

Make the operation synchronous instead. ctx->iv is then only ever
dereferenced under the socket lock held by recvmsg(), which removes the
race, and the req->iv writeback lands in ctx->iv as before, which keeps
MSG_MORE chaining intact for statesize == 0 ciphers. The ctx->state
import/export path is unchanged for ciphers that do have state.

This is equivalent to the upstream resolution: commit fcc77d33a3
("net: Remove support for AIO on sockets") removed the AIO socket path
across net/ entirely, producing the same end state for this file -
algif_skcipher never processes an AIO request asynchronously. After this
patch, _skcipher_recvmsg() matches mainline's crypto/algif_skcipher.c as
it stands today, including the same now-dead -EIOCBQUEUED check. This
patch deviates from that commit deliberately: rather than removing AIO
socket support tree-wide, which would be far too invasive for stable, it
removes only the AIO branch in crypto/algif_skcipher.c. io_submit() now
completes synchronously, which is valid for the AIO interface; AF_ALG
async is rarely used in practice.

The -EIOCBQUEUED check in skcipher_recvmsg() is now dead but harmless,
and is left alone to keep the fix minimal.

Fixes: e870456d8e ("crypto: algif_skcipher - overhaul memory management")
Cc: <stable@vger.kernel.org>
Reported-by: Muhammet Kaan KILINÇ <muhammetkaankilinc@gmail.com>
Signed-off-by: Muhammet Kaan KILINÇ <muhammetkaankilinc@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
Muhammet Kaan KILINÇ
2026-07-24 16:15:39 +02:00
committed by Greg Kroah-Hartman
parent de5a46f3b2
commit d7860b682d
+24 -51
View File
@@ -79,20 +79,6 @@ static int algif_skcipher_export(struct sock *sk, struct skcipher_request *req)
return err;
}
static void algif_skcipher_done(void *data, int err)
{
struct af_alg_async_req *areq = data;
struct sock *sk = areq->sk;
if (err)
goto out;
err = algif_skcipher_export(sk, &areq->cra_u.skcipher_req);
out:
af_alg_async_cb(data, err);
}
static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
size_t ignored, int flags)
{
@@ -171,43 +157,30 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
cflags |= CRYPTO_SKCIPHER_REQ_CONT;
}
if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) {
/* AIO operation */
sock_hold(sk);
areq->iocb = msg->msg_iocb;
/* Remember output size that will be generated. */
areq->outlen = len;
skcipher_request_set_callback(&areq->cra_u.skcipher_req,
cflags |
CRYPTO_TFM_REQ_MAY_SLEEP,
algif_skcipher_done, areq);
err = ctx->enc ?
crypto_skcipher_encrypt(&areq->cra_u.skcipher_req) :
crypto_skcipher_decrypt(&areq->cra_u.skcipher_req);
/* AIO operation in progress */
if (err == -EINPROGRESS)
return -EIOCBQUEUED;
sock_put(sk);
} else {
/* Synchronous operation */
skcipher_request_set_callback(&areq->cra_u.skcipher_req,
cflags |
CRYPTO_TFM_REQ_MAY_SLEEP |
CRYPTO_TFM_REQ_MAY_BACKLOG,
crypto_req_done, &ctx->wait);
err = crypto_wait_req(ctx->enc ?
crypto_skcipher_encrypt(&areq->cra_u.skcipher_req) :
crypto_skcipher_decrypt(&areq->cra_u.skcipher_req),
&ctx->wait);
if (!err)
err = algif_skcipher_export(
sk, &areq->cra_u.skcipher_req);
}
/*
* Force synchronous processing. The async (AIO) path passed the
* socket-wide ctx->iv into the request, which the worker
* dereferenced after the socket lock had been dropped, letting a
* concurrent sendmsg(ALG_SET_IV) inject an attacker IV. Mainline
* removed the AIO socket path in commit fcc77d33a34c ("net: Remove
* support for AIO on sockets"); the minimal stable fix is to always
* complete synchronously, so ctx->iv is only ever dereferenced under
* the socket lock. This also keeps the IV chaining intact: for
* ciphers with statesize == 0 (e.g. ctr, cbc) the chained IV is
* carried by the req->iv writeback into ctx->iv, which is only
* consistent on the synchronous path.
*/
skcipher_request_set_callback(&areq->cra_u.skcipher_req,
cflags |
CRYPTO_TFM_REQ_MAY_SLEEP |
CRYPTO_TFM_REQ_MAY_BACKLOG,
crypto_req_done, &ctx->wait);
err = crypto_wait_req(ctx->enc ?
crypto_skcipher_encrypt(&areq->cra_u.skcipher_req) :
crypto_skcipher_decrypt(&areq->cra_u.skcipher_req),
&ctx->wait);
if (!err)
err = algif_skcipher_export(sk, &areq->cra_u.skcipher_req);
free:
af_alg_free_resources(areq);