From: Amery Hung <ameryhung@gmail.com>
To: Kui-Feng Lee <sinquersw@gmail.com>
Cc: netdev@vger.kernel.org, bpf@vger.kernel.org,
yangpeihao@sjtu.edu.cn, daniel@iogearbox.net, andrii@kernel.org,
martin.lau@kernel.org, toke@redhat.com, jhs@mojatatu.com,
jiri@resnulli.us, sdf@google.com, xiyou.wangcong@gmail.com,
yepeilin.cs@gmail.com
Subject: Re: [RFC PATCH v8 02/20] selftests/bpf: Test referenced kptr arguments of struct_ops programs
Date: Fri, 10 May 2024 15:16:32 -0700 [thread overview]
Message-ID: <CAMB2axN3XwSmvk2eC9OnaUk5QvXS6sLVv148NrepkbtjCixVwg@mail.gmail.com> (raw)
In-Reply-To: <b2486867-0fee-4972-ad71-7b54e8a5d2b6@gmail.com>
On Fri, May 10, 2024 at 2:33 PM Kui-Feng Lee <sinquersw@gmail.com> wrote:
>
>
>
> On 5/10/24 12:23, Amery Hung wrote:
> > A reference is automatically acquired for a referenced kptr argument
> > annotated via the stub function with "__ref_acquired" in a struct_ops
> > program. It must be released and cannot be acquired more than once.
> >
> > The test first checks whether a reference to the correct type is acquired
> > in "ref_acquire". Then, we check if the verifier correctly rejects the
> > program that fails to release the reference (i.e., reference leak) in
> > "ref_acquire_ref_leak". Finally, we check if the reference can be only
> > acquired once through the argument in "ref_acquire_dup_ref".
> >
> > Signed-off-by: Amery Hung <amery.hung@bytedance.com>
> > ---
> > .../selftests/bpf/bpf_testmod/bpf_testmod.c | 7 +++
> > .../selftests/bpf/bpf_testmod/bpf_testmod.h | 2 +
> > .../prog_tests/test_struct_ops_ref_acquire.c | 58 +++++++++++++++++++
> > .../bpf/progs/struct_ops_ref_acquire.c | 27 +++++++++
> > .../progs/struct_ops_ref_acquire_dup_ref.c | 24 ++++++++
> > .../progs/struct_ops_ref_acquire_ref_leak.c | 19 ++++++
> > 6 files changed, 137 insertions(+)
> > create mode 100644 tools/testing/selftests/bpf/prog_tests/test_struct_ops_ref_acquire.c
> > create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_ref_acquire.c
> > create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_ref_acquire_dup_ref.c
> > create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_ref_acquire_ref_leak.c
> >
> >
> ... skipped ...
> > +
> > diff --git a/tools/testing/selftests/bpf/progs/struct_ops_ref_acquire.c b/tools/testing/selftests/bpf/progs/struct_ops_ref_acquire.c
> > new file mode 100644
> > index 000000000000..bae342db0fdb
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/struct_ops_ref_acquire.c
> > @@ -0,0 +1,27 @@
> > +#include <vmlinux.h>
> > +#include <bpf/bpf_tracing.h>
> > +#include "../bpf_testmod/bpf_testmod.h"
> > +
> > +char _license[] SEC("license") = "GPL";
> > +
> > +void bpf_task_release(struct task_struct *p) __ksym;
> > +
> > +/* This is a test BPF program that uses struct_ops to access a referenced
> > + * kptr argument. This is a test for the verifier to ensure that it recongnizes
> > + * the task as a referenced object (i.e., ref_obj_id > 0).
> > + */
> > +SEC("struct_ops/test_ref_acquire")
> > +int BPF_PROG(test_ref_acquire, int dummy,
> > + struct task_struct *task)
> > +{
> > + bpf_task_release(task);
>
> This looks weird for me.
>
> According to what you mentioned in the patch 1, the purpose is to
> prevent acquiring multiple references from happening. So, is it possible
> to return NULL from the acquire function if having returned a reference
> before?
The purpose of req_acquired is to allow acquiring a referenced kptr in
struct_ops argument just once. Whether multiple references can be
acquired/duplicated later I think could be orthogonal.
In bpf qdisc, we ensure unique reference of skb through ref_acquired and
the fact that there is no bpf_ref_count in sk_buff (so that users cannot
use bpf_ref_acquire()).
In this case, it is true that programs like below will be able to get
multiple references to task (Is this the scenario you have in mind?).
Thus, if the users want to enforce the unique reference semantic, they
need to make bpf_task_acquire() unavailable as well.
SEC("struct_ops/test_ref_acquire")
int BPF_PROG(test_ref_acquire, int dummy,
struct task_struct *task)
{
struct task_struct task2;
task2 = bpf_task_acquire(task);
bpf_task_release(task);
if (task2)
bpf_task_release(task2);
return 0;
}
>
>
> > +
> > + return 0;
> > +}
> > +
> > +
> ... skipped ...
next prev parent reply other threads:[~2024-05-10 22:16 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-10 19:23 [RFC PATCH v8 00/20] bpf qdisc Amery Hung
2024-05-10 19:23 ` [RFC PATCH v8 01/20] bpf: Support passing referenced kptr to struct_ops programs Amery Hung
2024-05-16 23:59 ` Kumar Kartikeya Dwivedi
2024-05-17 0:17 ` Amery Hung
2024-05-17 0:23 ` Kumar Kartikeya Dwivedi
2024-05-17 1:22 ` Amery Hung
2024-05-17 2:00 ` Kumar Kartikeya Dwivedi
2024-05-10 19:23 ` [RFC PATCH v8 02/20] selftests/bpf: Test referenced kptr arguments of " Amery Hung
2024-05-10 21:33 ` Kui-Feng Lee
2024-05-10 22:16 ` Amery Hung [this message]
2024-05-16 23:14 ` Amery Hung
2024-05-16 23:43 ` Martin KaFai Lau
2024-05-17 0:54 ` Amery Hung
2024-05-17 1:07 ` Martin KaFai Lau
2024-05-10 19:23 ` [RFC PATCH v8 03/20] bpf: Allow struct_ops prog to return referenced kptr Amery Hung
2024-05-17 2:06 ` Amery Hung
2024-05-17 5:30 ` Martin KaFai Lau
2024-05-10 19:23 ` [RFC PATCH v8 04/20] selftests/bpf: Test returning kptr from struct_ops programs Amery Hung
2024-05-10 19:23 ` [RFC PATCH v8 05/20] bpf: Generate btf_struct_metas for kernel BTF Amery Hung
2024-05-10 19:23 ` [RFC PATCH v8 06/20] bpf: Recognize kernel types as graph values Amery Hung
2024-05-10 19:23 ` [RFC PATCH v8 07/20] bpf: Allow adding kernel objects to collections Amery Hung
2024-05-10 19:24 ` [RFC PATCH v8 08/20] selftests/bpf: Test adding kernel object to bpf graph Amery Hung
2024-05-10 19:24 ` [RFC PATCH v8 09/20] bpf: Find special BTF fields in union Amery Hung
2024-05-16 23:37 ` Amery Hung
2024-05-10 19:24 ` [RFC PATCH v8 10/20] bpf: Introduce exclusive-ownership list and rbtree nodes Amery Hung
2024-05-10 19:24 ` [RFC PATCH v8 11/20] bpf: Allow adding exclusive nodes to bpf list and rbtree Amery Hung
2024-05-10 19:24 ` [RFC PATCH v8 12/20] selftests/bpf: Modify linked_list tests to work with macro-ified removes Amery Hung
2024-05-10 19:24 ` [RFC PATCH v8 13/20] bpf: net_sched: Support implementation of Qdisc_ops in bpf Amery Hung
2024-05-10 19:24 ` [RFC PATCH v8 14/20] bpf: net_sched: Add bpf qdisc kfuncs Amery Hung
2024-05-22 23:55 ` Martin KaFai Lau
2024-05-23 1:06 ` Amery Hung
2024-05-10 19:24 ` [RFC PATCH v8 15/20] bpf: net_sched: Allow more optional methods in Qdisc_ops Amery Hung
2024-05-10 19:24 ` [RFC PATCH v8 16/20] libbpf: Support creating and destroying qdisc Amery Hung
2024-05-10 19:24 ` [RFC PATCH v8 17/20] selftests: Add a basic fifo qdisc test Amery Hung
2024-05-21 3:15 ` Stanislav Fomichev
2024-05-21 15:03 ` Amery Hung
2024-05-21 17:57 ` Stanislav Fomichev
2024-05-10 19:24 ` [RFC PATCH v8 18/20] selftests: Add a bpf fq qdisc to selftest Amery Hung
2024-05-10 19:24 ` [RFC PATCH v8 19/20] selftests: Add a bpf netem " Amery Hung
2024-05-10 19:24 ` [RFC PATCH v8 20/20] selftests: Add a prio bpf qdisc Amery Hung
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMB2axN3XwSmvk2eC9OnaUk5QvXS6sLVv148NrepkbtjCixVwg@mail.gmail.com \
--to=ameryhung@gmail.com \
--cc=andrii@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=jhs@mojatatu.com \
--cc=jiri@resnulli.us \
--cc=martin.lau@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=sdf@google.com \
--cc=sinquersw@gmail.com \
--cc=toke@redhat.com \
--cc=xiyou.wangcong@gmail.com \
--cc=yangpeihao@sjtu.edu.cn \
--cc=yepeilin.cs@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).