[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ca2ca170-b76c-4fc3-aa44-cae4a1e25aa4@linux.dev>
Date: Wed, 24 Jan 2024 17:26:24 -0800
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Vadim Fedorenko <vadfed@...a.com>
Cc: netdev@...r.kernel.org, linux-crypto@...r.kernel.org,
bpf@...r.kernel.org, Victor Stewart <v@...etag.social>,
Vadim Fedorenko <vadim.fedorenko@...ux.dev>, Jakub Kicinski
<kuba@...nel.org>, Andrii Nakryiko <andrii@...nel.org>,
Alexei Starovoitov <ast@...nel.org>, Mykola Lysenko <mykolal@...com>,
Herbert Xu <herbert@...dor.apana.org.au>
Subject: Re: [PATCH bpf-next v8 3/3] selftests: bpf: crypto skcipher algo
selftests
On 1/15/24 2:08 PM, Vadim Fedorenko wrote:
> +static void deinit_afalg(void)
> +{
> + if (tfmfd)
The test should be (tfmfd != -1) ?
> + close(tfmfd);
> + if (opfd)
Same here.
> + close(opfd);
> +}
[ ... ]
> +SEC("?fentry.s/bpf_fentry_test1")
> +__failure __msg("Unreleased reference")
The error message is not checked. Take a look at how other tests use RUN_TESTS.
> +int BPF_PROG(crypto_acquire)
> +{
> + struct bpf_crypto_ctx *cctx;
> + struct bpf_dynptr key = {};
> + int err = 0;
> +
> + status = 0;
> +
> + bpf_dynptr_from_mem(crypto_key, sizeof(crypto_key), 0, &key);
> + cctx = bpf_crypto_ctx_create("skcipher", "ecb(aes)", &key, 0, &err);
> +
> + if (!cctx) {
> + status = err;
> + return 0;
> + }
> +
> + cctx = bpf_crypto_ctx_acquire(cctx);
> + if (!cctx)
> + return -EINVAL;
> +
> + bpf_crypto_ctx_release(cctx);
> +
> + return 0;
> +}
> +
> +SEC("tc")
> +int decrypt_sanity(struct __sk_buff *skb)
> +{
> + struct __crypto_ctx_value *v;
> + struct bpf_crypto_ctx *ctx;
> + struct bpf_dynptr psrc, pdst, iv;
> + int err;
> +
> + err = skb_dynptr_validate(skb, &psrc);
> + if (err < 0) {
> + status = err;
> + return TC_ACT_SHOT;
> + }
> +
> + v = crypto_ctx_value_lookup();
> + if (!v) {
> + status = -ENOENT;
> + return TC_ACT_SHOT;
> + }
> +
> + ctx = v->ctx;
> + if (!ctx) {
> + status = -ENOENT;
> + return TC_ACT_SHOT;
> + }
> +
> + 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);
It would be nice to allow passing NULL as an optional "iv" arg. It could be a
future improvement.
Overall lgtm. Please add a cover letter in v4 and also the benchmark test that
was brought up a while back.
> +
> + status = bpf_crypto_decrypt(ctx, &psrc, &pdst, &iv);
> +
> + return TC_ACT_SHOT;
> +}
> +
> +SEC("tc")
> +int encrypt_sanity(struct __sk_buff *skb)
> +{
> + struct __crypto_ctx_value *v;
> + struct bpf_crypto_ctx *ctx;
> + struct bpf_dynptr psrc, pdst, iv;
> + int err;
> +
> + status = 0;
> +
> + err = skb_dynptr_validate(skb, &psrc);
> + if (err < 0) {
> + status = err;
> + return TC_ACT_SHOT;
> + }
> +
> + v = crypto_ctx_value_lookup();
> + if (!v) {
> + status = -ENOENT;
> + return TC_ACT_SHOT;
> + }
> +
> + ctx = v->ctx;
> + if (!ctx) {
> + status = -ENOENT;
> + return TC_ACT_SHOT;
> + }
> +
> + 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);
> +
> + return TC_ACT_SHOT;
> +}
> +
> +char __license[] SEC("license") = "GPL";
Powered by blists - more mailing lists