Linux cryptographic layer development
 help / color / mirror / Atom feed
* [PATCH bpf-next] bpf: crypto: Use AES-CBC and AES-ECB libraries
@ 2026-08-31 19:21 Eric Biggers
  2026-08-31 20:29 ` bot+bpf-ci
  2026-08-31 21:25 ` Karl Mehltretter
  0 siblings, 2 replies; 6+ messages in thread
From: Eric Biggers @ 2026-08-31 19:21 UTC (permalink / raw)
  To: bpf, Vadim Fedorenko, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi
  Cc: linux-crypto, linux-kernel, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
	John Fastabend, Karl Mehltretter, Eric Biggers

BPF crypto was implemented using the lskcipher API, which doesn't seem
to be going anywhere.  lskcipher supports only "arc4", "cbc(aes)",
"ecb(aes)", and only with unoptimized implementations.

Library APIs also have been found to be a much better approach, for a
variety of reasons, including reduced overhead, greater flexibility, and
having to be explicit about the crypto algorithms that are supported.

We can safely ignore the theoretical ARC4 support in BPF crypto as
unused, which leaves AES-CBC and AES-ECB.  AES-CBC was stated to be
needed for decrypting packets using a homebrew UDP-based protocol
(https://lore.kernel.org/r/d1cdfc23-b336-49a9-8833-29f05b5b9fec@linux.dev/).
AES-ECB is used by the BPF self-tests, and it was stated to maybe be
used in the future for QUIC-LB
(https://lore.kernel.org/r/5f9c3aab-5339-463c-a86d-edac297e1e95@linux.dev/).

Those reasons don't make much sense either, especially AES-ECB which
isn't appropriate in new systems and should be dropped.  Regardless,
let's assume that both AES-CBC and AES-ECB need to be kept for now.

There are library APIs for both of these now, which are much easier to
use and more efficient.  Reimplement BPF crypto on top of them, greatly
simplifying the code.  As part of this, the bpf_crypto_type abstraction
layer is removed, as it's not useful.

Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 MAINTAINERS                  |   2 -
 crypto/Makefile              |   3 -
 crypto/bpf_crypto_skcipher.c |  83 -----------
 include/linux/bpf_crypto.h   |  24 ----
 kernel/bpf/Kconfig           |   9 ++
 kernel/bpf/Makefile          |   4 +-
 kernel/bpf/crypto.c          | 271 +++++++++++++++--------------------
 7 files changed, 122 insertions(+), 274 deletions(-)
 delete mode 100644 crypto/bpf_crypto_skcipher.c
 delete mode 100644 include/linux/bpf_crypto.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 3a19da74d00c..16f13270ce1f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4936,8 +4936,6 @@ BPF [CRYPTO]
 M:	Vadim Fedorenko <vadim.fedorenko@linux.dev>
 L:	bpf@vger.kernel.org
 S:	Maintained
-F:	crypto/bpf_crypto_skcipher.c
-F:	include/linux/bpf_crypto.h
 F:	kernel/bpf/crypto.c
 
 BPF [DOCUMENTATION] (Related to Standardization)
diff --git a/crypto/Makefile b/crypto/Makefile
index 8386d55a9755..33bb5ad595e8 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -22,9 +22,6 @@ crypto_skcipher-y += lskcipher.o
 crypto_skcipher-y += skcipher.o
 
 obj-$(CONFIG_CRYPTO_SKCIPHER2) += crypto_skcipher.o
-ifeq ($(CONFIG_BPF_SYSCALL),y)
-obj-$(CONFIG_CRYPTO_SKCIPHER2) += bpf_crypto_skcipher.o
-endif
 
 obj-$(CONFIG_CRYPTO_SEQIV) += seqiv.o
 obj-$(CONFIG_CRYPTO_ECHAINIV) += echainiv.o
diff --git a/crypto/bpf_crypto_skcipher.c b/crypto/bpf_crypto_skcipher.c
deleted file mode 100644
index a88798d3e8c8..000000000000
--- a/crypto/bpf_crypto_skcipher.c
+++ /dev/null
@@ -1,83 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/* Copyright (c) 2024 Meta, Inc */
-#include <linux/types.h>
-#include <linux/module.h>
-#include <linux/bpf_crypto.h>
-#include <crypto/skcipher.h>
-
-static void *bpf_crypto_lskcipher_alloc_tfm(const char *algo)
-{
-	return crypto_alloc_lskcipher(algo, 0, 0);
-}
-
-static void bpf_crypto_lskcipher_free_tfm(void *tfm)
-{
-	crypto_free_lskcipher(tfm);
-}
-
-static int bpf_crypto_lskcipher_has_algo(const char *algo)
-{
-	return crypto_has_skcipher(algo, CRYPTO_ALG_TYPE_LSKCIPHER, CRYPTO_ALG_TYPE_MASK);
-}
-
-static int bpf_crypto_lskcipher_setkey(void *tfm, const u8 *key, unsigned int keylen)
-{
-	return crypto_lskcipher_setkey(tfm, key, keylen);
-}
-
-static u32 bpf_crypto_lskcipher_get_flags(void *tfm)
-{
-	return crypto_lskcipher_get_flags(tfm);
-}
-
-static unsigned int bpf_crypto_lskcipher_ivsize(void *tfm)
-{
-	return crypto_lskcipher_ivsize(tfm);
-}
-
-static unsigned int bpf_crypto_lskcipher_statesize(void *tfm)
-{
-	return crypto_lskcipher_statesize(tfm);
-}
-
-static int bpf_crypto_lskcipher_encrypt(void *tfm, const u8 *src, u8 *dst,
-					unsigned int len, u8 *siv)
-{
-	return crypto_lskcipher_encrypt(tfm, src, dst, len, siv);
-}
-
-static int bpf_crypto_lskcipher_decrypt(void *tfm, const u8 *src, u8 *dst,
-					unsigned int len, u8 *siv)
-{
-	return crypto_lskcipher_decrypt(tfm, src, dst, len, siv);
-}
-
-static const struct bpf_crypto_type bpf_crypto_lskcipher_type = {
-	.alloc_tfm	= bpf_crypto_lskcipher_alloc_tfm,
-	.free_tfm	= bpf_crypto_lskcipher_free_tfm,
-	.has_algo	= bpf_crypto_lskcipher_has_algo,
-	.setkey		= bpf_crypto_lskcipher_setkey,
-	.encrypt	= bpf_crypto_lskcipher_encrypt,
-	.decrypt	= bpf_crypto_lskcipher_decrypt,
-	.ivsize		= bpf_crypto_lskcipher_ivsize,
-	.statesize	= bpf_crypto_lskcipher_statesize,
-	.get_flags	= bpf_crypto_lskcipher_get_flags,
-	.owner		= THIS_MODULE,
-	.name		= "skcipher",
-};
-
-static int __init bpf_crypto_skcipher_init(void)
-{
-	return bpf_crypto_register_type(&bpf_crypto_lskcipher_type);
-}
-
-static void __exit bpf_crypto_skcipher_exit(void)
-{
-	int err = bpf_crypto_unregister_type(&bpf_crypto_lskcipher_type);
-	WARN_ON_ONCE(err);
-}
-
-module_init(bpf_crypto_skcipher_init);
-module_exit(bpf_crypto_skcipher_exit);
-MODULE_LICENSE("GPL");
-MODULE_DESCRIPTION("Symmetric key cipher support for BPF");
diff --git a/include/linux/bpf_crypto.h b/include/linux/bpf_crypto.h
deleted file mode 100644
index a41e71d4e2d9..000000000000
--- a/include/linux/bpf_crypto.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
-#ifndef _BPF_CRYPTO_H
-#define _BPF_CRYPTO_H
-
-struct bpf_crypto_type {
-	void *(*alloc_tfm)(const char *algo);
-	void (*free_tfm)(void *tfm);
-	int (*has_algo)(const char *algo);
-	int (*setkey)(void *tfm, const u8 *key, unsigned int keylen);
-	int (*setauthsize)(void *tfm, unsigned int authsize);
-	int (*encrypt)(void *tfm, const u8 *src, u8 *dst, unsigned int len, u8 *iv);
-	int (*decrypt)(void *tfm, const u8 *src, u8 *dst, unsigned int len, u8 *iv);
-	unsigned int (*ivsize)(void *tfm);
-	unsigned int (*statesize)(void *tfm);
-	u32 (*get_flags)(void *tfm);
-	struct module *owner;
-	char name[14];
-};
-
-int bpf_crypto_register_type(const struct bpf_crypto_type *type);
-int bpf_crypto_unregister_type(const struct bpf_crypto_type *type);
-
-#endif /* _BPF_CRYPTO_H */
diff --git a/kernel/bpf/Kconfig b/kernel/bpf/Kconfig
index eb3de35734f0..822599b42be2 100644
--- a/kernel/bpf/Kconfig
+++ b/kernel/bpf/Kconfig
@@ -87,6 +87,15 @@ config BPF_UNPRIV_DEFAULT_OFF
 
 	  If you are unsure how to answer this question, answer Y.
 
+config BPF_CRYPTO
+	def_bool y
+	depends on BPF_SYSCALL
+	select CRYPTO_LIB_AES_CBC
+	select CRYPTO_LIB_AES_ECB
+	help
+	  Provide the kfuncs needed for BPF programs to encrypt and decrypt
+	  data. The supported algorithms are AES-CBC and AES-ECB.
+
 source "kernel/bpf/preload/Kconfig"
 
 config BPF_LSM
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index 90255d80e5be..ab2e5d2782e6 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -55,9 +55,7 @@ obj-$(CONFIG_BPF_SYSCALL) += cpumask.o
 # semantics within pahole are revisited accordingly.
 obj-${CONFIG_BPF_LSM} += bpf_lsm_proto.o bpf_lsm.o
 endif
-ifneq ($(CONFIG_CRYPTO),)
-obj-$(CONFIG_BPF_SYSCALL) += crypto.o
-endif
+obj-$(CONFIG_BPF_CRYPTO) += crypto.o
 obj-$(CONFIG_BPF_PRELOAD) += preload/
 
 obj-$(CONFIG_BPF_SYSCALL) += relo_core.o
diff --git a/kernel/bpf/crypto.c b/kernel/bpf/crypto.c
index 51f89cecefb4..8cdc16f9b374 100644
--- a/kernel/bpf/crypto.c
+++ b/kernel/bpf/crypto.c
@@ -1,19 +1,13 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /* Copyright (c) 2024 Meta, Inc */
 #include <linux/bpf.h>
-#include <linux/bpf_crypto.h>
 #include <linux/bpf_mem_alloc.h>
 #include <linux/btf.h>
 #include <linux/btf_ids.h>
 #include <linux/filter.h>
-#include <linux/scatterlist.h>
 #include <linux/skbuff.h>
-#include <crypto/skcipher.h>
-
-struct bpf_crypto_type_list {
-	const struct bpf_crypto_type *type;
-	struct list_head list;
-};
+#include <crypto/aes-cbc.h>
+#include <crypto/aes-ecb.h>
 
 /* BPF crypto initialization parameters struct */
 /**
@@ -36,94 +30,53 @@ struct bpf_crypto_params {
 	u32 authsize;
 };
 
-static LIST_HEAD(bpf_crypto_types);
-static DECLARE_RWSEM(bpf_crypto_types_sem);
+enum bpf_crypto_algo_id {
+	BPF_ALGO_AES_CBC,
+	BPF_ALGO_AES_ECB,
+};
+
+static const struct {
+	const char *type_name;
+	const char *algo_name;
+	enum bpf_crypto_algo_id algo;
+} bpf_crypto_algos[] = {
+	{ "skcipher", "cbc(aes)", BPF_ALGO_AES_CBC },
+	{ "skcipher", "ecb(aes)", BPF_ALGO_AES_ECB },
+};
+
+static bool bpf_crypto_find_algo(const struct bpf_crypto_params *params,
+				 enum bpf_crypto_algo_id *id_ret)
+{
+	for (size_t i = 0; i < ARRAY_SIZE(bpf_crypto_algos); i++) {
+		if (strncmp(bpf_crypto_algos[i].type_name, params->type,
+			    sizeof(params->type)) == 0 &&
+		    strncmp(bpf_crypto_algos[i].algo_name, params->algo,
+			    sizeof(params->algo)) == 0) {
+			*id_ret = bpf_crypto_algos[i].algo;
+			return true;
+		}
+	}
+	return false;
+}
 
 /**
  * struct bpf_crypto_ctx - refcounted BPF crypto context structure
- * @type:	The pointer to bpf crypto type
- * @tfm:	The pointer to instance of crypto API struct.
- * @siv_len:    Size of IV and state storage for cipher
+ * @algo:	The crypto algorithm ID
+ * @key:	The crypto key
  * @rcu:	The RCU head used to free the crypto context with RCU safety.
  * @usage:	Object reference counter. When the refcount goes to 0, the
  *		memory is released back to the BPF allocator, which provides
  *		RCU safety.
  */
 struct bpf_crypto_ctx {
-	const struct bpf_crypto_type *type;
-	void *tfm;
-	u32 siv_len;
+	enum bpf_crypto_algo_id algo;
+	union {
+		struct aes_key aes;
+	} key;
 	struct rcu_head rcu;
 	refcount_t usage;
 };
 
-int bpf_crypto_register_type(const struct bpf_crypto_type *type)
-{
-	struct bpf_crypto_type_list *node;
-	int err = -EBUSY;
-
-	down_write(&bpf_crypto_types_sem);
-	list_for_each_entry(node, &bpf_crypto_types, list) {
-		if (!strcmp(node->type->name, type->name))
-			goto unlock;
-	}
-
-	node = kmalloc_obj(*node);
-	err = -ENOMEM;
-	if (!node)
-		goto unlock;
-
-	node->type = type;
-	list_add(&node->list, &bpf_crypto_types);
-	err = 0;
-
-unlock:
-	up_write(&bpf_crypto_types_sem);
-
-	return err;
-}
-EXPORT_SYMBOL_GPL(bpf_crypto_register_type);
-
-int bpf_crypto_unregister_type(const struct bpf_crypto_type *type)
-{
-	struct bpf_crypto_type_list *node;
-	int err = -ENOENT;
-
-	down_write(&bpf_crypto_types_sem);
-	list_for_each_entry(node, &bpf_crypto_types, list) {
-		if (strcmp(node->type->name, type->name))
-			continue;
-
-		list_del(&node->list);
-		kfree(node);
-		err = 0;
-		break;
-	}
-	up_write(&bpf_crypto_types_sem);
-
-	return err;
-}
-EXPORT_SYMBOL_GPL(bpf_crypto_unregister_type);
-
-static const struct bpf_crypto_type *bpf_crypto_get_type(const char *name)
-{
-	const struct bpf_crypto_type *type = ERR_PTR(-ENOENT);
-	struct bpf_crypto_type_list *node;
-
-	down_read(&bpf_crypto_types_sem);
-	list_for_each_entry(node, &bpf_crypto_types, list) {
-		if (strcmp(node->type->name, name))
-			continue;
-
-		if (try_module_get(node->type->owner))
-			type = node->type;
-		break;
-	}
-	up_read(&bpf_crypto_types_sem);
-
-	return type;
-}
-
 __bpf_kfunc_start_defs();
 
 /**
@@ -146,7 +99,6 @@ __bpf_kfunc struct bpf_crypto_ctx *
 bpf_crypto_ctx_create(const struct bpf_crypto_params *params, u32 params__sz,
 		      int *err)
 {
-	const struct bpf_crypto_type *type;
 	struct bpf_crypto_ctx *ctx;
 
 	if (!params || params->reserved[0] || params->reserved[1] ||
@@ -155,69 +107,40 @@ bpf_crypto_ctx_create(const struct bpf_crypto_params *params, u32 params__sz,
 		return NULL;
 	}
 
-	type = bpf_crypto_get_type(params->type);
-	if (IS_ERR(type)) {
-		*err = PTR_ERR(type);
-		return NULL;
-	}
-
-	if (!type->has_algo(params->algo)) {
-		*err = -EOPNOTSUPP;
-		goto err_module_put;
-	}
-
-	if (!!params->authsize ^ !!type->setauthsize) {
-		*err = -EOPNOTSUPP;
-		goto err_module_put;
-	}
-
-	if (!params->key_len || params->key_len > sizeof(params->key)) {
-		*err = -EINVAL;
-		goto err_module_put;
-	}
-
 	ctx = kzalloc_obj(*ctx);
 	if (!ctx) {
 		*err = -ENOMEM;
-		goto err_module_put;
+		return NULL;
 	}
 
-	ctx->type = type;
-	ctx->tfm = type->alloc_tfm(params->algo);
-	if (IS_ERR(ctx->tfm)) {
-		*err = PTR_ERR(ctx->tfm);
-		goto err_free_ctx;
+	if (!bpf_crypto_find_algo(params, &ctx->algo)) {
+		*err = -ENOENT;
+		goto out;
 	}
 
-	if (params->authsize) {
-		*err = type->setauthsize(ctx->tfm, params->authsize);
-		if (*err)
-			goto err_free_tfm;
+	switch (ctx->algo) {
+	case BPF_ALGO_AES_CBC:
+	case BPF_ALGO_AES_ECB:
+		if (params->authsize)
+			*err = -EOPNOTSUPP;
+		else
+			*err = aes_preparekey(&ctx->key.aes, params->key,
+					      params->key_len);
+		break;
+	default:
+		WARN_ON(1);
+		*err = -ENOENT;
+		break;
 	}
 
-	*err = type->setkey(ctx->tfm, params->key, params->key_len);
-	if (*err)
-		goto err_free_tfm;
-
-	if (type->get_flags(ctx->tfm) & CRYPTO_TFM_NEED_KEY) {
-		*err = -EINVAL;
-		goto err_free_tfm;
+out:
+	if (*err) {
+		kfree_sensitive(ctx);
+		return NULL;
 	}
 
-	ctx->siv_len = type->ivsize(ctx->tfm) + type->statesize(ctx->tfm);
-
 	refcount_set(&ctx->usage, 1);
-
 	return ctx;
-
-err_free_tfm:
-	type->free_tfm(ctx->tfm);
-err_free_ctx:
-	kfree(ctx);
-err_module_put:
-	module_put(type->owner);
-
-	return NULL;
 }
 
 static void crypto_free_cb(struct rcu_head *head)
@@ -225,9 +148,7 @@ static void crypto_free_cb(struct rcu_head *head)
 	struct bpf_crypto_ctx *ctx;
 
 	ctx = container_of(head, struct bpf_crypto_ctx, rcu);
-	ctx->type->free_tfm(ctx->tfm);
-	module_put(ctx->type->owner);
-	kfree(ctx);
+	kfree_sensitive(ctx);
 }
 
 /**
@@ -267,27 +188,53 @@ __bpf_kfunc void bpf_crypto_ctx_release_dtor(void *ctx)
 }
 CFI_NOSEAL(bpf_crypto_ctx_release_dtor);
 
+static int bpf_aes_cbc_crypt(u8 *dst, u32 dst_len, const u8 *src, u32 src_len,
+			     u8 *iv, u32 iv_len,
+			     const struct bpf_crypto_ctx *ctx, bool decrypt)
+{
+	if (iv_len != AES_BLOCK_SIZE)
+		return -EINVAL;
+	if (src_len % AES_BLOCK_SIZE || dst_len < src_len)
+		return -EINVAL;
+	if (decrypt)
+		aes_cbc_decrypt(dst, src, src_len, iv, &ctx->key.aes);
+	else
+		aes_cbc_encrypt(dst, src, src_len, iv, &ctx->key.aes);
+	return 0;
+}
+
+static int bpf_aes_ecb_crypt(u8 *dst, u32 dst_len, const u8 *src, u32 src_len,
+			     u8 *iv, u32 iv_len,
+			     const struct bpf_crypto_ctx *ctx, bool decrypt)
+{
+	if (iv_len != 0)
+		return -EINVAL;
+	if (src_len % AES_BLOCK_SIZE || dst_len < src_len)
+		return -EINVAL;
+	if (decrypt)
+		aes_ecb_decrypt(dst, src, src_len, &ctx->key.aes);
+	else
+		aes_ecb_encrypt(dst, src, src_len, &ctx->key.aes);
+	return 0;
+}
+
 static int bpf_crypto_crypt(const struct bpf_crypto_ctx *ctx,
 			    const struct bpf_dynptr_kern *src,
 			    const struct bpf_dynptr_kern *dst,
-			    const struct bpf_dynptr_kern *siv,
+			    const struct bpf_dynptr_kern *iv,
 			    bool decrypt)
 {
-	u32 src_len, dst_len, siv_len;
+	u32 src_len, dst_len, iv_len;
 	const u8 *psrc;
 	u8 *pdst, *piv;
-	int err;
 
 	if (__bpf_dynptr_is_rdonly(dst))
 		return -EINVAL;
 
-	siv_len = siv ? __bpf_dynptr_size(siv) : 0;
+	iv_len = iv ? __bpf_dynptr_size(iv) : 0;
 	src_len = __bpf_dynptr_size(src);
 	dst_len = __bpf_dynptr_size(dst);
-	if (!src_len || !dst_len || src_len > dst_len)
-		return -EINVAL;
-
-	if (siv_len != ctx->siv_len)
+	if (!src_len || !dst_len)
 		return -EINVAL;
 
 	psrc = __bpf_dynptr_data(src, src_len);
@@ -297,14 +244,20 @@ static int bpf_crypto_crypt(const struct bpf_crypto_ctx *ctx,
 	if (!pdst)
 		return -EINVAL;
 
-	piv = siv_len ? __bpf_dynptr_data_rw(siv, siv_len) : NULL;
-	if (siv_len && !piv)
+	piv = iv_len ? __bpf_dynptr_data_rw(iv, iv_len) : NULL;
+	if (iv_len && !piv)
 		return -EINVAL;
 
-	err = decrypt ? ctx->type->decrypt(ctx->tfm, psrc, pdst, src_len, piv)
-		      : ctx->type->encrypt(ctx->tfm, psrc, pdst, src_len, piv);
-
-	return err;
+	switch (ctx->algo) {
+	case BPF_ALGO_AES_CBC:
+		return bpf_aes_cbc_crypt(pdst, dst_len, psrc, src_len, piv,
+					 iv_len, ctx, decrypt);
+	case BPF_ALGO_AES_ECB:
+		return bpf_aes_ecb_crypt(pdst, dst_len, psrc, src_len, piv,
+					 iv_len, ctx, decrypt);
+	default:
+		return -EINVAL;
+	}
 }
 
 /**
@@ -312,20 +265,20 @@ static int bpf_crypto_crypt(const struct bpf_crypto_ctx *ctx,
  * @ctx:		The crypto context being used. The ctx must be a trusted pointer.
  * @src:		bpf_dynptr to the encrypted data. Must be a trusted pointer.
  * @dst:		bpf_dynptr to the buffer where to store the result. Must be a trusted pointer.
- * @siv__nullable:	bpf_dynptr to IV data and state data to be used by decryptor. May be NULL.
+ * @iv__nullable:	bpf_dynptr to the initialization vector. May be NULL.
  *
  * Decrypts provided buffer using IV data and the crypto context. Crypto context must be configured.
  */
 __bpf_kfunc int bpf_crypto_decrypt(struct bpf_crypto_ctx *ctx,
 				   const struct bpf_dynptr *src,
 				   const struct bpf_dynptr *dst,
-				   const struct bpf_dynptr *siv__nullable)
+				   const struct bpf_dynptr *iv__nullable)
 {
 	const struct bpf_dynptr_kern *src_kern = (struct bpf_dynptr_kern *)src;
 	const struct bpf_dynptr_kern *dst_kern = (struct bpf_dynptr_kern *)dst;
-	const struct bpf_dynptr_kern *siv_kern = (struct bpf_dynptr_kern *)siv__nullable;
+	const struct bpf_dynptr_kern *iv_kern = (struct bpf_dynptr_kern *)iv__nullable;
 
-	return bpf_crypto_crypt(ctx, src_kern, dst_kern, siv_kern, true);
+	return bpf_crypto_crypt(ctx, src_kern, dst_kern, iv_kern, true);
 }
 
 /**
@@ -333,20 +286,20 @@ __bpf_kfunc int bpf_crypto_decrypt(struct bpf_crypto_ctx *ctx,
  * @ctx:		The crypto context being used. The ctx must be a trusted pointer.
  * @src:		bpf_dynptr to the plain data. Must be a trusted pointer.
  * @dst:		bpf_dynptr to the buffer where to store the result. Must be a trusted pointer.
- * @siv__nullable:	bpf_dynptr to IV data and state data to be used by decryptor. May be NULL.
+ * @iv__nullable:	bpf_dynptr to the initialization vector. May be NULL.
  *
  * Encrypts provided buffer using IV data and the crypto context. Crypto context must be configured.
  */
 __bpf_kfunc int bpf_crypto_encrypt(struct bpf_crypto_ctx *ctx,
 				   const struct bpf_dynptr *src,
 				   const struct bpf_dynptr *dst,
-				   const struct bpf_dynptr *siv__nullable)
+				   const struct bpf_dynptr *iv__nullable)
 {
 	const struct bpf_dynptr_kern *src_kern = (struct bpf_dynptr_kern *)src;
 	const struct bpf_dynptr_kern *dst_kern = (struct bpf_dynptr_kern *)dst;
-	const struct bpf_dynptr_kern *siv_kern = (struct bpf_dynptr_kern *)siv__nullable;
+	const struct bpf_dynptr_kern *iv_kern = (struct bpf_dynptr_kern *)iv__nullable;
 
-	return bpf_crypto_crypt(ctx, src_kern, dst_kern, siv_kern, false);
+	return bpf_crypto_crypt(ctx, src_kern, dst_kern, iv_kern, false);
 }
 
 __bpf_kfunc_end_defs();

base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next] bpf: crypto: Use AES-CBC and AES-ECB libraries
  2026-08-31 19:21 [PATCH bpf-next] bpf: crypto: Use AES-CBC and AES-ECB libraries Eric Biggers
@ 2026-08-31 20:29 ` bot+bpf-ci
  2026-08-31 21:59   ` Eric Biggers
  2026-08-31 21:25 ` Karl Mehltretter
  1 sibling, 1 reply; 6+ messages in thread
From: bot+bpf-ci @ 2026-08-31 20:29 UTC (permalink / raw)
  To: ebiggers, bpf, vadim.fedorenko, ast, daniel, andrii, eddyz87,
	memxor
  Cc: linux-crypto, linux-kernel, martin.lau, song, yonghong.song,
	jolsa, emil, ihor.solodrai, john.fastabend, kmehltretter,
	ebiggers, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song,
	mason, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 7825 bytes --]

> BPF crypto was implemented using the lskcipher API, which doesn't seem
> to be going anywhere.  lskcipher supports only "arc4", "cbc(aes)",
> "ecb(aes)", and only with unoptimized implementations.

The claim that lskcipher supports only these three specific algorithm
names is not accurate. The cbc and ecb templates in crypto/cbc.c and
crypto/ecb.c instantiate lskcipher interfaces for any plain cipher
algorithm, not just AES:

crypto/cbc.c:crypto_cbc_create() calls lskcipher_alloc_instance_simple()
crypto/ecb.c:crypto_ecb_create() calls lskcipher_alloc_instance_simple()

So cbc(<any-cipher>) and ecb(<any-cipher>) are valid lskcipher
algorithms. The kernel includes about 14 plain cipher algorithms besides
AES, including sm4, des, des3_ede, camellia, twofish, aria, blowfish,
cast5, cast6, serpent, seed, khazad, anubis, tea, and xtea.

The pre-patch code passed params->algo directly to
crypto_alloc_lskcipher() with no whitelist, so a BPF program could use
cbc(sm4), cbc(des3_ede), ecb(camellia), or ecb(twofish).

After this patch, bpf_crypto_find_algo() only matches the two-entry
table with cbc(aes) and ecb(aes), so all non-AES cbc/ecb modes now fail.

The changelog justifies dropping ARC4 but does not mention that this
also drops support for all non-AES block ciphers. Should the commit
message explicitly state that non-AES cbc()/ecb() support is being
removed, or should the characterization of lskcipher's scope be
corrected?

Additionally, the error code for an unsupported algorithm changes from
-EOPNOTSUPP (old has_algo failure path) to -ENOENT (new
bpf_crypto_find_algo() path), which is a visible behavior change for
existing BPF programs that inspect errno.

[ ... ]

> diff --git a/kernel/bpf/Kconfig b/kernel/bpf/Kconfig
> index eb3de35734f09..822599b42be2e 100644
> --- a/kernel/bpf/Kconfig
> +++ b/kernel/bpf/Kconfig
> @@ -87,6 +87,15 @@ config BPF_UNPRIV_DEFAULT_OFF
>
>  	  If you are unsure how to answer this question, answer Y.
>
> +config BPF_CRYPTO
> +	def_bool y
> +	depends on BPF_SYSCALL
> +	select CRYPTO_LIB_AES_CBC
> +	select CRYPTO_LIB_AES_ECB
> +	help
> +	  Provide the kfuncs needed for BPF programs to encrypt and decrypt
> +	  data. The supported algorithms are AES-CBC and AES-ECB.
> +

The def_bool y makes BPF_CRYPTO invisible and unconditionally enabled
whenever CONFIG_BPF_SYSCALL=y, so the AES library code is always linked
with no opt-out.

Before this patch, kernel/bpf/Makefile gated crypto.o on:

  ifneq ($(CONFIG_CRYPTO),)
  obj-$(CONFIG_BPF_SYSCALL) += crypto.o
  endif

so a CONFIG_BPF_SYSCALL=y, CONFIG_CRYPTO=n kernel previously built
neither the BPF crypto kfuncs nor any AES code. Now the AES
key-schedule, CBC, and ECB library code is unconditionally present.

This is particularly notable given that the commit message says AES-ECB
"isn't appropriate in new systems and should be dropped" yet it is now
unconditionally linked into every BPF_SYSCALL kernel.

Should this be a user-selectable option with a prompt like "bool 'BPF
crypto kfuncs'" so it can be disabled, or is the intent really to make
it always-on?

[ ... ]

> diff --git a/crypto/bpf_crypto_skcipher.c b/crypto/bpf_crypto_skcipher.c
> deleted file mode 100644
> index a88798d3e8c87..0000000000000
> --- a/crypto/bpf_crypto_skcipher.c
> +++ /dev/null
> @@ -1,83 +0,0 @@
[ ... ]
> -static void *bpf_crypto_lskcipher_alloc_tfm(const char *algo)
> -{
> -	return crypto_alloc_lskcipher(algo, 0, 0);
> -}

Deleting this file removes the last in-tree caller of
crypto_alloc_lskcipher(). After this patch, the symbol only exists at
its definition and export in crypto/lskcipher.c, with no remaining
callers anywhere in the tree.

Given the premise that the lskcipher API "doesn't seem to be going
anywhere," should crypto_alloc_lskcipher() be removed in a follow-up, or
left as an exported API with zero users?

[ ... ]

> diff --git a/kernel/bpf/crypto.c b/kernel/bpf/crypto.c
> index 51f89cecefb4d..8cdc16f9b374a 100644
> --- a/kernel/bpf/crypto.c
> +++ b/kernel/bpf/crypto.c
[ ... ]
> @@ -146,7 +99,6 @@ __bpf_kfunc struct bpf_crypto_ctx *
>  bpf_crypto_ctx_create(const struct bpf_crypto_params *params, u32 params__sz,
>  		      int *err)
>  {
> -	const struct bpf_crypto_type *type;
>  	struct bpf_crypto_ctx *ctx;
>
>  	if (!params || params->reserved[0] || params->reserved[1] ||
> @@ -155,69 +107,40 @@ bpf_crypto_ctx_create(const struct bpf_crypto_params *params, u32 params__sz,
>  		return NULL;
>  	}
>
[ ... ]
> +	ctx = kzalloc_obj(*ctx);
> +	if (!ctx) {
> +		*err = -ENOMEM;
> +		return NULL;
> +	}
> +
> +	if (!bpf_crypto_find_algo(params, &ctx->algo)) {
> +		*err = -ENOENT;
> +		goto out;
> +	}
> +
> +	switch (ctx->algo) {
> +	case BPF_ALGO_AES_CBC:
> +	case BPF_ALGO_AES_ECB:
> +		if (params->authsize)
> +			*err = -EOPNOTSUPP;
> +		else
> +			*err = aes_preparekey(&ctx->key.aes, params->key,
> +					      params->key_len);
> +		break;
> +	default:
> +		WARN_ON(1);
> +		*err = -ENOENT;
> +		break;
> +	}
> +
> +out:
> +	if (*err) {
> +		kfree_sensitive(ctx);
> +		return NULL;
> +	}
> +
>  	refcount_set(&ctx->usage, 1);
> -
>  	return ctx;
>  }

Does this code have a double-fetch issue? The function now uses the
caller-supplied 'int *err' out-parameter as its own internal success
flag: every failure path stores into *err, then the 'if (*err)' at the
'out:' label re-reads it to decide whether to free ctx and fail.

The 'err' parameter is classified by the verifier as KF_ARG_PTR_TO_MEM,
so a BPF program can legally pass a pointer into a shared map value
rather than its own stack. That makes the store followed by the reload a
double fetch of concurrently-writable memory.

The pre-patch version never read *err - it used distinct error labels
and returned NULL unconditionally on error, so the failure decision
could not be influenced by the caller.

If the reload observes 0 because another CPU zeroed the map value
between the store and the load, the function takes the success path for
a create that actually failed. For instance, if aes_preparekey() returns
-EINVAL from aes_check_keylen() before assigning enc_key->len and
enc_key->nrounds, the kzalloc'ed key stays all-zero with nrounds == 0.

A later bpf_crypto_encrypt()/bpf_crypto_decrypt() on that ctx reaches
aes_encrypt_generic()/aes_decrypt_generic(), which compute 'int n =
nrounds - 1' (= -1) and then run 'do { ... } while (--n);', reading 16
bytes past the round-key pointer per iteration for roughly 2^32
iterations - an unbounded out-of-bounds read and unrecoverable soft
lockup.

The reload can also mis-handle the -ENOENT path from
bpf_crypto_find_algo(), returning a ctx whose algo is uninitialized
(BPF_ALGO_AES_CBC == 0) with no key.

Could this be fixed by using a local 'int ret' for the internal flow and
assigning '*err = ret' exactly once?

[ ... ]

> @@ -86,23 +83,17 @@ struct bpf_crypto_ctx {
>   * As crypto API functions use GFP_KERNEL allocations, this function can
>   * only be used in sleepable BPF programs.

The kernel-doc for bpf_crypto_ctx_create() still justifies the
KF_SLEEPABLE requirement with "As crypto API functions use GFP_KERNEL
allocations."

This patch removes every crypto API call from the function
(crypto_alloc_lskcipher()/crypto_lskcipher_setkey() are gone). The body
now only does kzalloc_obj(), bpf_crypto_find_algo(), and
aes_preparekey(), and the AES library is documented as usable in "Any
context".

The conclusion still holds because kzalloc_obj() uses GFP_KERNEL, not
because of the crypto API. Could the comment be updated to reflect the
actual reason for the sleepable restriction after this refactoring?


---
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/33431341694

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next] bpf: crypto: Use AES-CBC and AES-ECB libraries
  2026-08-31 19:21 [PATCH bpf-next] bpf: crypto: Use AES-CBC and AES-ECB libraries Eric Biggers
  2026-08-31 20:29 ` bot+bpf-ci
@ 2026-08-31 21:25 ` Karl Mehltretter
  2026-08-31 21:48   ` Eric Biggers
  1 sibling, 1 reply; 6+ messages in thread
From: Karl Mehltretter @ 2026-08-31 21:25 UTC (permalink / raw)
  To: Eric Biggers
  Cc: bpf, Vadim Fedorenko, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	linux-crypto, linux-kernel, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
	John Fastabend

On Mon, Aug 31, 2026 at 12:21:39PM +0100, Eric Biggers wrote:
> There are library APIs for both of these now, which are much easier to
> use and more efficient.  Reimplement BPF crypto on top of them, greatly
> simplifying the code.  As part of this, the bpf_crypto_type abstraction
> layer is removed, as it's not useful.
> 

This relates to my recent patch fixing state preservation in lskcipher's
unaligned path, which exercised ARC4 through BPF:

https://lore.kernel.org/r/20260829194314.42685-1-kmehltretter@gmail.com

While looking at your patch, I tested the actual BPF-visible algorithm
surface. The ECB and CBC templates allow considerably more than AES.

I compared:

  A: cee9395acd80 (v7.3-rc1)
  B: cee9395acd80 plus this patch

both used the same arm64 QEMU configuration and the same BPF/userspace
test artifacts. Each request was exactly one cipher block.

"OK" means context creation plus BPF encrypt/decrypt round-trip succeeded.

Algorithm                     Bytes  A          B
--------------------------------------------------------
ecb(aes)                         16  OK         OK
cbc(aes)                         16  OK         OK
ecb(aes-lib)                     16  OK         -ENOENT
cbc(aes-lib)                     16  OK         -ENOENT
ecb(anubis)                      16  OK         -ENOENT
cbc(anubis)                      16  OK         -ENOENT
ecb(anubis-generic)              16  OK         -ENOENT
cbc(anubis-generic)              16  OK         -ENOENT
ecb(aria)                        16  OK         -ENOENT
cbc(aria)                        16  OK         -ENOENT
ecb(aria-generic)                16  OK         -ENOENT
cbc(aria-generic)                16  OK         -ENOENT
ecb(blowfish)                     8  OK         -ENOENT
cbc(blowfish)                     8  OK         -ENOENT
ecb(blowfish-generic)             8  OK         -ENOENT
cbc(blowfish-generic)             8  OK         -ENOENT
ecb(camellia)                    16  OK         -ENOENT
cbc(camellia)                    16  OK         -ENOENT
ecb(camellia-generic)            16  OK         -ENOENT
cbc(camellia-generic)            16  OK         -ENOENT
ecb(cast5)                        8  OK         -ENOENT
cbc(cast5)                        8  OK         -ENOENT
ecb(cast5-generic)                8  OK         -ENOENT
cbc(cast5-generic)                8  OK         -ENOENT
ecb(cast6)                       16  OK         -ENOENT
cbc(cast6)                       16  OK         -ENOENT
ecb(cast6-generic)               16  OK         -ENOENT
cbc(cast6-generic)               16  OK         -ENOENT
ecb(des)                          8  OK         -ENOENT
cbc(des)                          8  OK         -ENOENT
ecb(des-generic)                  8  OK         -ENOENT
cbc(des-generic)                  8  OK         -ENOENT
ecb(des3_ede)                     8  OK         -ENOENT
cbc(des3_ede)                     8  OK         -ENOENT
ecb(des3_ede-generic)             8  OK         -ENOENT
cbc(des3_ede-generic)             8  OK         -ENOENT
ecb(khazad)                       8  OK         -ENOENT
cbc(khazad)                       8  OK         -ENOENT
ecb(khazad-generic)               8  OK         -ENOENT
cbc(khazad-generic)               8  OK         -ENOENT
ecb(seed)                        16  OK         -ENOENT
cbc(seed)                        16  OK         -ENOENT
ecb(seed-generic)                16  OK         -ENOENT
cbc(seed-generic)                16  OK         -ENOENT
ecb(serpent)                     16  OK         -ENOENT
cbc(serpent)                     16  OK         -ENOENT
ecb(serpent-generic)             16  OK         -ENOENT
cbc(serpent-generic)             16  OK         -ENOENT
ecb(sm4)                         16  OK         -ENOENT
cbc(sm4)                         16  OK         -ENOENT
ecb(sm4-generic)                 16  OK         -ENOENT
cbc(sm4-generic)                 16  OK         -ENOENT
ecb(tea)                          8  OK         -ENOENT
cbc(tea)                          8  OK         -ENOENT
ecb(tea-generic)                  8  OK         -ENOENT
cbc(tea-generic)                  8  OK         -ENOENT
ecb(xtea)                         8  OK         -ENOENT
cbc(xtea)                         8  OK         -ENOENT
ecb(xtea-generic)                 8  OK         -ENOENT
cbc(xtea-generic)                 8  OK         -ENOENT
ecb(xeta)                         8  OK         -ENOENT
cbc(xeta)                         8  OK         -ENOENT
ecb(xeta-generic)                 8  OK         -ENOENT
cbc(xeta-generic)                 8  OK         -ENOENT
ecb(twofish)                     16  OK         -ENOENT
cbc(twofish)                     16  OK         -ENOENT
ecb(twofish-generic)             16  OK         -ENOENT
cbc(twofish-generic)             16  OK         -ENOENT
arc4                              1  OK         -ENOENT
arc4-generic                      1  OK         -ENOENT
ecb(arc4)                         1  OK         -ENOENT
ecb(arc4-generic)                 1  OK         -ENOENT

This leaves only 2 of the 72 tested algorithm names: the other 70,
covering AES aliases, non-AES block ciphers, and ARC4, now return
-ENOENT.

This table describes the BPF-visible support removed by this patch. That
may well be worthwhile cleanup, but it is still the removal of existing,
likely only theoretical, support.

Thanks,
Karl

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next] bpf: crypto: Use AES-CBC and AES-ECB libraries
  2026-08-31 21:25 ` Karl Mehltretter
@ 2026-08-31 21:48   ` Eric Biggers
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Biggers @ 2026-08-31 21:48 UTC (permalink / raw)
  To: Karl Mehltretter
  Cc: bpf, Vadim Fedorenko, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	linux-crypto, linux-kernel, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
	John Fastabend

On Mon, Aug 31, 2026 at 11:25:54PM +0200, Karl Mehltretter wrote:
> On Mon, Aug 31, 2026 at 12:21:39PM +0100, Eric Biggers wrote:
> > There are library APIs for both of these now, which are much easier to
> > use and more efficient.  Reimplement BPF crypto on top of them, greatly
> > simplifying the code.  As part of this, the bpf_crypto_type abstraction
> > layer is removed, as it's not useful.
> > 
> 
> This relates to my recent patch fixing state preservation in lskcipher's
> unaligned path, which exercised ARC4 through BPF:
> 
> https://lore.kernel.org/r/20260829194314.42685-1-kmehltretter@gmail.com
> 
> While looking at your patch, I tested the actual BPF-visible algorithm
> surface. The ECB and CBC templates allow considerably more than AES.

Right, apparently any "crypto_cipher" can be composed with "ecb" or
"cbc" as an "lskcipher".  I think in this case it's only of theoretical
interest and is a bug, not a feature, though.  Especially given the
presence of ARC4, DES, TEA, etc. on that list.

We see this a lot with the "Crypto API", where a new kernel feature
actually uses one or two algorithms, then unnecessarily allows every
single algorithm to be theoretically reachable (including insecure,
obsolete, or nonsense options) just because the API takes a string.

- Eric

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next] bpf: crypto: Use AES-CBC and AES-ECB libraries
  2026-08-31 20:29 ` bot+bpf-ci
@ 2026-08-31 21:59   ` Eric Biggers
  2026-08-31 22:43     ` Eric Biggers
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Biggers @ 2026-08-31 21:59 UTC (permalink / raw)
  To: bot+bpf-ci
  Cc: bpf, vadim.fedorenko, ast, daniel, andrii, eddyz87, memxor,
	linux-crypto, linux-kernel, martin.lau, song, yonghong.song,
	jolsa, emil, ihor.solodrai, john.fastabend, kmehltretter,
	martin.lau, mason

On Mon, Aug 31, 2026 at 08:29:02PM +0000, bot+bpf-ci@kernel.org wrote:
> The claim that lskcipher supports only these three specific algorithm
> names is not accurate. The cbc and ecb templates in crypto/cbc.c and
> crypto/ecb.c instantiate lskcipher interfaces for any plain cipher
> algorithm, not just AES:
> 
> crypto/cbc.c:crypto_cbc_create() calls lskcipher_alloc_instance_simple()
> crypto/ecb.c:crypto_ecb_create() calls lskcipher_alloc_instance_simple()
> 
> So cbc(<any-cipher>) and ecb(<any-cipher>) are valid lskcipher
> algorithms. The kernel includes about 14 plain cipher algorithms besides
> AES, including sm4, des, des3_ede, camellia, twofish, aria, blowfish,
> cast5, cast6, serpent, seed, khazad, anubis, tea, and xtea.
> 
> The pre-patch code passed params->algo directly to
> crypto_alloc_lskcipher() with no whitelist, so a BPF program could use
> cbc(sm4), cbc(des3_ede), ecb(camellia), or ecb(twofish).
> 
> After this patch, bpf_crypto_find_algo() only matches the two-entry
> table with cbc(aes) and ecb(aes), so all non-AES cbc/ecb modes now fail.
> 
> The changelog justifies dropping ARC4 but does not mention that this
> also drops support for all non-AES block ciphers. Should the commit
> message explicitly state that non-AES cbc()/ecb() support is being
> removed, or should the characterization of lskcipher's scope be
> corrected?

I'll update the commit message to clarify that other block ciphers are
dropped as well.

> Additionally, the error code for an unsupported algorithm changes from
> -EOPNOTSUPP (old has_algo failure path) to -ENOENT (new
> bpf_crypto_find_algo() path), which is a visible behavior change for
> existing BPF programs that inspect errno.

I'll change it to EOPNOTSUPP.

> > +config BPF_CRYPTO
> > +	def_bool y
> > +	depends on BPF_SYSCALL
> > +	select CRYPTO_LIB_AES_CBC
> > +	select CRYPTO_LIB_AES_ECB
> > +	help
> > +	  Provide the kfuncs needed for BPF programs to encrypt and decrypt
> > +	  data. The supported algorithms are AES-CBC and AES-ECB.
> > +
> 
> The def_bool y makes BPF_CRYPTO invisible and unconditionally enabled
> whenever CONFIG_BPF_SYSCALL=y, so the AES library code is always linked
> with no opt-out.
> 
> Before this patch, kernel/bpf/Makefile gated crypto.o on:
> 
>   ifneq ($(CONFIG_CRYPTO),)
>   obj-$(CONFIG_BPF_SYSCALL) += crypto.o
>   endif
> 
> so a CONFIG_BPF_SYSCALL=y, CONFIG_CRYPTO=n kernel previously built
> neither the BPF crypto kfuncs nor any AES code. Now the AES
> key-schedule, CBC, and ECB library code is unconditionally present.
> 
> This is particularly notable given that the commit message says AES-ECB
> "isn't appropriate in new systems and should be dropped" yet it is now
> unconditionally linked into every BPF_SYSCALL kernel.
> 
> Should this be a user-selectable option with a prompt like "bool 'BPF
> crypto kfuncs'" so it can be disabled, or is the intent really to make
> it always-on?

Almost every kernel has CRYPTO and CRYPTO_SKCIPHER2 enabled anyway, and
CRYPTO_SKCIPHER2 also enables ECB support.  So the change isn't actually
as drastic as the bot implies, but I'd be glad to give BPF_CRYPTO a
prompt if the BPF folks are okay with that.

> Deleting this file removes the last in-tree caller of
> crypto_alloc_lskcipher(). After this patch, the symbol only exists at
> its definition and export in crypto/lskcipher.c, with no remaining
> callers anywhere in the tree.
> 
> Given the premise that the lskcipher API "doesn't seem to be going
> anywhere," should crypto_alloc_lskcipher() be removed in a follow-up, or
> left as an exported API with zero users?

Yes, lskcipher should be deleted in a follow-up series.

> Does this code have a double-fetch issue? The function now uses the
> caller-supplied 'int *err' out-parameter as its own internal success
> flag: every failure path stores into *err, then the 'if (*err)' at the
> 'out:' label re-reads it to decide whether to free ctx and fail.
> 
> The 'err' parameter is classified by the verifier as KF_ARG_PTR_TO_MEM,
> so a BPF program can legally pass a pointer into a shared map value
> rather than its own stack. That makes the store followed by the reload a
> double fetch of concurrently-writable memory.
> 
> The pre-patch version never read *err - it used distinct error labels
> and returned NULL unconditionally on error, so the failure decision
> could not be influenced by the caller.
> 
> If the reload observes 0 because another CPU zeroed the map value
> between the store and the load, the function takes the success path for
> a create that actually failed. For instance, if aes_preparekey() returns
> -EINVAL from aes_check_keylen() before assigning enc_key->len and
> enc_key->nrounds, the kzalloc'ed key stays all-zero with nrounds == 0.
> 
> A later bpf_crypto_encrypt()/bpf_crypto_decrypt() on that ctx reaches
> aes_encrypt_generic()/aes_decrypt_generic(), which compute 'int n =
> nrounds - 1' (= -1) and then run 'do { ... } while (--n);', reading 16
> bytes past the round-key pointer per iteration for roughly 2^32
> iterations - an unbounded out-of-bounds read and unrecoverable soft
> lockup.
> 
> The reload can also mis-handle the -ENOENT path from
> bpf_crypto_find_algo(), returning a ctx whose algo is uninitialized
> (BPF_ALGO_AES_CBC == 0) with no key.
> 
> Could this be fixed by using a local 'int ret' for the internal flow and
> assigning '*err = ret' exactly once?

I'll change it to use a local variable.

> > @@ -86,23 +83,17 @@ struct bpf_crypto_ctx {
> >   * As crypto API functions use GFP_KERNEL allocations, this function can
> >   * only be used in sleepable BPF programs.
> 
> The kernel-doc for bpf_crypto_ctx_create() still justifies the
> KF_SLEEPABLE requirement with "As crypto API functions use GFP_KERNEL
> allocations."
> 
> This patch removes every crypto API call from the function
> (crypto_alloc_lskcipher()/crypto_lskcipher_setkey() are gone). The body
> now only does kzalloc_obj(), bpf_crypto_find_algo(), and
> aes_preparekey(), and the AES library is documented as usable in "Any
> context".
> 
> The conclusion still holds because kzalloc_obj() uses GFP_KERNEL, not
> because of the crypto API. Could the comment be updated to reflect the
> actual reason for the sleepable restriction after this refactoring?

I'll update the comment.

- Eric

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next] bpf: crypto: Use AES-CBC and AES-ECB libraries
  2026-08-31 21:59   ` Eric Biggers
@ 2026-08-31 22:43     ` Eric Biggers
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Biggers @ 2026-08-31 22:43 UTC (permalink / raw)
  To: bot+bpf-ci
  Cc: bpf, vadim.fedorenko, ast, daniel, andrii, eddyz87, memxor,
	linux-crypto, linux-kernel, martin.lau, song, yonghong.song,
	jolsa, emil, ihor.solodrai, john.fastabend, kmehltretter,
	martin.lau, mason

On Mon, Aug 31, 2026 at 02:59:39PM -0700, Eric Biggers wrote:
> Almost every kernel has CRYPTO and CRYPTO_SKCIPHER2 enabled anyway, and
> CRYPTO_SKCIPHER2 also enables ECB support.  So the change isn't actually
> as drastic as the bot implies, but I'd be glad to give BPF_CRYPTO a
> prompt if the BPF folks are okay with that.

I don't see other examples of kfuncs with visible kconfig options.  The
convention is to automatically enable them when the parent subsystem is
enabled.

It feels like bpf crypto is a de facto standalone feature, so I'm not
sure how well that applies here.  Regardless, for now I'll use "depends
on CRYPTO_LIB_AES_CBC" and "depends on CRYPTO_LIB_AES_ECB".

- Eric

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-31 22:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-31 19:21 [PATCH bpf-next] bpf: crypto: Use AES-CBC and AES-ECB libraries Eric Biggers
2026-08-31 20:29 ` bot+bpf-ci
2026-08-31 21:59   ` Eric Biggers
2026-08-31 22:43     ` Eric Biggers
2026-08-31 21:25 ` Karl Mehltretter
2026-08-31 21:48   ` Eric Biggers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox