| Message ID | 20231202010604.1877561-1-vadfed@meta.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | [bpf-next,v7,1/3] bpf: make common crypto API for TC/XDP programs | expand |
On 12/1/23 5:06 PM, Vadim Fedorenko wrote: > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index eb447b0a9423..0143ff6c93a1 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -1228,6 +1228,7 @@ int bpf_dynptr_check_size(u32 size); > u32 __bpf_dynptr_size(const struct bpf_dynptr_kern *ptr); > const void *__bpf_dynptr_data(const struct bpf_dynptr_kern *ptr, u32 len); > void *__bpf_dynptr_data_rw(const struct bpf_dynptr_kern *ptr, u32 len); > +bool __bpf_dynptr_is_rdonly(const struct bpf_dynptr_kern *ptr); > > #ifdef CONFIG_BPF_JIT > int bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr); > diff --git a/include/linux/bpf_crypto.h b/include/linux/bpf_crypto.h > new file mode 100644 > index 000000000000..e81bd8ab979c > --- /dev/null > +++ b/include/linux/bpf_crypto.h > @@ -0,0 +1,23 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* Copyright (c) 2023 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); > + u32 (*get_flags)(void *tfm); > + struct module *owner; > + char name[14]; Does it have a macro (from crypto ?) that can be reused here instead of a numeric constant? > +}; > + > +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/Makefile b/kernel/bpf/Makefile > index f526b7573e97..bcde762bb2c2 100644 > --- a/kernel/bpf/Makefile > +++ b/kernel/bpf/Makefile > @@ -41,6 +41,9 @@ obj-$(CONFIG_BPF_SYSCALL) += bpf_struct_ops.o > obj-$(CONFIG_BPF_SYSCALL) += cpumask.o > obj-${CONFIG_BPF_LSM} += bpf_lsm.o > endif > +ifeq ($(CONFIG_CRYPTO),y) > +obj-$(CONFIG_BPF_SYSCALL) += crypto.o > +endif > obj-$(CONFIG_BPF_PRELOAD) += preload/ > > obj-$(CONFIG_BPF_SYSCALL) += relo_core.o > diff --git a/kernel/bpf/crypto.c b/kernel/bpf/crypto.c > new file mode 100644 > index 000000000000..a1e543d1d7fe > --- /dev/null > +++ b/kernel/bpf/crypto.c > @@ -0,0 +1,364 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* Copyright (c) 2023 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; > +}; > + > +static LIST_HEAD(bpf_crypto_types); > +static DECLARE_RWSEM(bpf_crypto_types_sem); > + > +/** > + * 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. > + * @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; > + 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 = -EEXIST; > + > + 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(sizeof(*node), GFP_KERNEL); > + 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)) If I read patch 2 correctly, it is always built-in. I am not sure I understand the module_put/get in this patch. > + type = node->type; > + break; > + } > + up_read(&bpf_crypto_types_sem); > + > + return type; > +} > + > +__bpf_kfunc_start_defs(); > + > +/** > + * bpf_crypto_ctx_create() - Create a mutable BPF crypto context. > + * > + * Allocates a crypto context that can be used, acquired, and released by > + * a BPF program. The crypto context returned by this function must either > + * be embedded in a map as a kptr, or freed with bpf_crypto_ctx_release(). > + * As crypto API functions use GFP_KERNEL allocations, this function can > + * only be used in sleepable BPF programs. > + * > + * bpf_crypto_ctx_create() allocates memory for crypto context. > + * It may return NULL if no memory is available. > + * @type__str: pointer to string representation of crypto type. > + * @algo__str: pointer to string representation of algorithm. > + * @pkey: bpf_dynptr which holds cipher key to do crypto. > + * @err: integer to store error code when NULL is returned > + */ > +__bpf_kfunc struct bpf_crypto_ctx * > +bpf_crypto_ctx_create(const char *type__str, const char *algo__str, > + const struct bpf_dynptr_kern *pkey, > + unsigned int authsize, int *err) > +{ > + const struct bpf_crypto_type *type = bpf_crypto_get_type(type__str); > + struct bpf_crypto_ctx *ctx; > + const u8 *key; > + u32 key_len; > + > + type = bpf_crypto_get_type(type__str); > + if (IS_ERR(type)) { > + *err = PTR_ERR(type); > + return NULL; > + } > + > + if (!type->has_algo(algo__str)) { > + *err = -EOPNOTSUPP; > + goto err; ctx is still not initialized. The error path will crash. > + } > + > + if (!authsize && type->setauthsize) { > + *err = -EOPNOTSUPP; > + goto err; > + } > + > + if (authsize && !type->setauthsize) { > + *err = -EOPNOTSUPP; > + goto err; > + } > + > + key_len = __bpf_dynptr_size(pkey); > + if (!key_len) { > + *err = -EINVAL; > + goto err; > + } > + key = __bpf_dynptr_data(pkey, key_len); > + if (!key) { > + *err = -EINVAL; > + goto err; > + } > + > + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); > + if (!ctx) { > + *err = -ENOMEM; > + goto err; > + } > + > + ctx->type = type; > + ctx->tfm = type->alloc_tfm(algo__str); > + if (IS_ERR(ctx->tfm)) { > + *err = PTR_ERR(ctx->tfm); > + ctx->tfm = NULL; > + goto err; > + } > + > + if (authsize) { > + *err = type->setauthsize(ctx->tfm, authsize); > + if (*err) > + goto err; > + } > + > + *err = type->setkey(ctx->tfm, key, key_len); > + if (*err) > + goto err; > + > + refcount_set(&ctx->usage, 1); > + > + return ctx; > +err: > + if (ctx->tfm) > + type->free_tfm(ctx->tfm); > + kfree(ctx); > + module_put(type->owner); > + > + return NULL; > +} > + > +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); > +} > + > +/** > + * bpf_crypto_ctx_acquire() - Acquire a reference to a BPF crypto context. > + * @ctx: The BPF crypto context being acquired. The ctx must be a trusted > + * pointer. > + * > + * Acquires a reference to a BPF crypto context. The context returned by this function > + * must either be embedded in a map as a kptr, or freed with > + * bpf_crypto_skcipher_ctx_release(). > + */ > +__bpf_kfunc struct bpf_crypto_ctx * > +bpf_crypto_ctx_acquire(struct bpf_crypto_ctx *ctx) > +{ > + refcount_inc(&ctx->usage); > + return ctx; > +} > + > +/** > + * bpf_crypto_ctx_release() - Release a previously acquired BPF crypto context. > + * @ctx: The crypto context being released. > + * > + * Releases a previously acquired reference to a BPF crypto context. When the final > + * reference of the BPF crypto context has been released, it is subsequently freed in > + * an RCU callback in the BPF memory allocator. > + */ > +__bpf_kfunc void bpf_crypto_ctx_release(struct bpf_crypto_ctx *ctx) > +{ > + if (refcount_dec_and_test(&ctx->usage)) > + call_rcu(&ctx->rcu, crypto_free_cb); > +} > + > +static int bpf_crypto_crypt(const struct bpf_crypto_ctx *ctx, > + const struct bpf_dynptr_kern *src, > + struct bpf_dynptr_kern *dst, > + const struct bpf_dynptr_kern *iv, > + bool decrypt) > +{ > + u32 src_len, dst_len, iv_len; > + const u8 *psrc; > + u8 *pdst, *piv; > + int err; > + > + if (ctx->type->get_flags(ctx->tfm) & CRYPTO_TFM_NEED_KEY) > + return -EINVAL; > + > + if (__bpf_dynptr_is_rdonly(dst)) > + return -EINVAL; > + > + iv_len = __bpf_dynptr_size(iv); > + src_len = __bpf_dynptr_size(src); > + dst_len = __bpf_dynptr_size(dst); > + if (!src_len || !dst_len) > + return -EINVAL; > + > + if (iv_len != ctx->type->ivsize(ctx->tfm)) > + return -EINVAL; > + > + psrc = __bpf_dynptr_data(src, src_len); > + if (!psrc) > + return -EINVAL; > + pdst = __bpf_dynptr_data_rw(dst, dst_len); > + if (!pdst) > + return -EINVAL; > + > + 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; > +} > + > +/** > + * bpf_crypto_decrypt() - Decrypt buffer using configured context and IV provided. > + * @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. > + * @iv: bpf_dynptr to IV data to be used by decryptor. > + * > + * 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_kern *src, > + struct bpf_dynptr_kern *dst, > + struct bpf_dynptr_kern *iv) > +{ > + return bpf_crypto_crypt(ctx, src, dst, iv, true); > +} > + > +/** > + * bpf_crypto_encrypt() - Encrypt buffer using configured context and IV provided. > + * @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 buffer where to store the result. Must be a trusted pointer. > + * @iv: bpf_dynptr to IV data to be used by decryptor. > + * > + * 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_kern *src, > + struct bpf_dynptr_kern *dst, > + struct bpf_dynptr_kern *iv) > +{ > + return bpf_crypto_crypt(ctx, src, dst, iv, false); > +} > + > +__bpf_kfunc_end_defs(); > + > +BTF_SET8_START(crypt_init_kfunc_btf_ids) > +BTF_ID_FLAGS(func, bpf_crypto_ctx_create, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE) > +BTF_ID_FLAGS(func, bpf_crypto_ctx_release, KF_RELEASE) > +BTF_ID_FLAGS(func, bpf_crypto_ctx_acquire, KF_ACQUIRE | KF_TRUSTED_ARGS) Considering bpf_crypto_ctx is rcu protected, the acquire may use "KF_ACQUIRE | KF_RCU | KF_RET_NULL" such that the bpf_crypto_ctx_acquire(ctx_from_map_value) will work and the user will prepare checking NULL from day one. > +BTF_SET8_END(crypt_init_kfunc_btf_ids) > + > +static const struct btf_kfunc_id_set crypt_init_kfunc_set = { > + .owner = THIS_MODULE, > + .set = &crypt_init_kfunc_btf_ids, > +}; > + > +BTF_SET8_START(crypt_kfunc_btf_ids) > +BTF_ID_FLAGS(func, bpf_crypto_decrypt, KF_RCU) > +BTF_ID_FLAGS(func, bpf_crypto_encrypt, KF_RCU) > +BTF_SET8_END(crypt_kfunc_btf_ids) > + > +static const struct btf_kfunc_id_set crypt_kfunc_set = { > + .owner = THIS_MODULE, > + .set = &crypt_kfunc_btf_ids, > +}; > + > +BTF_ID_LIST(bpf_crypto_dtor_ids) > +BTF_ID(struct, bpf_crypto_ctx) > +BTF_ID(func, bpf_crypto_ctx_release) > + > +static int __init crypto_kfunc_init(void) > +{ > + int ret; > + const struct btf_id_dtor_kfunc bpf_crypto_dtors[] = { > + { > + .btf_id = bpf_crypto_dtor_ids[0], > + .kfunc_btf_id = bpf_crypto_dtor_ids[1] > + }, > + }; > + > + ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &crypt_kfunc_set); > + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_ACT, &crypt_kfunc_set); > + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &crypt_kfunc_set); > + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_UNSPEC, > + &crypt_init_kfunc_set); > + return ret ?: register_btf_id_dtor_kfuncs(bpf_crypto_dtors, > + ARRAY_SIZE(bpf_crypto_dtors), > + THIS_MODULE); > +} > + > +late_initcall(crypto_kfunc_init); > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > index b45a8381f9bd..b73314c0124e 100644 > --- a/kernel/bpf/helpers.c > +++ b/kernel/bpf/helpers.c > @@ -1436,7 +1436,7 @@ static const struct bpf_func_proto bpf_kptr_xchg_proto = { > #define DYNPTR_SIZE_MASK 0xFFFFFF > #define DYNPTR_RDONLY_BIT BIT(31) > > -static bool __bpf_dynptr_is_rdonly(const struct bpf_dynptr_kern *ptr) > +bool __bpf_dynptr_is_rdonly(const struct bpf_dynptr_kern *ptr) > { > return ptr->size & DYNPTR_RDONLY_BIT; > } > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 8e7b6072e3f4..c54716966d5d 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -5162,6 +5162,7 @@ BTF_ID(struct, cgroup) > #endif > BTF_ID(struct, bpf_cpumask) > BTF_ID(struct, task_struct) > +BTF_ID(struct, bpf_crypto_ctx) > BTF_SET_END(rcu_protected_types) > > static bool rcu_protected_object(const struct btf *btf, u32 btf_id)
On Fri, Dec 01, 2023 at 05:06:02PM -0800, Vadim Fedorenko wrote: > Add crypto API support to BPF to be able to decrypt or encrypt packets > in TC/XDP BPF programs. Special care should be taken for initialization > part of crypto algo because crypto alloc) doesn't work with preemtion > disabled, it can be run only in sleepable BPF program. Also async crypto > is not supported because of the very same issue - TC/XDP BPF programs > are not sleepable. > > Signed-off-by: Vadim Fedorenko <vadfed@meta.com> ... > +/** > + * bpf_crypto_ctx_create() - Create a mutable BPF crypto context. > + * > + * Allocates a crypto context that can be used, acquired, and released by > + * a BPF program. The crypto context returned by this function must either > + * be embedded in a map as a kptr, or freed with bpf_crypto_ctx_release(). > + * As crypto API functions use GFP_KERNEL allocations, this function can > + * only be used in sleepable BPF programs. > + * > + * bpf_crypto_ctx_create() allocates memory for crypto context. > + * It may return NULL if no memory is available. > + * @type__str: pointer to string representation of crypto type. > + * @algo__str: pointer to string representation of algorithm. > + * @pkey: bpf_dynptr which holds cipher key to do crypto. Hi Vadim, a minor nit from my side: something about @authsize should go here. > + * @err: integer to store error code when NULL is returned > + */ > +__bpf_kfunc struct bpf_crypto_ctx * > +bpf_crypto_ctx_create(const char *type__str, const char *algo__str, > + const struct bpf_dynptr_kern *pkey, > + unsigned int authsize, int *err) ...
On 02.12.2023 01:48, Martin KaFai Lau wrote: > On 12/1/23 5:06 PM, Vadim Fedorenko wrote: >> diff --git a/include/linux/bpf.h b/include/linux/bpf.h >> index eb447b0a9423..0143ff6c93a1 100644 >> --- a/include/linux/bpf.h >> +++ b/include/linux/bpf.h >> @@ -1228,6 +1228,7 @@ int bpf_dynptr_check_size(u32 size); >> u32 __bpf_dynptr_size(const struct bpf_dynptr_kern *ptr); >> const void *__bpf_dynptr_data(const struct bpf_dynptr_kern *ptr, u32 len); >> void *__bpf_dynptr_data_rw(const struct bpf_dynptr_kern *ptr, u32 len); >> +bool __bpf_dynptr_is_rdonly(const struct bpf_dynptr_kern *ptr); >> #ifdef CONFIG_BPF_JIT >> int bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct >> bpf_trampoline *tr); >> diff --git a/include/linux/bpf_crypto.h b/include/linux/bpf_crypto.h >> new file mode 100644 >> index 000000000000..e81bd8ab979c >> --- /dev/null >> +++ b/include/linux/bpf_crypto.h >> @@ -0,0 +1,23 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* Copyright (c) 2023 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); >> + u32 (*get_flags)(void *tfm); >> + struct module *owner; >> + char name[14]; > > Does it have a macro (from crypto ?) that can be reused here instead of a > numeric constant? > I have checked AF_ALG which uses the same way and there is no constant unfortunately: https://elixir.bootlin.com/linux/v6.7-rc3/source/include/crypto/if_alg.h#L55 Maybe Herbert will suggest some constant instead? >> +}; >> + >> +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/Makefile b/kernel/bpf/Makefile >> index f526b7573e97..bcde762bb2c2 100644 >> --- a/kernel/bpf/Makefile >> +++ b/kernel/bpf/Makefile >> @@ -41,6 +41,9 @@ obj-$(CONFIG_BPF_SYSCALL) += bpf_struct_ops.o >> obj-$(CONFIG_BPF_SYSCALL) += cpumask.o >> obj-${CONFIG_BPF_LSM} += bpf_lsm.o >> endif >> +ifeq ($(CONFIG_CRYPTO),y) >> +obj-$(CONFIG_BPF_SYSCALL) += crypto.o >> +endif >> obj-$(CONFIG_BPF_PRELOAD) += preload/ >> obj-$(CONFIG_BPF_SYSCALL) += relo_core.o >> diff --git a/kernel/bpf/crypto.c b/kernel/bpf/crypto.c >> new file mode 100644 >> index 000000000000..a1e543d1d7fe >> --- /dev/null >> +++ b/kernel/bpf/crypto.c >> @@ -0,0 +1,364 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* Copyright (c) 2023 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; >> +}; >> + >> +static LIST_HEAD(bpf_crypto_types); >> +static DECLARE_RWSEM(bpf_crypto_types_sem); >> + >> +/** >> + * 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. >> + * @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; >> + 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 = -EEXIST; >> + >> + 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(sizeof(*node), GFP_KERNEL); >> + 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)) > > If I read patch 2 correctly, it is always built-in. I am not sure I understand > the module_put/get in this patch. Well, yeah, right now it's built-in, but it can be easily converted to module with it's own Kconfig option. Especially if we think about adding aead crypto and using bpf in embedded setups with less amount of resources. >> + type = node->type; >> + break; >> + } >> + up_read(&bpf_crypto_types_sem); >> + >> + return type; >> +} >> + >> +__bpf_kfunc_start_defs(); >> + >> +/** >> + * bpf_crypto_ctx_create() - Create a mutable BPF crypto context. >> + * >> + * Allocates a crypto context that can be used, acquired, and released by >> + * a BPF program. The crypto context returned by this function must either >> + * be embedded in a map as a kptr, or freed with bpf_crypto_ctx_release(). >> + * As crypto API functions use GFP_KERNEL allocations, this function can >> + * only be used in sleepable BPF programs. >> + * >> + * bpf_crypto_ctx_create() allocates memory for crypto context. >> + * It may return NULL if no memory is available. >> + * @type__str: pointer to string representation of crypto type. >> + * @algo__str: pointer to string representation of algorithm. >> + * @pkey: bpf_dynptr which holds cipher key to do crypto. >> + * @err: integer to store error code when NULL is returned >> + */ >> +__bpf_kfunc struct bpf_crypto_ctx * >> +bpf_crypto_ctx_create(const char *type__str, const char *algo__str, >> + const struct bpf_dynptr_kern *pkey, >> + unsigned int authsize, int *err) >> +{ >> + const struct bpf_crypto_type *type = bpf_crypto_get_type(type__str); >> + struct bpf_crypto_ctx *ctx; >> + const u8 *key; >> + u32 key_len; >> + >> + type = bpf_crypto_get_type(type__str); >> + if (IS_ERR(type)) { >> + *err = PTR_ERR(type); >> + return NULL; >> + } >> + >> + if (!type->has_algo(algo__str)) { >> + *err = -EOPNOTSUPP; >> + goto err; > > ctx is still not initialized. The error path will crash. good catch, thanks! >> + } >> + >> + if (!authsize && type->setauthsize) { >> + *err = -EOPNOTSUPP; >> + goto err; >> + } >> + >> + if (authsize && !type->setauthsize) { >> + *err = -EOPNOTSUPP; >> + goto err; >> + } >> + >> + key_len = __bpf_dynptr_size(pkey); >> + if (!key_len) { >> + *err = -EINVAL; >> + goto err; >> + } >> + key = __bpf_dynptr_data(pkey, key_len); >> + if (!key) { >> + *err = -EINVAL; >> + goto err; >> + } >> + >> + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); >> + if (!ctx) { >> + *err = -ENOMEM; >> + goto err; >> + } >> + >> + ctx->type = type; >> + ctx->tfm = type->alloc_tfm(algo__str); >> + if (IS_ERR(ctx->tfm)) { >> + *err = PTR_ERR(ctx->tfm); >> + ctx->tfm = NULL; >> + goto err; >> + } >> + >> + if (authsize) { >> + *err = type->setauthsize(ctx->tfm, authsize); >> + if (*err) >> + goto err; >> + } >> + >> + *err = type->setkey(ctx->tfm, key, key_len); >> + if (*err) >> + goto err; >> + >> + refcount_set(&ctx->usage, 1); >> + >> + return ctx; >> +err: >> + if (ctx->tfm) >> + type->free_tfm(ctx->tfm); >> + kfree(ctx); >> + module_put(type->owner); >> + >> + return NULL; >> +} >> + >> +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); >> +} >> + >> +/** >> + * bpf_crypto_ctx_acquire() - Acquire a reference to a BPF crypto context. >> + * @ctx: The BPF crypto context being acquired. The ctx must be a trusted >> + * pointer. >> + * >> + * Acquires a reference to a BPF crypto context. The context returned by this >> function >> + * must either be embedded in a map as a kptr, or freed with >> + * bpf_crypto_skcipher_ctx_release(). >> + */ >> +__bpf_kfunc struct bpf_crypto_ctx * >> +bpf_crypto_ctx_acquire(struct bpf_crypto_ctx *ctx) >> +{ >> + refcount_inc(&ctx->usage); >> + return ctx; >> +} >> + >> +/** >> + * bpf_crypto_ctx_release() - Release a previously acquired BPF crypto context. >> + * @ctx: The crypto context being released. >> + * >> + * Releases a previously acquired reference to a BPF crypto context. When the >> final >> + * reference of the BPF crypto context has been released, it is subsequently >> freed in >> + * an RCU callback in the BPF memory allocator. >> + */ >> +__bpf_kfunc void bpf_crypto_ctx_release(struct bpf_crypto_ctx *ctx) >> +{ >> + if (refcount_dec_and_test(&ctx->usage)) >> + call_rcu(&ctx->rcu, crypto_free_cb); >> +} >> + >> +static int bpf_crypto_crypt(const struct bpf_crypto_ctx *ctx, >> + const struct bpf_dynptr_kern *src, >> + struct bpf_dynptr_kern *dst, >> + const struct bpf_dynptr_kern *iv, >> + bool decrypt) >> +{ >> + u32 src_len, dst_len, iv_len; >> + const u8 *psrc; >> + u8 *pdst, *piv; >> + int err; >> + >> + if (ctx->type->get_flags(ctx->tfm) & CRYPTO_TFM_NEED_KEY) >> + return -EINVAL; >> + >> + if (__bpf_dynptr_is_rdonly(dst)) >> + return -EINVAL; >> + >> + iv_len = __bpf_dynptr_size(iv); >> + src_len = __bpf_dynptr_size(src); >> + dst_len = __bpf_dynptr_size(dst); >> + if (!src_len || !dst_len) >> + return -EINVAL; >> + >> + if (iv_len != ctx->type->ivsize(ctx->tfm)) >> + return -EINVAL; >> + >> + psrc = __bpf_dynptr_data(src, src_len); >> + if (!psrc) >> + return -EINVAL; >> + pdst = __bpf_dynptr_data_rw(dst, dst_len); >> + if (!pdst) >> + return -EINVAL; >> + >> + 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; >> +} >> + >> +/** >> + * bpf_crypto_decrypt() - Decrypt buffer using configured context and IV >> provided. >> + * @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. >> + * @iv: bpf_dynptr to IV data to be used by decryptor. >> + * >> + * 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_kern *src, >> + struct bpf_dynptr_kern *dst, >> + struct bpf_dynptr_kern *iv) >> +{ >> + return bpf_crypto_crypt(ctx, src, dst, iv, true); >> +} >> + >> +/** >> + * bpf_crypto_encrypt() - Encrypt buffer using configured context and IV >> provided. >> + * @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 buffer where to store the result. Must be a trusted >> pointer. >> + * @iv: bpf_dynptr to IV data to be used by decryptor. >> + * >> + * 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_kern *src, >> + struct bpf_dynptr_kern *dst, >> + struct bpf_dynptr_kern *iv) >> +{ >> + return bpf_crypto_crypt(ctx, src, dst, iv, false); >> +} >> + >> +__bpf_kfunc_end_defs(); >> + >> +BTF_SET8_START(crypt_init_kfunc_btf_ids) >> +BTF_ID_FLAGS(func, bpf_crypto_ctx_create, KF_ACQUIRE | KF_RET_NULL | >> KF_SLEEPABLE) >> +BTF_ID_FLAGS(func, bpf_crypto_ctx_release, KF_RELEASE) >> +BTF_ID_FLAGS(func, bpf_crypto_ctx_acquire, KF_ACQUIRE | KF_TRUSTED_ARGS) > > Considering bpf_crypto_ctx is rcu protected, the acquire may use "KF_ACQUIRE | > KF_RCU | KF_RET_NULL" such that the bpf_crypto_ctx_acquire(ctx_from_map_value) > will work and the user will prepare checking NULL from day one. > Got it. What about create? Should it also include KF_RCU?
On 03.12.2023 10:57, Simon Horman wrote: > On Fri, Dec 01, 2023 at 05:06:02PM -0800, Vadim Fedorenko wrote: >> Add crypto API support to BPF to be able to decrypt or encrypt packets >> in TC/XDP BPF programs. Special care should be taken for initialization >> part of crypto algo because crypto alloc) doesn't work with preemtion >> disabled, it can be run only in sleepable BPF program. Also async crypto >> is not supported because of the very same issue - TC/XDP BPF programs >> are not sleepable. >> >> Signed-off-by: Vadim Fedorenko <vadfed@meta.com> > > ... > >> +/** >> + * bpf_crypto_ctx_create() - Create a mutable BPF crypto context. >> + * >> + * Allocates a crypto context that can be used, acquired, and released by >> + * a BPF program. The crypto context returned by this function must either >> + * be embedded in a map as a kptr, or freed with bpf_crypto_ctx_release(). >> + * As crypto API functions use GFP_KERNEL allocations, this function can >> + * only be used in sleepable BPF programs. >> + * >> + * bpf_crypto_ctx_create() allocates memory for crypto context. >> + * It may return NULL if no memory is available. >> + * @type__str: pointer to string representation of crypto type. >> + * @algo__str: pointer to string representation of algorithm. >> + * @pkey: bpf_dynptr which holds cipher key to do crypto. > > Hi Vadim, > > a minor nit from my side: something about @authsize should go here. > Hi Simon! Good catch, I'll definitely add description to authsize, thanks! >> + * @err: integer to store error code when NULL is returned >> + */ >> +__bpf_kfunc struct bpf_crypto_ctx * >> +bpf_crypto_ctx_create(const char *type__str, const char *algo__str, >> + const struct bpf_dynptr_kern *pkey, >> + unsigned int authsize, int *err) > > ...
On 12/3/23 11:02 AM, Vadim Fedorenko wrote: >>> +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)) >> >> If I read patch 2 correctly, it is always built-in. I am not sure I understand >> the module_put/get in this patch. > > Well, yeah, right now it's built-in, but it can be easily converted to module > with it's own Kconfig option. Especially if we think about adding aead crypto > and using bpf in embedded setups with less amount of resources. What code is missing to support module? It sounds like all codes are ready. and does it really need a separate kconfig option? Can it depend on CONFIG_BPF_SYSCALL and CONFIG_CRYPTO_SKCIPHER? >>> +BTF_SET8_START(crypt_init_kfunc_btf_ids) >>> +BTF_ID_FLAGS(func, bpf_crypto_ctx_create, KF_ACQUIRE | KF_RET_NULL | >>> KF_SLEEPABLE) >>> +BTF_ID_FLAGS(func, bpf_crypto_ctx_release, KF_RELEASE) >>> +BTF_ID_FLAGS(func, bpf_crypto_ctx_acquire, KF_ACQUIRE | KF_TRUSTED_ARGS) >> >> Considering bpf_crypto_ctx is rcu protected, the acquire may use "KF_ACQUIRE | >> KF_RCU | KF_RET_NULL" such that the bpf_crypto_ctx_acquire(ctx_from_map_value) >> will work and the user will prepare checking NULL from day one. >> > > Got it. What about create? Should it also include KF_RCU? create should not need KF_RCU. The return value is a trusted/refcounted pointer. It has a reg->ref_obj_id => is_trusted_reg().
Hi Vadim, kernel test robot noticed the following build warnings: [auto build test WARNING on bpf-next/master] url: https://github.com/intel-lab-lkp/linux/commits/Vadim-Fedorenko/bpf-crypto-add-skcipher-to-bpf-crypto/20231202-091254 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master patch link: https://lore.kernel.org/r/20231202010604.1877561-1-vadfed%40meta.com patch subject: [PATCH bpf-next v7 1/3] bpf: make common crypto API for TC/XDP programs config: x86_64-buildonly-randconfig-001-20231202 (https://download.01.org/0day-ci/archive/20231206/202312060457.bRXN2xnb-lkp@intel.com/config) compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231206/202312060457.bRXN2xnb-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202312060457.bRXN2xnb-lkp@intel.com/ All warnings (new ones prefixed by >>): >> kernel/bpf/crypto.c:159:6: warning: variable 'ctx' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] if (!key) { ^~~~ kernel/bpf/crypto.c:192:6: note: uninitialized use occurs here if (ctx->tfm) ^~~ kernel/bpf/crypto.c:159:2: note: remove the 'if' if its condition is always false if (!key) { ^~~~~~~~~~~ kernel/bpf/crypto.c:154:6: warning: variable 'ctx' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] if (!key_len) { ^~~~~~~~ kernel/bpf/crypto.c:192:6: note: uninitialized use occurs here if (ctx->tfm) ^~~ kernel/bpf/crypto.c:154:2: note: remove the 'if' if its condition is always false if (!key_len) { ^~~~~~~~~~~~~~~ kernel/bpf/crypto.c:148:6: warning: variable 'ctx' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] if (authsize && !type->setauthsize) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ kernel/bpf/crypto.c:192:6: note: uninitialized use occurs here if (ctx->tfm) ^~~ kernel/bpf/crypto.c:148:2: note: remove the 'if' if its condition is always false if (authsize && !type->setauthsize) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ kernel/bpf/crypto.c:143:6: warning: variable 'ctx' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] if (!authsize && type->setauthsize) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ kernel/bpf/crypto.c:192:6: note: uninitialized use occurs here if (ctx->tfm) ^~~ kernel/bpf/crypto.c:143:2: note: remove the 'if' if its condition is always false if (!authsize && type->setauthsize) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ kernel/bpf/crypto.c:138:6: warning: variable 'ctx' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] if (!type->has_algo(algo__str)) { ^~~~~~~~~~~~~~~~~~~~~~~~~~ kernel/bpf/crypto.c:192:6: note: uninitialized use occurs here if (ctx->tfm) ^~~ kernel/bpf/crypto.c:138:2: note: remove the 'if' if its condition is always false if (!type->has_algo(algo__str)) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ kernel/bpf/crypto.c:128:28: note: initialize the variable 'ctx' to silence this warning struct bpf_crypto_ctx *ctx; ^ = NULL 5 warnings generated. vim +159 kernel/bpf/crypto.c 105 106 /** 107 * bpf_crypto_ctx_create() - Create a mutable BPF crypto context. 108 * 109 * Allocates a crypto context that can be used, acquired, and released by 110 * a BPF program. The crypto context returned by this function must either 111 * be embedded in a map as a kptr, or freed with bpf_crypto_ctx_release(). 112 * As crypto API functions use GFP_KERNEL allocations, this function can 113 * only be used in sleepable BPF programs. 114 * 115 * bpf_crypto_ctx_create() allocates memory for crypto context. 116 * It may return NULL if no memory is available. 117 * @type__str: pointer to string representation of crypto type. 118 * @algo__str: pointer to string representation of algorithm. 119 * @pkey: bpf_dynptr which holds cipher key to do crypto. 120 * @err: integer to store error code when NULL is returned 121 */ 122 __bpf_kfunc struct bpf_crypto_ctx * 123 bpf_crypto_ctx_create(const char *type__str, const char *algo__str, 124 const struct bpf_dynptr_kern *pkey, 125 unsigned int authsize, int *err) 126 { 127 const struct bpf_crypto_type *type = bpf_crypto_get_type(type__str); 128 struct bpf_crypto_ctx *ctx; 129 const u8 *key; 130 u32 key_len; 131 132 type = bpf_crypto_get_type(type__str); 133 if (IS_ERR(type)) { 134 *err = PTR_ERR(type); 135 return NULL; 136 } 137 138 if (!type->has_algo(algo__str)) { 139 *err = -EOPNOTSUPP; 140 goto err; 141 } 142 143 if (!authsize && type->setauthsize) { 144 *err = -EOPNOTSUPP; 145 goto err; 146 } 147 148 if (authsize && !type->setauthsize) { 149 *err = -EOPNOTSUPP; 150 goto err; 151 } 152 153 key_len = __bpf_dynptr_size(pkey); 154 if (!key_len) { 155 *err = -EINVAL; 156 goto err; 157 } 158 key = __bpf_dynptr_data(pkey, key_len); > 159 if (!key) { 160 *err = -EINVAL; 161 goto err; 162 } 163 164 ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); 165 if (!ctx) { 166 *err = -ENOMEM; 167 goto err; 168 } 169 170 ctx->type = type; 171 ctx->tfm = type->alloc_tfm(algo__str); 172 if (IS_ERR(ctx->tfm)) { 173 *err = PTR_ERR(ctx->tfm); 174 ctx->tfm = NULL; 175 goto err; 176 } 177 178 if (authsize) { 179 *err = type->setauthsize(ctx->tfm, authsize); 180 if (*err) 181 goto err; 182 } 183 184 *err = type->setkey(ctx->tfm, key, key_len); 185 if (*err) 186 goto err; 187 188 refcount_set(&ctx->usage, 1); 189 190 return ctx; 191 err: 192 if (ctx->tfm) 193 type->free_tfm(ctx->tfm); 194 kfree(ctx); 195 module_put(type->owner); 196 197 return NULL; 198 } 199
Hi Vadim, kernel test robot noticed the following build warnings: [auto build test WARNING on bpf-next/master] url: https://github.com/intel-lab-lkp/linux/commits/Vadim-Fedorenko/bpf-crypto-add-skcipher-to-bpf-crypto/20231202-091254 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master patch link: https://lore.kernel.org/r/20231202010604.1877561-1-vadfed%40meta.com patch subject: [PATCH bpf-next v7 1/3] bpf: make common crypto API for TC/XDP programs config: m68k-randconfig-r081-20231202 (https://download.01.org/0day-ci/archive/20231206/202312060500.uMJaMydz-lkp@intel.com/config) compiler: m68k-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231206/202312060500.uMJaMydz-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202312060500.uMJaMydz-lkp@intel.com/ All warnings (new ones prefixed by >>): >> kernel/bpf/crypto.c:126: warning: Function parameter or member 'authsize' not described in 'bpf_crypto_ctx_create' vim +126 kernel/bpf/crypto.c 105 106 /** 107 * bpf_crypto_ctx_create() - Create a mutable BPF crypto context. 108 * 109 * Allocates a crypto context that can be used, acquired, and released by 110 * a BPF program. The crypto context returned by this function must either 111 * be embedded in a map as a kptr, or freed with bpf_crypto_ctx_release(). 112 * As crypto API functions use GFP_KERNEL allocations, this function can 113 * only be used in sleepable BPF programs. 114 * 115 * bpf_crypto_ctx_create() allocates memory for crypto context. 116 * It may return NULL if no memory is available. 117 * @type__str: pointer to string representation of crypto type. 118 * @algo__str: pointer to string representation of algorithm. 119 * @pkey: bpf_dynptr which holds cipher key to do crypto. 120 * @err: integer to store error code when NULL is returned 121 */ 122 __bpf_kfunc struct bpf_crypto_ctx * 123 bpf_crypto_ctx_create(const char *type__str, const char *algo__str, 124 const struct bpf_dynptr_kern *pkey, 125 unsigned int authsize, int *err) > 126 { 127 const struct bpf_crypto_type *type = bpf_crypto_get_type(type__str); 128 struct bpf_crypto_ctx *ctx; 129 const u8 *key; 130 u32 key_len; 131 132 type = bpf_crypto_get_type(type__str); 133 if (IS_ERR(type)) { 134 *err = PTR_ERR(type); 135 return NULL; 136 } 137 138 if (!type->has_algo(algo__str)) { 139 *err = -EOPNOTSUPP; 140 goto err; 141 } 142 143 if (!authsize && type->setauthsize) { 144 *err = -EOPNOTSUPP; 145 goto err; 146 } 147 148 if (authsize && !type->setauthsize) { 149 *err = -EOPNOTSUPP; 150 goto err; 151 } 152 153 key_len = __bpf_dynptr_size(pkey); 154 if (!key_len) { 155 *err = -EINVAL; 156 goto err; 157 } 158 key = __bpf_dynptr_data(pkey, key_len); 159 if (!key) { 160 *err = -EINVAL; 161 goto err; 162 } 163 164 ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); 165 if (!ctx) { 166 *err = -ENOMEM; 167 goto err; 168 } 169 170 ctx->type = type; 171 ctx->tfm = type->alloc_tfm(algo__str); 172 if (IS_ERR(ctx->tfm)) { 173 *err = PTR_ERR(ctx->tfm); 174 ctx->tfm = NULL; 175 goto err; 176 } 177 178 if (authsize) { 179 *err = type->setauthsize(ctx->tfm, authsize); 180 if (*err) 181 goto err; 182 } 183 184 *err = type->setkey(ctx->tfm, key, key_len); 185 if (*err) 186 goto err; 187 188 refcount_set(&ctx->usage, 1); 189 190 return ctx; 191 err: 192 if (ctx->tfm) 193 type->free_tfm(ctx->tfm); 194 kfree(ctx); 195 module_put(type->owner); 196 197 return NULL; 198 } 199
Hi Vadim,
kernel test robot noticed the following build warnings:
url: https://github.com/intel-lab-lkp/linux/commits/Vadim-Fedorenko/bpf-crypto-add-skcipher-to-bpf-crypto/20231202-091254
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20231202010604.1877561-1-vadfed%40meta.com
patch subject: [PATCH bpf-next v7 1/3] bpf: make common crypto API for TC/XDP programs
config: x86_64-randconfig-161-20231202 (https://download.01.org/0day-ci/archive/20231206/202312060647.2JfAE3rk-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce: (https://download.01.org/0day-ci/archive/20231206/202312060647.2JfAE3rk-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202312060647.2JfAE3rk-lkp@intel.com/
smatch warnings:
kernel/bpf/crypto.c:192 bpf_crypto_ctx_create() error: we previously assumed 'ctx' could be null (see line 165)
kernel/bpf/crypto.c:192 bpf_crypto_ctx_create() error: potentially dereferencing uninitialized 'ctx'.
vim +/ctx +192 kernel/bpf/crypto.c
0c47cb96ac404e Vadim Fedorenko 2023-12-01 122 __bpf_kfunc struct bpf_crypto_ctx *
0c47cb96ac404e Vadim Fedorenko 2023-12-01 123 bpf_crypto_ctx_create(const char *type__str, const char *algo__str,
0c47cb96ac404e Vadim Fedorenko 2023-12-01 124 const struct bpf_dynptr_kern *pkey,
0c47cb96ac404e Vadim Fedorenko 2023-12-01 125 unsigned int authsize, int *err)
0c47cb96ac404e Vadim Fedorenko 2023-12-01 126 {
0c47cb96ac404e Vadim Fedorenko 2023-12-01 127 const struct bpf_crypto_type *type = bpf_crypto_get_type(type__str);
Delete this assignment. (Duplicated).
0c47cb96ac404e Vadim Fedorenko 2023-12-01 128 struct bpf_crypto_ctx *ctx;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 129 const u8 *key;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 130 u32 key_len;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 131
0c47cb96ac404e Vadim Fedorenko 2023-12-01 132 type = bpf_crypto_get_type(type__str);
0c47cb96ac404e Vadim Fedorenko 2023-12-01 133 if (IS_ERR(type)) {
0c47cb96ac404e Vadim Fedorenko 2023-12-01 134 *err = PTR_ERR(type);
0c47cb96ac404e Vadim Fedorenko 2023-12-01 135 return NULL;
Why doesn't this function just return error pointers?
0c47cb96ac404e Vadim Fedorenko 2023-12-01 136 }
0c47cb96ac404e Vadim Fedorenko 2023-12-01 137
0c47cb96ac404e Vadim Fedorenko 2023-12-01 138 if (!type->has_algo(algo__str)) {
0c47cb96ac404e Vadim Fedorenko 2023-12-01 139 *err = -EOPNOTSUPP;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 140 goto err;
ctx is uninitialized on this path.
0c47cb96ac404e Vadim Fedorenko 2023-12-01 141 }
0c47cb96ac404e Vadim Fedorenko 2023-12-01 142
0c47cb96ac404e Vadim Fedorenko 2023-12-01 143 if (!authsize && type->setauthsize) {
0c47cb96ac404e Vadim Fedorenko 2023-12-01 144 *err = -EOPNOTSUPP;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 145 goto err;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 146 }
0c47cb96ac404e Vadim Fedorenko 2023-12-01 147
0c47cb96ac404e Vadim Fedorenko 2023-12-01 148 if (authsize && !type->setauthsize) {
0c47cb96ac404e Vadim Fedorenko 2023-12-01 149 *err = -EOPNOTSUPP;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 150 goto err;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 151 }
0c47cb96ac404e Vadim Fedorenko 2023-12-01 152
0c47cb96ac404e Vadim Fedorenko 2023-12-01 153 key_len = __bpf_dynptr_size(pkey);
0c47cb96ac404e Vadim Fedorenko 2023-12-01 154 if (!key_len) {
0c47cb96ac404e Vadim Fedorenko 2023-12-01 155 *err = -EINVAL;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 156 goto err;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 157 }
0c47cb96ac404e Vadim Fedorenko 2023-12-01 158 key = __bpf_dynptr_data(pkey, key_len);
0c47cb96ac404e Vadim Fedorenko 2023-12-01 159 if (!key) {
0c47cb96ac404e Vadim Fedorenko 2023-12-01 160 *err = -EINVAL;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 161 goto err;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 162 }
0c47cb96ac404e Vadim Fedorenko 2023-12-01 163
0c47cb96ac404e Vadim Fedorenko 2023-12-01 164 ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
0c47cb96ac404e Vadim Fedorenko 2023-12-01 @165 if (!ctx) {
0c47cb96ac404e Vadim Fedorenko 2023-12-01 166 *err = -ENOMEM;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 167 goto err;
ctx is NULL here.
0c47cb96ac404e Vadim Fedorenko 2023-12-01 168 }
0c47cb96ac404e Vadim Fedorenko 2023-12-01 169
0c47cb96ac404e Vadim Fedorenko 2023-12-01 170 ctx->type = type;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 171 ctx->tfm = type->alloc_tfm(algo__str);
0c47cb96ac404e Vadim Fedorenko 2023-12-01 172 if (IS_ERR(ctx->tfm)) {
0c47cb96ac404e Vadim Fedorenko 2023-12-01 173 *err = PTR_ERR(ctx->tfm);
0c47cb96ac404e Vadim Fedorenko 2023-12-01 174 ctx->tfm = NULL;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 175 goto err;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 176 }
0c47cb96ac404e Vadim Fedorenko 2023-12-01 177
0c47cb96ac404e Vadim Fedorenko 2023-12-01 178 if (authsize) {
0c47cb96ac404e Vadim Fedorenko 2023-12-01 179 *err = type->setauthsize(ctx->tfm, authsize);
0c47cb96ac404e Vadim Fedorenko 2023-12-01 180 if (*err)
0c47cb96ac404e Vadim Fedorenko 2023-12-01 181 goto err;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 182 }
0c47cb96ac404e Vadim Fedorenko 2023-12-01 183
0c47cb96ac404e Vadim Fedorenko 2023-12-01 184 *err = type->setkey(ctx->tfm, key, key_len);
0c47cb96ac404e Vadim Fedorenko 2023-12-01 185 if (*err)
0c47cb96ac404e Vadim Fedorenko 2023-12-01 186 goto err;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 187
0c47cb96ac404e Vadim Fedorenko 2023-12-01 188 refcount_set(&ctx->usage, 1);
0c47cb96ac404e Vadim Fedorenko 2023-12-01 189
0c47cb96ac404e Vadim Fedorenko 2023-12-01 190 return ctx;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 191 err:
0c47cb96ac404e Vadim Fedorenko 2023-12-01 @192 if (ctx->tfm)
^^^^^^^^
NULL dereference. These two error handling bugs in three lines of code
are canonical One Err Label type bugs. Better to do a ladder where each
error label frees the last thing that was allocated. Easier to review.
Then you could delete the "ctx->tfm = NULL;" assignment on line 174.
return ctx;
err_free_tfm:
type->free_tfm(ctx->tfm);
err_free_ctx:
kfree(ctx);
err_module_put:
module_put(type->owner);
return NULL;
I have written about this at length on my blog:
https://staticthinking.wordpress.com/2022/04/28/free-the-last-thing-style/
0c47cb96ac404e Vadim Fedorenko 2023-12-01 193 type->free_tfm(ctx->tfm);
0c47cb96ac404e Vadim Fedorenko 2023-12-01 194 kfree(ctx);
0c47cb96ac404e Vadim Fedorenko 2023-12-01 195 module_put(type->owner);
0c47cb96ac404e Vadim Fedorenko 2023-12-01 196
0c47cb96ac404e Vadim Fedorenko 2023-12-01 197 return NULL;
0c47cb96ac404e Vadim Fedorenko 2023-12-01 198 }
On 05/12/2023 21:56, Dan Carpenter wrote: > Hi Vadim, > > kernel test robot noticed the following build warnings: > > url: https://github.com/intel-lab-lkp/linux/commits/Vadim-Fedorenko/bpf-crypto-add-skcipher-to-bpf-crypto/20231202-091254 > base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master > patch link: https://lore.kernel.org/r/20231202010604.1877561-1-vadfed%40meta.com > patch subject: [PATCH bpf-next v7 1/3] bpf: make common crypto API for TC/XDP programs > config: x86_64-randconfig-161-20231202 (https://download.01.org/0day-ci/archive/20231206/202312060647.2JfAE3rk-lkp@intel.com/config) > compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07) > reproduce: (https://download.01.org/0day-ci/archive/20231206/202312060647.2JfAE3rk-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > | Closes: https://lore.kernel.org/r/202312060647.2JfAE3rk-lkp@intel.com/ > > smatch warnings: > kernel/bpf/crypto.c:192 bpf_crypto_ctx_create() error: we previously assumed 'ctx' could be null (see line 165) > kernel/bpf/crypto.c:192 bpf_crypto_ctx_create() error: potentially dereferencing uninitialized 'ctx'. > > vim +/ctx +192 kernel/bpf/crypto.c > > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 122 __bpf_kfunc struct bpf_crypto_ctx * > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 123 bpf_crypto_ctx_create(const char *type__str, const char *algo__str, > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 124 const struct bpf_dynptr_kern *pkey, > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 125 unsigned int authsize, int *err) > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 126 { > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 127 const struct bpf_crypto_type *type = bpf_crypto_get_type(type__str); > > Delete this assignment. (Duplicated). > Ah, yeah, will remove it. > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 128 struct bpf_crypto_ctx *ctx; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 129 const u8 *key; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 130 u32 key_len; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 131 > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 132 type = bpf_crypto_get_type(type__str); > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 133 if (IS_ERR(type)) { > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 134 *err = PTR_ERR(type); > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 135 return NULL; > > Why doesn't this function just return error pointers? bpf_kfuncs cannot return error pointers, it makes BPF verifier very unhappy. > > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 136 } > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 137 > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 138 if (!type->has_algo(algo__str)) { > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 139 *err = -EOPNOTSUPP; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 140 goto err; > > ctx is uninitialized on this path. > Yep, it was already highlighted in the feedback, thanks. > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 141 } > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 142 > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 143 if (!authsize && type->setauthsize) { > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 144 *err = -EOPNOTSUPP; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 145 goto err; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 146 } > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 147 > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 148 if (authsize && !type->setauthsize) { > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 149 *err = -EOPNOTSUPP; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 150 goto err; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 151 } > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 152 > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 153 key_len = __bpf_dynptr_size(pkey); > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 154 if (!key_len) { > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 155 *err = -EINVAL; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 156 goto err; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 157 } > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 158 key = __bpf_dynptr_data(pkey, key_len); > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 159 if (!key) { > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 160 *err = -EINVAL; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 161 goto err; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 162 } > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 163 > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 164 ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 @165 if (!ctx) { > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 166 *err = -ENOMEM; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 167 goto err; > > ctx is NULL here. > > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 168 } > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 169 > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 170 ctx->type = type; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 171 ctx->tfm = type->alloc_tfm(algo__str); > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 172 if (IS_ERR(ctx->tfm)) { > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 173 *err = PTR_ERR(ctx->tfm); > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 174 ctx->tfm = NULL; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 175 goto err; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 176 } > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 177 > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 178 if (authsize) { > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 179 *err = type->setauthsize(ctx->tfm, authsize); > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 180 if (*err) > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 181 goto err; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 182 } > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 183 > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 184 *err = type->setkey(ctx->tfm, key, key_len); > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 185 if (*err) > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 186 goto err; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 187 > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 188 refcount_set(&ctx->usage, 1); > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 189 > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 190 return ctx; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 191 err: > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 @192 if (ctx->tfm) > ^^^^^^^^ > NULL dereference. These two error handling bugs in three lines of code > are canonical One Err Label type bugs. Better to do a ladder where each > error label frees the last thing that was allocated. Easier to review. > Then you could delete the "ctx->tfm = NULL;" assignment on line 174. > > return ctx; > > err_free_tfm: > type->free_tfm(ctx->tfm); > err_free_ctx: > kfree(ctx); > err_module_put: > module_put(type->owner); > > return NULL; > > I have written about this at length on my blog: > https://staticthinking.wordpress.com/2022/04/28/free-the-last-thing-style/ Thanks, very good blog post, I'll follow this way in the next version. > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 193 type->free_tfm(ctx->tfm); > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 194 kfree(ctx); > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 195 module_put(type->owner); > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 196 > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 197 return NULL; > 0c47cb96ac404e Vadim Fedorenko 2023-12-01 198 } >
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index eb447b0a9423..0143ff6c93a1 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1228,6 +1228,7 @@ int bpf_dynptr_check_size(u32 size); u32 __bpf_dynptr_size(const struct bpf_dynptr_kern *ptr); const void *__bpf_dynptr_data(const struct bpf_dynptr_kern *ptr, u32 len); void *__bpf_dynptr_data_rw(const struct bpf_dynptr_kern *ptr, u32 len); +bool __bpf_dynptr_is_rdonly(const struct bpf_dynptr_kern *ptr); #ifdef CONFIG_BPF_JIT int bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr); diff --git a/include/linux/bpf_crypto.h b/include/linux/bpf_crypto.h new file mode 100644 index 000000000000..e81bd8ab979c --- /dev/null +++ b/include/linux/bpf_crypto.h @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* Copyright (c) 2023 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); + 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/Makefile b/kernel/bpf/Makefile index f526b7573e97..bcde762bb2c2 100644 --- a/kernel/bpf/Makefile +++ b/kernel/bpf/Makefile @@ -41,6 +41,9 @@ obj-$(CONFIG_BPF_SYSCALL) += bpf_struct_ops.o obj-$(CONFIG_BPF_SYSCALL) += cpumask.o obj-${CONFIG_BPF_LSM} += bpf_lsm.o endif +ifeq ($(CONFIG_CRYPTO),y) +obj-$(CONFIG_BPF_SYSCALL) += crypto.o +endif obj-$(CONFIG_BPF_PRELOAD) += preload/ obj-$(CONFIG_BPF_SYSCALL) += relo_core.o diff --git a/kernel/bpf/crypto.c b/kernel/bpf/crypto.c new file mode 100644 index 000000000000..a1e543d1d7fe --- /dev/null +++ b/kernel/bpf/crypto.c @@ -0,0 +1,364 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright (c) 2023 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; +}; + +static LIST_HEAD(bpf_crypto_types); +static DECLARE_RWSEM(bpf_crypto_types_sem); + +/** + * 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. + * @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; + 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 = -EEXIST; + + 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(sizeof(*node), GFP_KERNEL); + 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(); + +/** + * bpf_crypto_ctx_create() - Create a mutable BPF crypto context. + * + * Allocates a crypto context that can be used, acquired, and released by + * a BPF program. The crypto context returned by this function must either + * be embedded in a map as a kptr, or freed with bpf_crypto_ctx_release(). + * As crypto API functions use GFP_KERNEL allocations, this function can + * only be used in sleepable BPF programs. + * + * bpf_crypto_ctx_create() allocates memory for crypto context. + * It may return NULL if no memory is available. + * @type__str: pointer to string representation of crypto type. + * @algo__str: pointer to string representation of algorithm. + * @pkey: bpf_dynptr which holds cipher key to do crypto. + * @err: integer to store error code when NULL is returned + */ +__bpf_kfunc struct bpf_crypto_ctx * +bpf_crypto_ctx_create(const char *type__str, const char *algo__str, + const struct bpf_dynptr_kern *pkey, + unsigned int authsize, int *err) +{ + const struct bpf_crypto_type *type = bpf_crypto_get_type(type__str); + struct bpf_crypto_ctx *ctx; + const u8 *key; + u32 key_len; + + type = bpf_crypto_get_type(type__str); + if (IS_ERR(type)) { + *err = PTR_ERR(type); + return NULL; + } + + if (!type->has_algo(algo__str)) { + *err = -EOPNOTSUPP; + goto err; + } + + if (!authsize && type->setauthsize) { + *err = -EOPNOTSUPP; + goto err; + } + + if (authsize && !type->setauthsize) { + *err = -EOPNOTSUPP; + goto err; + } + + key_len = __bpf_dynptr_size(pkey); + if (!key_len) { + *err = -EINVAL; + goto err; + } + key = __bpf_dynptr_data(pkey, key_len); + if (!key) { + *err = -EINVAL; + goto err; + } + + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); + if (!ctx) { + *err = -ENOMEM; + goto err; + } + + ctx->type = type; + ctx->tfm = type->alloc_tfm(algo__str); + if (IS_ERR(ctx->tfm)) { + *err = PTR_ERR(ctx->tfm); + ctx->tfm = NULL; + goto err; + } + + if (authsize) { + *err = type->setauthsize(ctx->tfm, authsize); + if (*err) + goto err; + } + + *err = type->setkey(ctx->tfm, key, key_len); + if (*err) + goto err; + + refcount_set(&ctx->usage, 1); + + return ctx; +err: + if (ctx->tfm) + type->free_tfm(ctx->tfm); + kfree(ctx); + module_put(type->owner); + + return NULL; +} + +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); +} + +/** + * bpf_crypto_ctx_acquire() - Acquire a reference to a BPF crypto context. + * @ctx: The BPF crypto context being acquired. The ctx must be a trusted + * pointer. + * + * Acquires a reference to a BPF crypto context. The context returned by this function + * must either be embedded in a map as a kptr, or freed with + * bpf_crypto_skcipher_ctx_release(). + */ +__bpf_kfunc struct bpf_crypto_ctx * +bpf_crypto_ctx_acquire(struct bpf_crypto_ctx *ctx) +{ + refcount_inc(&ctx->usage); + return ctx; +} + +/** + * bpf_crypto_ctx_release() - Release a previously acquired BPF crypto context. + * @ctx: The crypto context being released. + * + * Releases a previously acquired reference to a BPF crypto context. When the final + * reference of the BPF crypto context has been released, it is subsequently freed in + * an RCU callback in the BPF memory allocator. + */ +__bpf_kfunc void bpf_crypto_ctx_release(struct bpf_crypto_ctx *ctx) +{ + if (refcount_dec_and_test(&ctx->usage)) + call_rcu(&ctx->rcu, crypto_free_cb); +} + +static int bpf_crypto_crypt(const struct bpf_crypto_ctx *ctx, + const struct bpf_dynptr_kern *src, + struct bpf_dynptr_kern *dst, + const struct bpf_dynptr_kern *iv, + bool decrypt) +{ + u32 src_len, dst_len, iv_len; + const u8 *psrc; + u8 *pdst, *piv; + int err; + + if (ctx->type->get_flags(ctx->tfm) & CRYPTO_TFM_NEED_KEY) + return -EINVAL; + + if (__bpf_dynptr_is_rdonly(dst)) + return -EINVAL; + + iv_len = __bpf_dynptr_size(iv); + src_len = __bpf_dynptr_size(src); + dst_len = __bpf_dynptr_size(dst); + if (!src_len || !dst_len) + return -EINVAL; + + if (iv_len != ctx->type->ivsize(ctx->tfm)) + return -EINVAL; + + psrc = __bpf_dynptr_data(src, src_len); + if (!psrc) + return -EINVAL; + pdst = __bpf_dynptr_data_rw(dst, dst_len); + if (!pdst) + return -EINVAL; + + 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; +} + +/** + * bpf_crypto_decrypt() - Decrypt buffer using configured context and IV provided. + * @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. + * @iv: bpf_dynptr to IV data to be used by decryptor. + * + * 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_kern *src, + struct bpf_dynptr_kern *dst, + struct bpf_dynptr_kern *iv) +{ + return bpf_crypto_crypt(ctx, src, dst, iv, true); +} + +/** + * bpf_crypto_encrypt() - Encrypt buffer using configured context and IV provided. + * @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 buffer where to store the result. Must be a trusted pointer. + * @iv: bpf_dynptr to IV data to be used by decryptor. + * + * 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_kern *src, + struct bpf_dynptr_kern *dst, + struct bpf_dynptr_kern *iv) +{ + return bpf_crypto_crypt(ctx, src, dst, iv, false); +} + +__bpf_kfunc_end_defs(); + +BTF_SET8_START(crypt_init_kfunc_btf_ids) +BTF_ID_FLAGS(func, bpf_crypto_ctx_create, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE) +BTF_ID_FLAGS(func, bpf_crypto_ctx_release, KF_RELEASE) +BTF_ID_FLAGS(func, bpf_crypto_ctx_acquire, KF_ACQUIRE | KF_TRUSTED_ARGS) +BTF_SET8_END(crypt_init_kfunc_btf_ids) + +static const struct btf_kfunc_id_set crypt_init_kfunc_set = { + .owner = THIS_MODULE, + .set = &crypt_init_kfunc_btf_ids, +}; + +BTF_SET8_START(crypt_kfunc_btf_ids) +BTF_ID_FLAGS(func, bpf_crypto_decrypt, KF_RCU) +BTF_ID_FLAGS(func, bpf_crypto_encrypt, KF_RCU) +BTF_SET8_END(crypt_kfunc_btf_ids) + +static const struct btf_kfunc_id_set crypt_kfunc_set = { + .owner = THIS_MODULE, + .set = &crypt_kfunc_btf_ids, +}; + +BTF_ID_LIST(bpf_crypto_dtor_ids) +BTF_ID(struct, bpf_crypto_ctx) +BTF_ID(func, bpf_crypto_ctx_release) + +static int __init crypto_kfunc_init(void) +{ + int ret; + const struct btf_id_dtor_kfunc bpf_crypto_dtors[] = { + { + .btf_id = bpf_crypto_dtor_ids[0], + .kfunc_btf_id = bpf_crypto_dtor_ids[1] + }, + }; + + ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &crypt_kfunc_set); + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_ACT, &crypt_kfunc_set); + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &crypt_kfunc_set); + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_UNSPEC, + &crypt_init_kfunc_set); + return ret ?: register_btf_id_dtor_kfuncs(bpf_crypto_dtors, + ARRAY_SIZE(bpf_crypto_dtors), + THIS_MODULE); +} + +late_initcall(crypto_kfunc_init); diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index b45a8381f9bd..b73314c0124e 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1436,7 +1436,7 @@ static const struct bpf_func_proto bpf_kptr_xchg_proto = { #define DYNPTR_SIZE_MASK 0xFFFFFF #define DYNPTR_RDONLY_BIT BIT(31) -static bool __bpf_dynptr_is_rdonly(const struct bpf_dynptr_kern *ptr) +bool __bpf_dynptr_is_rdonly(const struct bpf_dynptr_kern *ptr) { return ptr->size & DYNPTR_RDONLY_BIT; } diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 8e7b6072e3f4..c54716966d5d 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -5162,6 +5162,7 @@ BTF_ID(struct, cgroup) #endif BTF_ID(struct, bpf_cpumask) BTF_ID(struct, task_struct) +BTF_ID(struct, bpf_crypto_ctx) BTF_SET_END(rcu_protected_types) static bool rcu_protected_object(const struct btf *btf, u32 btf_id)
Add crypto API support to BPF to be able to decrypt or encrypt packets in TC/XDP BPF programs. Special care should be taken for initialization part of crypto algo because crypto alloc) doesn't work with preemtion disabled, it can be run only in sleepable BPF program. Also async crypto is not supported because of the very same issue - TC/XDP BPF programs are not sleepable. Signed-off-by: Vadim Fedorenko <vadfed@meta.com> --- v6 -> v7: - style fixes v5 -> v6: - replace lskcipher with infrastructure to provide pluggable cipher types - add BPF skcipher as plug-in module in a separate patch v4 -> v5: - replace crypto API to use lskcipher (suggested by Herbert Xu) - remove SG list usage and provide raw buffers v3 -> v4: - reuse __bpf_dynptr_data and remove own implementation - use const __str to provide algorithm name - use kfunc macroses to avoid compilator warnings v2 -> v3: - fix kdoc issues v1 -> v2: - use kmalloc in sleepable func, suggested by Alexei - use __bpf_dynptr_is_rdonly() to check destination, suggested by Jakub - use __bpf_dynptr_data_ptr() for all dynptr accesses --- include/linux/bpf.h | 1 + include/linux/bpf_crypto.h | 23 +++ kernel/bpf/Makefile | 3 + kernel/bpf/crypto.c | 364 +++++++++++++++++++++++++++++++++++++ kernel/bpf/helpers.c | 2 +- kernel/bpf/verifier.c | 1 + 6 files changed, 393 insertions(+), 1 deletion(-) create mode 100644 include/linux/bpf_crypto.h create mode 100644 kernel/bpf/crypto.c