* [PATCH bpf-next 1/4] bpf: verifier: make kfuncs args nullalble
2024-05-09 13:40 [PATCH bpf-next 0/4] bpf: make trusted args nullable Vadim Fedorenko
@ 2024-05-09 13:40 ` Vadim Fedorenko
2024-05-09 13:40 ` [PATCH bpf-next 2/4] bpf: crypto: make state and IV dynptr nullable Vadim Fedorenko
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Vadim Fedorenko @ 2024-05-09 13:40 UTC (permalink / raw)
To: Vadim Fedorenko, Martin KaFai Lau, Andrii Nakryiko,
Alexei Starovoitov, Mykola Lysenko, Jakub Kicinski
Cc: Vadim Fedorenko, bpf
Some arguments to kfuncs might be NULL in some cases. But currently it's
not possible to pass NULL to any BTF structures because the check for
the suffix is located after all type checks. Move it to earlier place
to allow nullable args.
Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
---
kernel/bpf/verifier.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 9e3aba08984e..ed67aed3c284 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11179,6 +11179,9 @@ get_kfunc_ptr_arg_type(struct bpf_verifier_env *env,
if (btf_is_prog_ctx_type(&env->log, meta->btf, t, resolve_prog_type(env->prog), argno))
return KF_ARG_PTR_TO_CTX;
+ if (is_kfunc_arg_nullable(meta->btf, &args[argno]) && register_is_null(reg))
+ return KF_ARG_PTR_TO_NULL;
+
if (is_kfunc_arg_alloc_obj(meta->btf, &args[argno]))
return KF_ARG_PTR_TO_ALLOC_BTF_ID;
@@ -11224,9 +11227,6 @@ get_kfunc_ptr_arg_type(struct bpf_verifier_env *env,
if (is_kfunc_arg_callback(env, meta->btf, &args[argno]))
return KF_ARG_PTR_TO_CALLBACK;
- if (is_kfunc_arg_nullable(meta->btf, &args[argno]) && register_is_null(reg))
- return KF_ARG_PTR_TO_NULL;
-
if (argno + 1 < nargs &&
(is_kfunc_arg_mem_size(meta->btf, &args[argno + 1], ®s[regno + 1]) ||
is_kfunc_arg_const_mem_size(meta->btf, &args[argno + 1], ®s[regno + 1])))
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH bpf-next 2/4] bpf: crypto: make state and IV dynptr nullable
2024-05-09 13:40 [PATCH bpf-next 0/4] bpf: make trusted args nullable Vadim Fedorenko
2024-05-09 13:40 ` [PATCH bpf-next 1/4] bpf: verifier: make kfuncs args nullalble Vadim Fedorenko
@ 2024-05-09 13:40 ` Vadim Fedorenko
2024-05-10 3:07 ` kernel test robot
2024-05-09 13:40 ` [PATCH bpf-next 3/4] selftests: bpf: crypto: use NULL instead of 0-sized dynptr Vadim Fedorenko
2024-05-09 13:40 ` [PATCH bpf-next 4/4] selftests: bpf: crypto: adjust bench to use nullable IV Vadim Fedorenko
3 siblings, 1 reply; 6+ messages in thread
From: Vadim Fedorenko @ 2024-05-09 13:40 UTC (permalink / raw)
To: Vadim Fedorenko, Martin KaFai Lau, Andrii Nakryiko,
Alexei Starovoitov, Mykola Lysenko, Jakub Kicinski
Cc: Vadim Fedorenko, bpf
Some ciphers do not require state and IV buffer, but with current
implementation 0-sized dynptr is always needed. With adjustment to
verifier we can provide NULL instead of 0-sized dynptr. Make crypto
kfuncs ready for this.
Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
---
kernel/bpf/crypto.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/kernel/bpf/crypto.c b/kernel/bpf/crypto.c
index 2bee4af91e38..4f4446c34b3c 100644
--- a/kernel/bpf/crypto.c
+++ b/kernel/bpf/crypto.c
@@ -275,7 +275,7 @@ static int bpf_crypto_crypt(const struct bpf_crypto_ctx *ctx,
if (__bpf_dynptr_is_rdonly(dst))
return -EINVAL;
- siv_len = __bpf_dynptr_size(siv);
+ siv_len = siv ? __bpf_dynptr_size(siv) : 0;
src_len = __bpf_dynptr_size(src);
dst_len = __bpf_dynptr_size(dst);
if (!src_len || !dst_len)
@@ -313,9 +313,9 @@ static int bpf_crypto_crypt(const struct bpf_crypto_ctx *ctx,
__bpf_kfunc int bpf_crypto_decrypt(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 *siv__nullable)
{
- return bpf_crypto_crypt(ctx, src, dst, siv, true);
+ return bpf_crypto_crypt(ctx, src, dst, siv__nullable, true);
}
/**
@@ -330,9 +330,9 @@ __bpf_kfunc int bpf_crypto_decrypt(struct bpf_crypto_ctx *ctx,
__bpf_kfunc int bpf_crypto_encrypt(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 *siv__nullable)
{
- return bpf_crypto_crypt(ctx, src, dst, siv, false);
+ return bpf_crypto_crypt(ctx, src, dst, siv__nullable, false);
}
__bpf_kfunc_end_defs();
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH bpf-next 2/4] bpf: crypto: make state and IV dynptr nullable
2024-05-09 13:40 ` [PATCH bpf-next 2/4] bpf: crypto: make state and IV dynptr nullable Vadim Fedorenko
@ 2024-05-10 3:07 ` kernel test robot
0 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2024-05-10 3:07 UTC (permalink / raw)
To: Vadim Fedorenko, Vadim Fedorenko, Martin KaFai Lau,
Andrii Nakryiko, Alexei Starovoitov, Mykola Lysenko,
Jakub Kicinski
Cc: oe-kbuild-all, bpf
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-verifier-make-kfuncs-args-nullalble/20240509-214252
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20240509134023.1289303-3-vadfed%40meta.com
patch subject: [PATCH bpf-next 2/4] bpf: crypto: make state and IV dynptr nullable
config: x86_64-randconfig-102-20240510 (https://download.01.org/0day-ci/archive/20240510/202405101026.4PbHjNBN-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240510/202405101026.4PbHjNBN-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/202405101026.4PbHjNBN-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> kernel/bpf/crypto.c:317: warning: Function parameter or struct member 'siv__nullable' not described in 'bpf_crypto_decrypt'
>> kernel/bpf/crypto.c:317: warning: Excess function parameter 'siv' description in 'bpf_crypto_decrypt'
>> kernel/bpf/crypto.c:334: warning: Function parameter or struct member 'siv__nullable' not described in 'bpf_crypto_encrypt'
>> kernel/bpf/crypto.c:334: warning: Excess function parameter 'siv' description in 'bpf_crypto_encrypt'
vim +317 kernel/bpf/crypto.c
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 303
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 304 /**
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 305 * bpf_crypto_decrypt() - Decrypt buffer using configured context and IV provided.
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 306 * @ctx: The crypto context being used. The ctx must be a trusted pointer.
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 307 * @src: bpf_dynptr to the encrypted data. Must be a trusted pointer.
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 308 * @dst: bpf_dynptr to the buffer where to store the result. Must be a trusted pointer.
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 309 * @siv: bpf_dynptr to IV data and state data to be used by decryptor.
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 310 *
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 311 * Decrypts provided buffer using IV data and the crypto context. Crypto context must be configured.
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 312 */
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 313 __bpf_kfunc int bpf_crypto_decrypt(struct bpf_crypto_ctx *ctx,
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 314 const struct bpf_dynptr_kern *src,
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 315 const struct bpf_dynptr_kern *dst,
9ce5fb6f36b954 Vadim Fedorenko 2024-05-09 316 const struct bpf_dynptr_kern *siv__nullable)
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 @317 {
9ce5fb6f36b954 Vadim Fedorenko 2024-05-09 318 return bpf_crypto_crypt(ctx, src, dst, siv__nullable, true);
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 319 }
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 320
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 321 /**
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 322 * bpf_crypto_encrypt() - Encrypt buffer using configured context and IV provided.
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 323 * @ctx: The crypto context being used. The ctx must be a trusted pointer.
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 324 * @src: bpf_dynptr to the plain data. Must be a trusted pointer.
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 325 * @dst: bpf_dynptr to buffer where to store the result. Must be a trusted pointer.
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 326 * @siv: bpf_dynptr to IV data and state data to be used by decryptor.
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 327 *
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 328 * Encrypts provided buffer using IV data and the crypto context. Crypto context must be configured.
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 329 */
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 330 __bpf_kfunc int bpf_crypto_encrypt(struct bpf_crypto_ctx *ctx,
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 331 const struct bpf_dynptr_kern *src,
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 332 const struct bpf_dynptr_kern *dst,
9ce5fb6f36b954 Vadim Fedorenko 2024-05-09 333 const struct bpf_dynptr_kern *siv__nullable)
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 @334 {
9ce5fb6f36b954 Vadim Fedorenko 2024-05-09 335 return bpf_crypto_crypt(ctx, src, dst, siv__nullable, false);
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 336 }
3e1c6f35409f9e Vadim Fedorenko 2024-04-22 337
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf-next 3/4] selftests: bpf: crypto: use NULL instead of 0-sized dynptr
2024-05-09 13:40 [PATCH bpf-next 0/4] bpf: make trusted args nullable Vadim Fedorenko
2024-05-09 13:40 ` [PATCH bpf-next 1/4] bpf: verifier: make kfuncs args nullalble Vadim Fedorenko
2024-05-09 13:40 ` [PATCH bpf-next 2/4] bpf: crypto: make state and IV dynptr nullable Vadim Fedorenko
@ 2024-05-09 13:40 ` Vadim Fedorenko
2024-05-09 13:40 ` [PATCH bpf-next 4/4] selftests: bpf: crypto: adjust bench to use nullable IV Vadim Fedorenko
3 siblings, 0 replies; 6+ messages in thread
From: Vadim Fedorenko @ 2024-05-09 13:40 UTC (permalink / raw)
To: Vadim Fedorenko, Martin KaFai Lau, Andrii Nakryiko,
Alexei Starovoitov, Mykola Lysenko, Jakub Kicinski
Cc: Vadim Fedorenko, bpf
Adjust selftests to use nullable option for state and IV arg.
Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
---
.../testing/selftests/bpf/progs/crypto_sanity.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/crypto_sanity.c b/tools/testing/selftests/bpf/progs/crypto_sanity.c
index 1be0a3fa5efd..645be6cddf36 100644
--- a/tools/testing/selftests/bpf/progs/crypto_sanity.c
+++ b/tools/testing/selftests/bpf/progs/crypto_sanity.c
@@ -89,7 +89,7 @@ int decrypt_sanity(struct __sk_buff *skb)
{
struct __crypto_ctx_value *v;
struct bpf_crypto_ctx *ctx;
- struct bpf_dynptr psrc, pdst, iv;
+ struct bpf_dynptr psrc, pdst;
int err;
err = skb_dynptr_validate(skb, &psrc);
@@ -114,12 +114,8 @@ int decrypt_sanity(struct __sk_buff *skb)
* production code, a percpu map should be used to store the result.
*/
bpf_dynptr_from_mem(dst, sizeof(dst), 0, &pdst);
- /* iv dynptr has to be initialized with 0 size, but proper memory region
- * has to be provided anyway
- */
- bpf_dynptr_from_mem(dst, 0, 0, &iv);
- status = bpf_crypto_decrypt(ctx, &psrc, &pdst, &iv);
+ status = bpf_crypto_decrypt(ctx, &psrc, &pdst, NULL);
return TC_ACT_SHOT;
}
@@ -129,7 +125,7 @@ int encrypt_sanity(struct __sk_buff *skb)
{
struct __crypto_ctx_value *v;
struct bpf_crypto_ctx *ctx;
- struct bpf_dynptr psrc, pdst, iv;
+ struct bpf_dynptr psrc, pdst;
int err;
status = 0;
@@ -156,12 +152,8 @@ int encrypt_sanity(struct __sk_buff *skb)
* production code, a percpu map should be used to store the result.
*/
bpf_dynptr_from_mem(dst, sizeof(dst), 0, &pdst);
- /* iv dynptr has to be initialized with 0 size, but proper memory region
- * has to be provided anyway
- */
- bpf_dynptr_from_mem(dst, 0, 0, &iv);
- status = bpf_crypto_encrypt(ctx, &psrc, &pdst, &iv);
+ status = bpf_crypto_encrypt(ctx, &psrc, &pdst, NULL);
return TC_ACT_SHOT;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH bpf-next 4/4] selftests: bpf: crypto: adjust bench to use nullable IV
2024-05-09 13:40 [PATCH bpf-next 0/4] bpf: make trusted args nullable Vadim Fedorenko
` (2 preceding siblings ...)
2024-05-09 13:40 ` [PATCH bpf-next 3/4] selftests: bpf: crypto: use NULL instead of 0-sized dynptr Vadim Fedorenko
@ 2024-05-09 13:40 ` Vadim Fedorenko
3 siblings, 0 replies; 6+ messages in thread
From: Vadim Fedorenko @ 2024-05-09 13:40 UTC (permalink / raw)
To: Vadim Fedorenko, Martin KaFai Lau, Andrii Nakryiko,
Alexei Starovoitov, Mykola Lysenko, Jakub Kicinski
Cc: Vadim Fedorenko, bpf
The bench shows some improvements, around 4% faster on decrypt.
Before:
Benchmark 'crypto-decrypt' started.
Iter 0 (325.719us): hits 5.105M/s ( 5.105M/prod), drops 0.000M/s, total operations 5.105M/s
Iter 1 (-17.295us): hits 5.224M/s ( 5.224M/prod), drops 0.000M/s, total operations 5.224M/s
Iter 2 ( 5.504us): hits 4.630M/s ( 4.630M/prod), drops 0.000M/s, total operations 4.630M/s
Iter 3 ( 9.239us): hits 5.148M/s ( 5.148M/prod), drops 0.000M/s, total operations 5.148M/s
Iter 4 ( 37.885us): hits 5.198M/s ( 5.198M/prod), drops 0.000M/s, total operations 5.198M/s
Iter 5 (-53.282us): hits 5.167M/s ( 5.167M/prod), drops 0.000M/s, total operations 5.167M/s
Iter 6 (-17.809us): hits 5.186M/s ( 5.186M/prod), drops 0.000M/s, total operations 5.186M/s
Summary: hits 5.092 ± 0.228M/s ( 5.092M/prod), drops 0.000 ±0.000M/s, total operations 5.092 ± 0.228M/s
After:
Benchmark 'crypto-decrypt' started.
Iter 0 (268.912us): hits 5.312M/s ( 5.312M/prod), drops 0.000M/s, total operations 5.312M/s
Iter 1 (124.869us): hits 5.354M/s ( 5.354M/prod), drops 0.000M/s, total operations 5.354M/s
Iter 2 (-36.801us): hits 5.334M/s ( 5.334M/prod), drops 0.000M/s, total operations 5.334M/s
Iter 3 (254.628us): hits 5.334M/s ( 5.334M/prod), drops 0.000M/s, total operations 5.334M/s
Iter 4 (-77.691us): hits 5.275M/s ( 5.275M/prod), drops 0.000M/s, total operations 5.275M/s
Iter 5 (-164.510us): hits 5.313M/s ( 5.313M/prod), drops 0.000M/s, total operations 5.313M/s
Iter 6 (-81.376us): hits 5.346M/s ( 5.346M/prod), drops 0.000M/s, total operations 5.346M/s
Summary: hits 5.326 ± 0.029M/s ( 5.326M/prod), drops 0.000 ±0.000M/s, total operations 5.326 ± 0.029M/s
Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
---
tools/testing/selftests/bpf/progs/crypto_bench.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/crypto_bench.c b/tools/testing/selftests/bpf/progs/crypto_bench.c
index e61fe0882293..4ac956b26240 100644
--- a/tools/testing/selftests/bpf/progs/crypto_bench.c
+++ b/tools/testing/selftests/bpf/progs/crypto_bench.c
@@ -57,7 +57,7 @@ int crypto_encrypt(struct __sk_buff *skb)
{
struct __crypto_ctx_value *v;
struct bpf_crypto_ctx *ctx;
- struct bpf_dynptr psrc, pdst, iv;
+ struct bpf_dynptr psrc, pdst;
v = crypto_ctx_value_lookup();
if (!v) {
@@ -73,9 +73,8 @@ int crypto_encrypt(struct __sk_buff *skb)
bpf_dynptr_from_skb(skb, 0, &psrc);
bpf_dynptr_from_mem(dst, len, 0, &pdst);
- bpf_dynptr_from_mem(dst, 0, 0, &iv);
- status = bpf_crypto_encrypt(ctx, &psrc, &pdst, &iv);
+ status = bpf_crypto_encrypt(ctx, &psrc, &pdst, NULL);
__sync_add_and_fetch(&hits, 1);
return 0;
@@ -84,7 +83,7 @@ int crypto_encrypt(struct __sk_buff *skb)
SEC("tc")
int crypto_decrypt(struct __sk_buff *skb)
{
- struct bpf_dynptr psrc, pdst, iv;
+ struct bpf_dynptr psrc, pdst;
struct __crypto_ctx_value *v;
struct bpf_crypto_ctx *ctx;
@@ -98,9 +97,8 @@ int crypto_decrypt(struct __sk_buff *skb)
bpf_dynptr_from_skb(skb, 0, &psrc);
bpf_dynptr_from_mem(dst, len, 0, &pdst);
- bpf_dynptr_from_mem(dst, 0, 0, &iv);
- status = bpf_crypto_decrypt(ctx, &psrc, &pdst, &iv);
+ status = bpf_crypto_decrypt(ctx, &psrc, &pdst, NULL);
__sync_add_and_fetch(&hits, 1);
return 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread