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: stable@vger.kernel.org Signed-off-by: Pengpeng Hou --- Changes since v2: https://lore.kernel.org/all/20260424070105.1-bpf-crypto-v2-pengpeng@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_sanity.c b/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c index 42bd07f7218d..2ef1e276a63a 100644 --- a/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c +++ b/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c @@ -117,6 +117,18 @@ void test_crypto_sanity(void) udp_test_port = skel->data->udp_test_port; memcpy(skel->bss->key, crypto_key, sizeof(crypto_key)); snprintf(skel->bss->algo, 128, "%s", algo); + + pfd = bpf_program__fd(skel->progs.skb_crypto_setup_bad_algo); + err = bpf_prog_test_run_opts(pfd, &opts); + if (!ASSERT_OK(err, "skb_crypto_setup_bad_algo") || + !ASSERT_OK(opts.retval, "skb_crypto_setup_bad_algo retval")) + goto fail; + + if (!ASSERT_OK(skel->bss->status, "skb_crypto_setup_bad_algo status")) + goto fail; + + opts.retval = 0; + pfd = bpf_program__fd(skel->progs.skb_crypto_setup); if (!ASSERT_GT(pfd, 0, "skb_crypto_setup fd")) goto fail; diff --git a/tools/testing/selftests/bpf/progs/crypto_sanity.c b/tools/testing/selftests/bpf/progs/crypto_sanity.c index dfd8a258f14a..32977b797042 100644 --- a/tools/testing/selftests/bpf/progs/crypto_sanity.c +++ b/tools/testing/selftests/bpf/progs/crypto_sanity.c @@ -82,6 +82,34 @@ int skb_crypto_setup(void *ctx) return 0; } +SEC("syscall") +int skb_crypto_setup_bad_algo(void *ctx) +{ + struct bpf_crypto_params params = { + .type = "skcipher", + .key_len = 16, + }; + struct bpf_crypto_ctx *cctx; + int err = 0; + + status = 0; + + __builtin_memset(params.algo, 'a', sizeof(params.algo)); + __builtin_memcpy(¶ms.key, key, sizeof(key)); + + cctx = bpf_crypto_ctx_create(¶ms, sizeof(params), &err); + if (cctx) { + bpf_crypto_ctx_release(cctx); + status = -EIO; + return 0; + } + + if (err != -EINVAL) + status = err; + + return 0; +} + SEC("tc") int decrypt_sanity(struct __sk_buff *skb) { -- 2.50.1 (Apple Git-155)