crypto: algif_aead - stop recvmsg looping after a completed request

From e0ed18c8ad9a7d2ecf939f0b97e2be0567180c1d Mon Sep 17 00:00:00 2001
From: Qiguang Wang <[email protected]>
Date: Sat, 27 Jun 2026 21:49:55 +0000
Subject: [PATCH] crypto: algif_aead - stop recvmsg looping after a completed request
To: Herbert Xu <[email protected]>
Cc: "David S. Miller" <[email protected]>, [email protected],
    [email protected]

A blocking recvmsg()/read() into an output buffer larger than the cipher result hangs forever.

After the first pass of the "while (msg_data_left(msg))" loop in aead_recvmsg() (and the identical loop in skcipher_recvmsg()) produces the result, af_alg_get_rsgl() has consumed only as many bytes from the output iterator as the cipher produced, so msg_data_left() is still non-zero and the loop runs a second pass.  By then af_alg_pull_tsgl() has executed

ctx->init = ctx->more;

which, for a request that was not flagged MSG_MORE, resets ctx->init to 0 and drains ctx->used.  The second pass therefore takes the _aead_recvmsg()/_skcipher_recvmsg() gate

if (!ctx->init || ctx->more)
err = af_alg_wait_for_data(sk, flags, 0);

and af_alg_wait_for_data() blocks on

ctx->init && (!ctx->more || (min && ctx->used >= min))

which can never become true again (ctx->init == 0, min == 0), so the task sleeps in MAX_SCHEDULE_TIMEOUT forever even though the result was already produced in pass 1.

The sleep is interruptible, so any signal -- or a poll(POLLIN) issued before the read -- makes recvmsg return the bytes already accumulated in ret, which is why the hang is easy to miss.  A plain blocking read with an oversized buffer hangs deterministically; it reproduces with stock gcm(aes).

Fix both loops by stopping once a non-MSG_MORE request has been fully consumed (ctx->more == 0 && ctx->used == 0) instead of re-entering the blocking wait.  Partial/AIO requests (ctx->used > 0), MSG_MORE streaming (ctx->more != 0) and the -EIOCBQUEUED/-EBADMSG paths are unaffected: the new check is only reached after "ret += err", i.e. after a pass that made forward progress.

---
crypto/algif_aead.c     | 12 ++++++++++++
crypto/algif_skcipher.c | 12 ++++++++++++

2 files changed, 24 insertions(+)

index 787aac8aeb24..d0756aef476d 100644
@@ -216,6 +216,7 @@ static int aead_recvmsg(struct socket *sock, struct msghdr *msg,

size_t ignored, int flags)
{
struct sock *sk = sock->sk;
+ struct af_alg_ctx *ctx = alg_sk(sk)->private;
int ret = 0;

lock_sock(sk);
@@ -237,6 +238,17 @@ static int aead_recvmsg(struct socket *sock, struct msghdr *msg, }

ret += err;
+
+ /*
+ * A request that was not flagged MSG_MORE has now been fully
+ * consumed: af_alg_pull_tsgl() reset ctx->init to ctx->more
+ * (== 0) and drained ctx->used.  Stop here instead of looping
+ * back into a blocking af_alg_wait_for_data() that can never
+ * complete, which is what happens when the supplied output
+ * buffer is larger than the cipher result.
+ */
+ if (!ctx->more && !ctx->used)
+ break;
}

out:

index df20bdfe1f1f..c3a5968baef4 100644
@@ -181,6 +181,7 @@ static int skcipher_recvmsg(struct socket *sock, struct msghdr *msg,

    size_t ignored, int flags)
{
struct sock *sk = sock->sk;
+ struct af_alg_ctx *ctx = alg_sk(sk)->private;
int ret = 0;

lock_sock(sk);
@@ -202,6 +203,17 @@ static int skcipher_recvmsg(struct socket *sock, struct msghdr *msg, }

ret += err;
+
+ /*
+ * A request that was not flagged MSG_MORE has now been fully
+ * consumed: af_alg_pull_tsgl() reset ctx->init to ctx->more
+ * (== 0) and drained ctx->used.  Stop here instead of looping
+ * back into a blocking af_alg_wait_for_data() that can never
+ * complete, which is what happens when the supplied output
+ * buffer is larger than the cipher result.
+ */
+ if (!ctx->more && !ctx->used)
+ break;
}

out:
--
2.53.0

The point of this loop is to wait for a new sendmsg on the socket which is supposed to set ctx->init to true again. So are you saying that even a new sendmsg cannot get out of this wait?

Thanks,

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Thanks -- you're right about the mechanism, and my changelog was imprecise. Let me correct it with what actually happens.

A new sendmsg *does* get out of that particular wait: it sets ctx->init = true and af_alg_data_wakeup() wakes the sleeper, so af_alg_wait_for_data() returns and _aead_recvmsg() processes the new request. What it does *not* do is make the recvmsg() return: the "while (msg_data_left(msg))" loop only stops once the output buffer is full, so after processing the new request it just loops back and blocks again in af_alg_wait_for_data() for the *next* one.

So the blocking read() as a whole never completes until the caller sends enough separate requests to fill the entire output buffer (or a signal arrives). I confirmed this on a live socket with stock gcm(aes), one initial request (48B PT -> 64B result) and a 256-byte read buffer, using a helper thread that sends additional full requests:

  extra sendmsgs | read() returns   | elapsed
  ---------------+------------------+------------------------------
   0             | 64 (watchdog)    | 4.00s  hung
   1             | 128 (watchdog)   | 4.00s  hung  (advanced, re-blocked) 3 (fills 256) | 256              | 1.80s  returned on its own

i.e. one extra sendmsg advanced the result 64 -> 128 and then blocked again; the read() only returned by itself once four requests had filled the 256-byte buffer exactly. (The "elapsed 4.00s" rows returned only because a 4s alarm interrupted the sleep, which makes the loop bail out returning the bytes accumulated so far -- that is also why a signal or strace made my original repro "work".)

So my one-line summary "hangs forever" was wrong; the accurate statement
is: a blocking recvmsg() into a buffer larger than the data the caller intends to send does not return -- it waits to fill the rest of the buffer from further requests that a one-shot caller has no reason to send (it did not set MSG_MORE).

Why I still think this is worth fixing rather than "size your buffer":
it breaks the poll()/read() contract. After a non-MSG_MORE request is consumed, af_alg_poll() reports EPOLLIN:

        if (!ctx->more || ctx->used)
                mask |= EPOLLIN | EPOLLRDNORM;

(!ctx->more is true), so a poll()-driven caller is told the socket is readable, but the subsequent blocking read() then sleeps in af_alg_wait_for_data() instead of returning the data poll() promised. Same test:

        sendmsg = 48
        poll rc=1 revents=0x1 POLLIN=1
        read=64 elapsed=4.00s
        -> poll() said POLLIN but read() blocked

That is what libkcapi/OpenSSL/cryptsetup avoid today only by always sizing the RX buffer exactly to the expected output; anything larger trips it.

On the fix itself: stopping the loop once a non-MSG_MORE request is fully consumed (ctx->more == 0 && ctx->used == 0) turns that case into a normal short read, which is what poll() already advertises. It deliberately does not change:

  - MSG_MORE streaming: ctx->more != 0 -> no break, keeps waiting for the rest of the message;
  - partial / AIO output: _*_recvmsg() leaves ctx->used > 0 -> no break;
  - the -EIOCBQUEUED / -EBADMSG paths: handled by the existing err <= 0 branch before the new check (the check is only reached after "ret += err", i.e. after a pass that made forward progress).

The only behaviour it removes is coalescing *multiple independent* non-MSG_MORE requests into a single oversized read(). That case is not something a caller can reach or rely on deterministically:

  - A single thread cannot even set it up: after one non-MSG_MORE request the next sendmsg hits the "ctx->init && !ctx->more" gate with ctx->used != 0 and fails with -EINVAL (verified). Coalescing is only reachable if a *second* context sends further requests while the first is blocked inside read() draining ctx->used.

  - How many requests get coalesced then depends purely on scheduling (how much of the buffer is filled before the next send lands), so the result length is nondeterministic -- not an API contract anything can depend on.

  - For AEAD it is meaningless anyway: each request has its own tag and independent result, so splitting them across separate reads loses nothing.

With the fix that pattern returns a short read (the first request's output) and the caller reads again -- which is exactly what poll() already tells it to do. No single-threaded, MSG_MORE, or poll()/nonblock-driven caller sees any change.

If you'd prefer, I can respin with the changelog rewritten around the poll()/read() inconsistency (which is the concrete, indefensible part), or take a different approach if you had one in mind. Happy to add a selftest as well.

Reproducers (single-threaded hang, the poll test, and the multi-thread sendmsg test above) are available if useful.

Thanks,
Qiguang

That's the intended behaviour. The recvmsg(2) call on af_alg will not return until the buffer is filled in full. This behaviour goes back to day one (commit 8ff590903d5fc7f5a0a988c38267a3d08e6393a2) and changing it now could break users.

Cheers,

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt