[PATCH] crypto: algif_aead - stop recvmsg looping after a completed request

[PATCH] crypto: algif_aead - stop recvmsg looping after a completed request

[cyper]

From e0ed18c8ad9a7d2ecf939f0b97e2be0567180c1d Mon Sep 17 00:00:00 2001
From: Qiguang Wang <cyper@tutanota.com>
Date: Sat, 27 Jun 2026 21:49:55 +0000
Subject: [PATCH] crypto: algif_aead - stop recvmsg looping after a completed
request
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>,
    linux-crypto@vger.kernel.org,
    linux-kernel@vger.kernel.org

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.

Fixes: f3c802a1f300 ("crypto: algif_aead - Only wake up when ctx->more is zero")
Cc: <stable@vger.kernel.org>
Signed-off-by: Qiguang Wang <cyper@tutanota.com>
---
crypto/algif_aead.c     | 12 ++++++++++++
crypto/algif_skcipher.c | 12 ++++++++++++
2 files changed, 24 insertions(+)

diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 787aac8aeb24..d0756aef476d 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -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:
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index df20bdfe1f1f..c3a5968baef4 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -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

Re: [PATCH] crypto: algif_aead - stop recvmsg looping after a completed request

[Herbert Xu]

On Sun, Jun 28, 2026 at 05:34:15PM +0200, cyper@tutanota.com wrote:
> >From e0ed18c8ad9a7d2ecf939f0b97e2be0567180c1d Mon Sep 17 00:00:00 2001
> From: Qiguang Wang <cyper@tutanota.com>
> Date: Sat, 27 Jun 2026 21:49:55 +0000
> Subject: [PATCH] crypto: algif_aead - stop recvmsg looping after a completed
> request
> To: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>,
>     linux-crypto@vger.kernel.org,
>     linux-kernel@vger.kernel.org
> 
> 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 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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: algif_aead - stop recvmsg looping after a completed request

[cyper]

> 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 -- 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

Re: [PATCH] crypto: algif_aead - stop recvmsg looping after a completed request

[Herbert Xu]

On Mon, Jul 13, 2026 at 06:02:42PM +0200, cyper@tutanota.com wrote:
>
> 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.

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt