* [PATCH] crypto: krb5enc - fix async decrypt skipping hash verification
@ 2026-04-16 13:54 Dudu Lu
2026-04-17 8:48 ` Herbert Xu
0 siblings, 1 reply; 4+ messages in thread
From: Dudu Lu @ 2026-04-16 13:54 UTC (permalink / raw)
To: linux-crypto; +Cc: herbert, Dudu Lu
krb5enc_dispatch_decrypt() sets req->base.complete as the skcipher
callback, which is the caller's own completion handler. When the
skcipher completes asynchronously, this signals "done" to the caller
without executing krb5enc_dispatch_decrypt_hash(), completely bypassing
the integrity verification (hash check).
Compare with the encrypt path which correctly uses
krb5enc_encrypt_done as an intermediate callback to chain into the
hash computation on async completion.
Fix by adding krb5enc_decrypt_done as an intermediate callback that
chains into krb5enc_dispatch_decrypt_hash() upon async skcipher
completion, matching the encrypt path's callback pattern. Handle
both -EINPROGRESS and -EBUSY notifications from backlogged requests,
consistent with authenc's authenc_request_complete(). Also fix
krb5enc_request_complete() to filter -EBUSY in addition to
-EINPROGRESS, matching the authenc reference implementation.
Fixes: d1775a177f7f ("crypto: Add 'krb5enc' hash and cipher AEAD algorithm")
Signed-off-by: Dudu Lu <phx0fer@gmail.com>
---
crypto/krb5enc.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/crypto/krb5enc.c b/crypto/krb5enc.c
index a1de55994d92..2490343873a9 100644
--- a/crypto/krb5enc.c
+++ b/crypto/krb5enc.c
@@ -41,7 +41,7 @@ struct krb5enc_request_ctx {
static void krb5enc_request_complete(struct aead_request *req, int err)
{
- if (err != -EINPROGRESS)
+ if (err != -EINPROGRESS && err != -EBUSY)
aead_request_complete(req, err);
}
@@ -300,6 +300,24 @@ static int krb5enc_dispatch_decrypt_hash(struct aead_request *req)
return krb5enc_verify_hash(req);
}
+static void krb5enc_decrypt_done(void *data, int err)
+{
+ struct aead_request *req = data;
+
+ if (err == -EINPROGRESS || err == -EBUSY)
+ return krb5enc_request_complete(req, err);
+
+ if (err)
+ goto out;
+
+ err = krb5enc_dispatch_decrypt_hash(req);
+ if (err == -EINPROGRESS || err == -EBUSY)
+ return;
+
+out:
+ aead_request_complete(req, err);
+}
+
/*
* Dispatch the decryption of the ciphertext.
*/
@@ -323,7 +341,7 @@ static int krb5enc_dispatch_decrypt(struct aead_request *req)
skcipher_request_set_tfm(skreq, ctx->enc);
skcipher_request_set_callback(skreq, aead_request_flags(req),
- req->base.complete, req->base.data);
+ krb5enc_decrypt_done, req);
skcipher_request_set_crypt(skreq, src, dst,
req->cryptlen - authsize, req->iv);
--
2.39.3 (Apple Git-145)
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] crypto: krb5enc - fix async decrypt skipping hash verification
2026-04-16 13:54 Dudu Lu
@ 2026-04-17 8:48 ` Herbert Xu
0 siblings, 0 replies; 4+ messages in thread
From: Herbert Xu @ 2026-04-17 8:48 UTC (permalink / raw)
To: Dudu Lu; +Cc: linux-crypto
On Thu, Apr 16, 2026 at 09:54:24PM +0800, Dudu Lu wrote:
>
> diff --git a/crypto/krb5enc.c b/crypto/krb5enc.c
> index a1de55994d92..2490343873a9 100644
> --- a/crypto/krb5enc.c
> +++ b/crypto/krb5enc.c
> @@ -41,7 +41,7 @@ struct krb5enc_request_ctx {
>
> static void krb5enc_request_complete(struct aead_request *req, int err)
> {
> - if (err != -EINPROGRESS)
> + if (err != -EINPROGRESS && err != -EBUSY)
This shouldn't filter anything out. The filtering needs to occur
further up the call stack. In fact just get rid of it and use
aead_request_complete directly.
The encrypt path is just as broken as the decrypt path and
needs to be fixed accordingly to filter out EBUSY/EINPROGRESS.
In particular, this should be done in krb5enc_encrypt_ahash_done.
Currently it's only filtering out EINPROGRESS.
> +static void krb5enc_decrypt_done(void *data, int err)
> +{
> + struct aead_request *req = data;
> +
> + if (err == -EINPROGRESS || err == -EBUSY)
> + return krb5enc_request_complete(req, err);
EINPROGRESS should always get passed up here because it means
that we originally returned an EBUSY and the caller is potentially
blocking on this notification.
EBUSY cannot occur in the context of the callback so there is no
need to check for it.
So this should simply become
if (err)
goto out;
> + err = krb5enc_dispatch_decrypt_hash(req);
> + if (err == -EINPROGRESS || err == -EBUSY)
> + return;
This is the only place where EBUSY needs to be checked.
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] crypto: krb5enc - fix async decrypt skipping hash verification
@ 2026-04-20 4:40 Dudu Lu
2026-04-20 8:13 ` Herbert Xu
0 siblings, 1 reply; 4+ messages in thread
From: Dudu Lu @ 2026-04-20 4:40 UTC (permalink / raw)
To: linux-crypto; +Cc: herbert, davem, Dudu Lu
krb5enc_dispatch_decrypt() sets req->base.complete as the skcipher
callback, which is the caller's own completion handler. When the
skcipher completes asynchronously, this signals "done" to the caller
without executing krb5enc_dispatch_decrypt_hash(), completely bypassing
the integrity verification (hash check).
Compare with the encrypt path which correctly uses
krb5enc_encrypt_done as an intermediate callback to chain into the
hash computation on async completion.
Fix by adding krb5enc_decrypt_done as an intermediate callback that
chains into krb5enc_dispatch_decrypt_hash() upon async skcipher
completion, matching the encrypt path's callback pattern.
Also fix EBUSY/EINPROGRESS handling throughout: remove
krb5enc_request_complete() which incorrectly swallowed EINPROGRESS
notifications that must be passed up to callers waiting on backlogged
requests, and add missing EBUSY checks in krb5enc_encrypt_ahash_done
for the dispatch_encrypt return value.
Fixes: d1775a177f7f ("crypto: Add 'krb5enc' hash and cipher AEAD algorithm")
Signed-off-by: Dudu Lu <phx0fer@gmail.com>
---
crypto/krb5enc.c | 42 ++++++++++++++++++++++++++----------------
1 file changed, 26 insertions(+), 16 deletions(-)
diff --git a/crypto/krb5enc.c b/crypto/krb5enc.c
index a1de55994d92..2df7d4c3baed 100644
--- a/crypto/krb5enc.c
+++ b/crypto/krb5enc.c
@@ -39,12 +39,6 @@ struct krb5enc_request_ctx {
char tail[];
};
-static void krb5enc_request_complete(struct aead_request *req, int err)
-{
- if (err != -EINPROGRESS)
- aead_request_complete(req, err);
-}
-
/**
* crypto_krb5enc_extractkeys - Extract Ke and Ki keys from the key blob.
* @keys: Where to put the key sizes and pointers
@@ -127,7 +121,7 @@ static void krb5enc_encrypt_done(void *data, int err)
{
struct aead_request *req = data;
- krb5enc_request_complete(req, err);
+ aead_request_complete(req, err);
}
/*
@@ -188,13 +182,16 @@ static void krb5enc_encrypt_ahash_done(void *data, int err)
struct ahash_request *ahreq = (void *)(areq_ctx->tail + ictx->reqoff);
if (err)
- return krb5enc_request_complete(req, err);
+ goto out;
krb5enc_insert_checksum(req, ahreq->result);
err = krb5enc_dispatch_encrypt(req, 0);
- if (err != -EINPROGRESS)
- aead_request_complete(req, err);
+ if (err == -EINPROGRESS || err == -EBUSY)
+ return;
+
+out:
+ aead_request_complete(req, err);
}
/*
@@ -264,11 +261,9 @@ static void krb5enc_decrypt_hash_done(void *data, int err)
{
struct aead_request *req = data;
- if (err)
- return krb5enc_request_complete(req, err);
-
- err = krb5enc_verify_hash(req);
- krb5enc_request_complete(req, err);
+ if (!err)
+ err = krb5enc_verify_hash(req);
+ aead_request_complete(req, err);
}
/*
@@ -300,6 +295,21 @@ static int krb5enc_dispatch_decrypt_hash(struct aead_request *req)
return krb5enc_verify_hash(req);
}
+static void krb5enc_decrypt_done(void *data, int err)
+{
+ struct aead_request *req = data;
+
+ if (err)
+ goto out;
+
+ err = krb5enc_dispatch_decrypt_hash(req);
+ if (err == -EINPROGRESS || err == -EBUSY)
+ return;
+
+out:
+ aead_request_complete(req, err);
+}
+
/*
* Dispatch the decryption of the ciphertext.
*/
@@ -323,7 +333,7 @@ static int krb5enc_dispatch_decrypt(struct aead_request *req)
skcipher_request_set_tfm(skreq, ctx->enc);
skcipher_request_set_callback(skreq, aead_request_flags(req),
- req->base.complete, req->base.data);
+ krb5enc_decrypt_done, req);
skcipher_request_set_crypt(skreq, src, dst,
req->cryptlen - authsize, req->iv);
--
2.39.3 (Apple Git-145)
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] crypto: krb5enc - fix async decrypt skipping hash verification
2026-04-20 4:40 [PATCH] crypto: krb5enc - fix async decrypt skipping hash verification Dudu Lu
@ 2026-04-20 8:13 ` Herbert Xu
0 siblings, 0 replies; 4+ messages in thread
From: Herbert Xu @ 2026-04-20 8:13 UTC (permalink / raw)
To: Dudu Lu; +Cc: linux-crypto, davem, David Howells
On Mon, Apr 20, 2026 at 12:40:27PM +0800, Dudu Lu wrote:
> krb5enc_dispatch_decrypt() sets req->base.complete as the skcipher
> callback, which is the caller's own completion handler. When the
> skcipher completes asynchronously, this signals "done" to the caller
> without executing krb5enc_dispatch_decrypt_hash(), completely bypassing
> the integrity verification (hash check).
>
> Compare with the encrypt path which correctly uses
> krb5enc_encrypt_done as an intermediate callback to chain into the
> hash computation on async completion.
>
> Fix by adding krb5enc_decrypt_done as an intermediate callback that
> chains into krb5enc_dispatch_decrypt_hash() upon async skcipher
> completion, matching the encrypt path's callback pattern.
>
> Also fix EBUSY/EINPROGRESS handling throughout: remove
> krb5enc_request_complete() which incorrectly swallowed EINPROGRESS
> notifications that must be passed up to callers waiting on backlogged
> requests, and add missing EBUSY checks in krb5enc_encrypt_ahash_done
> for the dispatch_encrypt return value.
>
> Fixes: d1775a177f7f ("crypto: Add 'krb5enc' hash and cipher AEAD algorithm")
> Signed-off-by: Dudu Lu <phx0fer@gmail.com>
> ---
> crypto/krb5enc.c | 42 ++++++++++++++++++++++++++----------------
> 1 file changed, 26 insertions(+), 16 deletions(-)
Thanks. Your patch looks good to me.
However, while applying it against mainline I got a conflict because
of the recent change to unset the MAY_SLEEP flag on the async completion
path.
That got me thinking and I found one more issue, if we try to do a
MAY_BACKLOG operation on the async completion path, then the user
will see two back-to-back EINPROGRESS notifications which is wrong.
As David still hasn't indicated his preference to whether MAY_BACKLOG
should be supported or not, I've taken the easy option and
simply disallowed MAY_BACKLOG on the async completion path. In order
to support it, you need to add a flag to the request context so that
the second EINPROGRESS notification can be selectively filtered out.
So this is what I ended up applying:
commit b51261043f3b6863ed4b65ccad967d998cd302f5
Author: Dudu Lu <phx0fer@gmail.com>
Date: Mon Apr 20 12:40:27 2026 +0800
crypto: krb5enc - fix async decrypt skipping hash verification
krb5enc_dispatch_decrypt() sets req->base.complete as the skcipher
callback, which is the caller's own completion handler. When the
skcipher completes asynchronously, this signals "done" to the caller
without executing krb5enc_dispatch_decrypt_hash(), completely bypassing
the integrity verification (hash check).
Compare with the encrypt path which correctly uses
krb5enc_encrypt_done as an intermediate callback to chain into the
hash computation on async completion.
Fix by adding krb5enc_decrypt_done as an intermediate callback that
chains into krb5enc_dispatch_decrypt_hash() upon async skcipher
completion, matching the encrypt path's callback pattern.
Also fix EBUSY/EINPROGRESS handling throughout: remove
krb5enc_request_complete() which incorrectly swallowed EINPROGRESS
notifications that must be passed up to callers waiting on backlogged
requests, and add missing EBUSY checks in krb5enc_encrypt_ahash_done
for the dispatch_encrypt return value.
Fixes: d1775a177f7f ("crypto: Add 'krb5enc' hash and cipher AEAD algorithm")
Signed-off-by: Dudu Lu <phx0fer@gmail.com>
Unset MAY_BACKLOG on the async completion path so the user won't
see back-to-back EINPROGRESS notifications.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/crypto/krb5enc.c b/crypto/krb5enc.c
index 1bfe8370cf94..fefa8d2c7532 100644
--- a/crypto/krb5enc.c
+++ b/crypto/krb5enc.c
@@ -39,12 +39,6 @@ struct krb5enc_request_ctx {
char tail[];
};
-static void krb5enc_request_complete(struct aead_request *req, int err)
-{
- if (err != -EINPROGRESS)
- aead_request_complete(req, err);
-}
-
/**
* crypto_krb5enc_extractkeys - Extract Ke and Ki keys from the key blob.
* @keys: Where to put the key sizes and pointers
@@ -127,7 +121,7 @@ static void krb5enc_encrypt_done(void *data, int err)
{
struct aead_request *req = data;
- krb5enc_request_complete(req, err);
+ aead_request_complete(req, err);
}
/*
@@ -188,14 +182,16 @@ static void krb5enc_encrypt_ahash_done(void *data, int err)
struct ahash_request *ahreq = (void *)(areq_ctx->tail + ictx->reqoff);
if (err)
- return krb5enc_request_complete(req, err);
+ goto out;
krb5enc_insert_checksum(req, ahreq->result);
- err = krb5enc_dispatch_encrypt(req,
- aead_request_flags(req) & ~CRYPTO_TFM_REQ_MAY_SLEEP);
- if (err != -EINPROGRESS)
- aead_request_complete(req, err);
+ err = krb5enc_dispatch_encrypt(req, 0);
+ if (err == -EINPROGRESS)
+ return;
+
+out:
+ aead_request_complete(req, err);
}
/*
@@ -265,17 +261,16 @@ static void krb5enc_decrypt_hash_done(void *data, int err)
{
struct aead_request *req = data;
- if (err)
- return krb5enc_request_complete(req, err);
-
- err = krb5enc_verify_hash(req);
- krb5enc_request_complete(req, err);
+ if (!err)
+ err = krb5enc_verify_hash(req);
+ aead_request_complete(req, err);
}
/*
* Dispatch the hashing of the plaintext after we've done the decryption.
*/
-static int krb5enc_dispatch_decrypt_hash(struct aead_request *req)
+static int krb5enc_dispatch_decrypt_hash(struct aead_request *req,
+ unsigned int flags)
{
struct crypto_aead *krb5enc = crypto_aead_reqtfm(req);
struct aead_instance *inst = aead_alg_instance(krb5enc);
@@ -291,7 +286,7 @@ static int krb5enc_dispatch_decrypt_hash(struct aead_request *req)
ahash_request_set_tfm(ahreq, auth);
ahash_request_set_crypt(ahreq, req->dst, hash,
req->assoclen + req->cryptlen - authsize);
- ahash_request_set_callback(ahreq, aead_request_flags(req),
+ ahash_request_set_callback(ahreq, flags,
krb5enc_decrypt_hash_done, req);
err = crypto_ahash_digest(ahreq);
@@ -301,6 +296,21 @@ static int krb5enc_dispatch_decrypt_hash(struct aead_request *req)
return krb5enc_verify_hash(req);
}
+static void krb5enc_decrypt_done(void *data, int err)
+{
+ struct aead_request *req = data;
+
+ if (err)
+ goto out;
+
+ err = krb5enc_dispatch_decrypt_hash(req, 0);
+ if (err == -EINPROGRESS)
+ return;
+
+out:
+ aead_request_complete(req, err);
+}
+
/*
* Dispatch the decryption of the ciphertext.
*/
@@ -324,7 +334,7 @@ static int krb5enc_dispatch_decrypt(struct aead_request *req)
skcipher_request_set_tfm(skreq, ctx->enc);
skcipher_request_set_callback(skreq, aead_request_flags(req),
- req->base.complete, req->base.data);
+ krb5enc_decrypt_done, req);
skcipher_request_set_crypt(skreq, src, dst,
req->cryptlen - authsize, req->iv);
@@ -339,7 +349,7 @@ static int krb5enc_decrypt(struct aead_request *req)
if (err < 0)
return err;
- return krb5enc_dispatch_decrypt_hash(req);
+ return krb5enc_dispatch_decrypt_hash(req, aead_request_flags(req));
}
static int krb5enc_init_tfm(struct crypto_aead *tfm)
--
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-04-20 8:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-20 4:40 [PATCH] crypto: krb5enc - fix async decrypt skipping hash verification Dudu Lu
2026-04-20 8:13 ` Herbert Xu
-- strict thread matches above, loose matches on Subject: below --
2026-04-16 13:54 Dudu Lu
2026-04-17 8:48 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox