[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <28c5b46387e15838028ba57a9d548a5ab066084a8eb7838a2bdeb32677dec11d@mail.kernel.org>
Date: Mon, 8 Dec 2025 03:25:14 +0000 (UTC)
From: bot+bpf-ci@...nel.org
To: git@...ielhodges.dev,bpf@...r.kernel.org
Cc: ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,vadim.fedorenko@...ux.dev,song@...nel.org,yatsenko@...a.com,martin.lau@...ux.dev,eddyz87@...il.com,haoluo@...gle.com,jolsa@...nel.org,john.fastabend@...il.com,kpsingh@...nel.org,sdf@...ichev.me,yonghong.song@...ux.dev,herbert@...dor.apana.org.au,davem@...emloft.net,linux-crypto@...r.kernel.org,linux-kernel@...r.kernel.org,linux-kselftest@...r.kernel.org,git@...ielhodges.dev,ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,martin.lau@...nel.org,eddyz87@...il.com,yonghong.song@...ux.dev,clm@...a.com,ihor.solodrai@...ux.dev
Subject: Re: [PATCH bpf-next v3 2/6] crypto: Add BPF hash algorithm type registration module
> diff --git a/crypto/bpf_crypto_shash.c b/crypto/bpf_crypto_shash.c
> new file mode 100644
> index 000000000000..95c178ec0ce8
> --- /dev/null
> +++ b/crypto/bpf_crypto_shash.c
> @@ -0,0 +1,95 @@
[ ... ]
> +
> +static const struct bpf_crypto_type bpf_crypto_shash_type = {
> + .alloc_tfm = bpf_crypto_shash_alloc_tfm,
> + .free_tfm = bpf_crypto_shash_free_tfm,
> + .has_algo = bpf_crypto_shash_has_algo,
> + .hash = bpf_crypto_shash_hash,
> + .digestsize = bpf_crypto_shash_digestsize,
> + .get_flags = bpf_crypto_shash_get_flags,
> + .owner = THIS_MODULE,
> + .name = "hash",
> +};
The bpf_crypto_type implementation leaves several function pointers as
NULL (setkey, setauthsize, encrypt, decrypt, ivsize, statesize), which is
appropriate for plain hash algorithms. However, this creates an interface
contract mismatch with bpf_crypto_ctx_create() in kernel/bpf/crypto.c.
Looking at the caller bpf_crypto_ctx_create():
Line 174-177:
if (!params->key_len || params->key_len > sizeof(params->key)) {
*err = -EINVAL;
goto err_module_put;
}
Can hash contexts be created? This check requires key_len to be non-zero,
but plain hash algorithms like SHA256 don't use keys. This prevents
creating hash contexts entirely.
Line 186-189:
ctx->tfm = type->alloc_tfm(params->algo);
if (IS_ERR(ctx->tfm)) {
*err = PTR_ERR(ctx->tfm);
goto err_free_tfm;
Can bpf_crypto_shash_free_tfm handle ERR_PTR? When alloc_tfm fails and
returns ERR_PTR, the code stores it in ctx->tfm then jumps to err_free_tfm
which calls type->free_tfm(ctx->tfm). In bpf_crypto_shash_free_tfm(), the
code dereferences ctx->tfm without checking for ERR_PTR, which will crash:
static void bpf_crypto_shash_free_tfm(void *tfm)
{
struct bpf_shash_ctx *ctx = tfm;
crypto_free_shash(ctx->tfm); /* dereferences ctx */
kfree(ctx);
}
Line 198:
*err = type->setkey(ctx->tfm, params->key, params->key_len);
Does this dereference NULL? The hash implementation doesn't provide setkey
(it's NULL in bpf_crypto_shash_type), but this line calls it
unconditionally.
Line 207:
ctx->siv_len = type->ivsize(ctx->tfm) + type->statesize(ctx->tfm);
Do these dereference NULL? The hash implementation doesn't provide ivsize
or statesize (both NULL), but this line calls them unconditionally.
Note: These issues are fixed in commit 14cc32f873e0 ("bpf: Add SHA hash
kfunc for cryptographic hashing") which makes the setkey, ivsize, and
statesize calls conditional, and fixes the ERR_PTR handling by changing
the error path from "goto err_free_tfm" to "goto err_free_ctx".
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20015426139
Powered by blists - more mailing lists