Re: [PATCH] crypto: krb5 - wait for async aead completion before freeing buffer

Re: [PATCH] crypto: krb5 - wait for async aead completion before freeing buffer

[Herbert Xu]

On Sat, May 02, 2026 at 09:25:06AM -0400, Michael Bommarito wrote:
>
> diff --git a/crypto/krb5/rfc3961_simplified.c b/crypto/krb5/rfc3961_simplified.c
> index e49cbdec7c40..c4b8e9b89c7b 100644
> --- a/crypto/krb5/rfc3961_simplified.c
> +++ b/crypto/krb5/rfc3961_simplified.c
> @@ -543,6 +543,7 @@ ssize_t krb5_aead_encrypt(const struct krb5_enctype *krb5,
>  			  size_t data_offset, size_t data_len,
>  			  bool preconfounded)
>  {
> +	DECLARE_CRYPTO_WAIT(wait);
>  	struct aead_request *req;
>  	ssize_t ret, done;
>  	size_t bsize, base_len, secure_offset, secure_len, pad_len, cksum_offset;
> @@ -588,9 +589,10 @@ ssize_t krb5_aead_encrypt(const struct krb5_enctype *krb5,
>  	iv = buffer + krb5_aead_size(aead);
>  
>  	aead_request_set_tfm(req, aead);
> -	aead_request_set_callback(req, 0, NULL, NULL);
> +	aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> +				  crypto_req_done, &wait);
>  	aead_request_set_crypt(req, sg, sg, secure_len, iv);
> -	ret = crypto_aead_encrypt(req);
> +	ret = crypto_wait_req(crypto_aead_encrypt(req), &wait);
>  	if (ret < 0)
>  		goto error;

Since this code was written synchronously, it's probably best
to change the allocation to filter out async algorithms:

	ci = crypto_alloc_aead(krb5->encrypt_name, 0, CRYPTO_ALG_ASYNC);

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

[PATCH v2] crypto: krb5 - filter out async aead implementations at alloc

[Michael Bommarito]

krb5_aead_encrypt(), krb5_aead_decrypt() in rfc3961_simplified.c and
rfc8009_encrypt(), rfc8009_decrypt() in rfc8009_aes2.c set a NULL
completion callback and treat any negative return from
crypto_aead_{encrypt,decrypt}() as terminal, falling through to
kfree_sensitive(buffer).  When the encrypt_name resolves to an
async AEAD instance the request returns -EINPROGRESS, the buffer
is freed while the backend's worker still holds a pointer, and the
worker dereferences the freed slab on completion.

KASAN report under UML+SLUB with a synthetic async aead backend
bound to krb5->encrypt_name:

  BUG: KASAN: slab-use-after-free in t5_stub_complete+0x7d/0xc7

The helpers were written synchronously, so filter the async
instances out at allocation time instead of plumbing
crypto_wait_req() through every call site.

Reachable via net/rxrpc/rxgk.c, fs/afs/cm_security.c and
net/ceph/crypto.c on systems with an async AEAD provider bound to
the krb5 enctype name.

Fixes: 00244da40f78 ("crypto/krb5: Implement the Kerberos5 rfc3961 encrypt and decrypt functions")
Fixes: 6c3c0e86c2ac ("crypto/krb5: Implement the AES enctypes from rfc8009")
Cc: stable@vger.kernel.org
Suggested-by: Herbert Xu <herbert@gondor.apana.org.au>
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 crypto/krb5/krb5_api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/krb5/krb5_api.c b/crypto/krb5/krb5_api.c
index 23026d4206c8..2b20284fa0ab 100644
--- a/crypto/krb5/krb5_api.c
+++ b/crypto/krb5/krb5_api.c
@@ -165,7 +165,7 @@ struct crypto_aead *krb5_prepare_encryption(const struct krb5_enctype *krb5,
 	struct crypto_aead *ci = NULL;
 	int ret = -ENOMEM;
 
-	ci = crypto_alloc_aead(krb5->encrypt_name, 0, 0);
+	ci = crypto_alloc_aead(krb5->encrypt_name, 0, CRYPTO_ALG_ASYNC);
 	if (IS_ERR(ci)) {
 		ret = PTR_ERR(ci);
 		if (ret == -ENOENT)
-- 
2.53.0

Re: [PATCH v2] crypto: krb5 - filter out async aead implementations at alloc

[Herbert Xu]

On Sun, May 10, 2026 at 07:24:55PM -0400, Michael Bommarito wrote:
> krb5_aead_encrypt(), krb5_aead_decrypt() in rfc3961_simplified.c and
> rfc8009_encrypt(), rfc8009_decrypt() in rfc8009_aes2.c set a NULL
> completion callback and treat any negative return from
> crypto_aead_{encrypt,decrypt}() as terminal, falling through to
> kfree_sensitive(buffer).  When the encrypt_name resolves to an
> async AEAD instance the request returns -EINPROGRESS, the buffer
> is freed while the backend's worker still holds a pointer, and the
> worker dereferences the freed slab on completion.
> 
> KASAN report under UML+SLUB with a synthetic async aead backend
> bound to krb5->encrypt_name:
> 
>   BUG: KASAN: slab-use-after-free in t5_stub_complete+0x7d/0xc7
> 
> The helpers were written synchronously, so filter the async
> instances out at allocation time instead of plumbing
> crypto_wait_req() through every call site.
> 
> Reachable via net/rxrpc/rxgk.c, fs/afs/cm_security.c and
> net/ceph/crypto.c on systems with an async AEAD provider bound to
> the krb5 enctype name.
> 
> Fixes: 00244da40f78 ("crypto/krb5: Implement the Kerberos5 rfc3961 encrypt and decrypt functions")
> Fixes: 6c3c0e86c2ac ("crypto/krb5: Implement the AES enctypes from rfc8009")
> Cc: stable@vger.kernel.org
> Suggested-by: Herbert Xu <herbert@gondor.apana.org.au>
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
>  crypto/krb5/krb5_api.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Patch applied.  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 v2] crypto: krb5 - filter out async aead implementations at alloc

[David Howells]

Michael Bommarito <michael.bommarito@gmail.com> wrote:

> -	ci = crypto_alloc_aead(krb5->encrypt_name, 0, 0);
> +	ci = crypto_alloc_aead(krb5->encrypt_name, 0, CRYPTO_ALG_ASYNC);

Apologies, but doesn't that do the opposite of what we want?

Documentation/crypto/architecture.rst says:

	The mask flag restricts the type of cipher. The only allowed flag is
	CRYPTO_ALG_ASYNC to restrict the cipher lookup function to
	asynchronous ciphers. Usually, a caller provides a 0 for the mask
	flag.

Don't we want only synchronous ciphers?

David

Re: [PATCH v2] crypto: krb5 - filter out async aead implementations at alloc

[Michael Bommarito]

On Fri, May 15, 2026 at 7:47 AM David Howells <dhowells@redhat.com> wrote:
>
> Michael Bommarito <michael.bommarito@gmail.com> wrote:
>
> > -     ci = crypto_alloc_aead(krb5->encrypt_name, 0, 0);
> > +     ci = crypto_alloc_aead(krb5->encrypt_name, 0, CRYPTO_ALG_ASYNC);
>
> Apologies, but doesn't that do the opposite of what we want?
>
> Documentation/crypto/architecture.rst says:
>
>         The mask flag restricts the type of cipher. The only allowed flag is
>         CRYPTO_ALG_ASYNC to restrict the cipher lookup function to
>         asynchronous ciphers. Usually, a caller provides a 0 for the mask
>         flag.
>
> Don't we want only synchronous ciphers?

This suggestion originally came from Herbert, but when I checked it, I
missed that note and just looked at the code at crypto/api.c:71:

71         if ((q->cra_flags ^ type) & mask)
  1             continue;

crypto_alloc_sync_aead does the same thing at L212 in aead.c.

So the bit mask should filter the way we want, despite the
documentation's implication.  Perhaps we should separately update that
line in the docs to be more clear about filter and how to properly use
it.

Thanks,
Mike