Re: [PATCH v3] bpf: crypto: snapshot params before string validation

[Pengpeng Hou]

Hi Vadim,

Thanks, you are right that v3 still does not answer the main question
clearly enough.

What I was trying to address is the BPF kfunc boundary, where
bpf_crypto_ctx_create() accepts a BPF-supplied struct and then passes
type/algo to string consumers. In the current tree that path reaches
strcmp() in bpf_crypto_get_type(), and for the skcipher backend it also
reaches crypto_has_skcipher() and crypto_alloc_lskcipher().

That said, your point about the snapshot is fair. v3 conflates the
bounded-string issue with a stability/TOCTOU argument, and I have not
yet re-verified what argument sources the verifier permits for this
kfunc well enough to justify that part.

I will not resend this version. I will re-audit the accepted argument
types for this kfunc and come back only if I can show a real
verifier-reachable failure mode and justify a narrower fix at the BPF
kfunc boundary. Otherwise I will drop the patch.

Thanks,
Pengpeng

Re: [PATCH v3] bpf: crypto: snapshot params before string validation

[Vadim Fedorenko]

On 30/04/2026 05:34, Pengpeng Hou wrote:

bpf_crypto_ctx_create() receives a BPF-supplied params pointer. The
current selftests use static initializers, but BPF programs can also
build the struct in writable BPF memory before calling the kfunc. The
verifier checks that the memory is accessible; it does not prove that
the fixed type[] and algo[] fields are NUL-terminated strings.

Copy the params once into a local snapshot, validate the reserved fields
and fixed-width strings there, and then use the same snapshot for all
later checks and crypto API calls. This also keeps key_len and authsize
stable across validation and use if params points at mutable BPF memory.


You didn't answer the question why copying params will somehow help?



Add a selftest that fills algo[] completely and expects -EINVAL.


What happens without the fix?

BPF Crypto follows in-kernel Crypto API as all other in-kernel users.
If there is a problem in crypto - we have to fix it in crypto subsystem.

NAck.

[PATCH v3] bpf: crypto: snapshot params before string validation

[Pengpeng Hou]

bpf_crypto_ctx_create() receives a BPF-supplied params pointer. The
current selftests use static initializers, but BPF programs can also
build the struct in writable BPF memory before calling the kfunc. The
verifier checks that the memory is accessible; it does not prove that
the fixed type[] and algo[] fields are NUL-terminated strings.

Copy the params once into a local snapshot, validate the reserved fields
and fixed-width strings there, and then use the same snapshot for all
later checks and crypto API calls. This also keeps key_len and authsize
stable across validation and use if params points at mutable BPF memory.

Clear the local snapshot before returning because it contains key
material.

Add a selftest that fills algo[] completely and expects -EINVAL.

Fixes: 3e1c6f35409f ("bpf: make common crypto API for TC/XDP programs")
Cc: sta...@vger.kernel.org
Signed-off-by: Pengpeng Hou 
---
Changes since v2: 
https://lore.kernel.org/all/20260424070105.1-bpf-crypto-v2-pengp...@iscas.ac.cn/
- clear the local params snapshot with memzero_explicit() on success and
  error paths because it contains key material
- remove the redundant fd > 0 check from the added selftest; skeleton
  open_and_load() already validates the program, and fd 0 is valid

 kernel/bpf/crypto.c | 49 -
 tools/testing/selftests/bpf/prog_tests/crypto_sanity.c | 16 
 tools/testing/selftests/bpf/progs/crypto_sanity.c  | 28 ++
 3 files changed, 76 insertions(+), 17 deletions(-)

diff --git a/kernel/bpf/crypto.c b/kernel/bpf/crypto.c
index 51f89cecefb4..72e5707c1ead 100644
--- a/kernel/bpf/crypto.c
+++ b/kernel/bpf/crypto.c
@@ -147,31 +147,47 @@ bpf_crypto_ctx_create(const struct bpf_crypto_params 
*params, u32 params__sz,
  int *err)
 {
const struct bpf_crypto_type *type;
+   struct bpf_crypto_params params_copy;
struct bpf_crypto_ctx *ctx;
 
-   if (!params || params->reserved[0] || params->reserved[1] ||
-   params__sz != sizeof(struct bpf_crypto_params)) {
+   if (!params || params__sz != sizeof(params_copy)) {
*err = -EINVAL;
return NULL;
}
 
-   type = bpf_crypto_get_type(params->type);
+   params_copy = *params;
+
+   if (params_copy.reserved[0] || params_copy.reserved[1]) {
+   *err = -EINVAL;
+   goto err_clear_params;
+   }
+
+   if (strnlen(params_copy.type, sizeof(params_copy.type)) ==
+   sizeof(params_copy.type) ||
+   strnlen(params_copy.algo, sizeof(params_copy.algo)) ==
+   sizeof(params_copy.algo)) {
+   *err = -EINVAL;
+   goto err_clear_params;
+   }
+
+   type = bpf_crypto_get_type(params_copy.type);
if (IS_ERR(type)) {
*err = PTR_ERR(type);
-   return NULL;
+   goto err_clear_params;
}
 
-   if (!type->has_algo(params->algo)) {
+   if (!type->has_algo(params_copy.algo)) {
*err = -EOPNOTSUPP;
goto err_module_put;
}
 
-   if (!!params->authsize ^ !!type->setauthsize) {
+   if (!!params_copy.authsize ^ !!type->setauthsize) {
*err = -EOPNOTSUPP;
goto err_module_put;
}
 
-   if (!params->key_len || params->key_len > sizeof(params->key)) {
+   if (!params_copy.key_len ||
+   params_copy.key_len > sizeof(params_copy.key)) {
*err = -EINVAL;
goto err_module_put;
}
@@ -183,19 +198,19 @@ bpf_crypto_ctx_create(const struct bpf_crypto_params 
*params, u32 params__sz,
}
 
ctx->type = type;
-   ctx->tfm = type->alloc_tfm(params->algo);
+   ctx->tfm = type->alloc_tfm(params_copy.algo);
if (IS_ERR(ctx->tfm)) {
*err = PTR_ERR(ctx->tfm);
goto err_free_ctx;
}
 
-   if (params->authsize) {
-   *err = type->setauthsize(ctx->tfm, params->authsize);
+   if (params_copy.authsize) {
+   *err = type->setauthsize(ctx->tfm, params_copy.authsize);
if (*err)
goto err_free_tfm;
}
 
-   *err = type->setkey(ctx->tfm, params->key, params->key_len);
+   *err = type->setkey(ctx->tfm, params_copy.key, params_copy.key_len);
if (*err)
goto err_free_tfm;
 
@@ -207,6 +222,7 @@ bpf_crypto_ctx_create(const struct bpf_crypto_params 
*params, u32 params__sz,
 
refcount_set(&ctx->usage, 1);
 
+   memzero_explicit(¶ms_copy, sizeof(params_copy));
return ctx;
 
 err_free_tfm:
@@ -215,7 +231,8 @@ bpf_crypto_ctx_create(const struct bpf_crypto_params 
*params, u32 params__sz,
kfree(ctx);
 err_module_put:
module_put(type->owner);
-
+err_clear_params:
+   memzero_explicit(¶ms_copy, sizeof(params_copy));
return NULL;
 }
 
diff --git a/tools/testing/selftests/bpf/prog_tests/crypto