Re: [PATCH v5 2/3] bpf: Add kfuncs for storing struct task_struct * as a kptr

[Kumar Kartikeya Dwivedi]

On Sat, 15 Oct 2022 at 02:51, David Vernet <void@xxxxxxxxxxxxx> wrote:
>
> Now that BPF supports adding new kernel functions with kfuncs, and
> storing kernel objects in maps with kptrs, we can add a set of kfuncs
> which allow struct task_struct objects to be stored in maps as
> referenced kptrs. The possible use cases for doing this are plentiful.
> During tracing, for example, it would be useful to be able to collect
> some tasks that performed a certain operation, and then periodically
> summarize who they are, which cgroup they're in, how much CPU time
> they've utilized, etc.
>
> In order to enable this, this patch adds three new kfuncs:
>
> struct task_struct *bpf_task_acquire(struct task_struct *p);
> struct task_struct *bpf_task_kptr_get(struct task_struct **pp);
> void bpf_task_release(struct task_struct *p);
>
> A follow-on patch will add selftests validating these kfuncs.
>
> Signed-off-by: David Vernet <void@xxxxxxxxxxxxx>
> ---
>  [...]
> +/**
> + * bpf_task_release - Release the reference acquired on a struct task_struct *.
> + * If this kfunc is invoked in an RCU read region, the task_struct is
> + * guaranteed to not be freed until the current grace period has ended, even if
> + * its refcount drops to 0.
> + * @p: The task on which a reference is being released.
> + */
> +__used noinline void bpf_task_release(struct task_struct *p)
> +{
> +       if (!p)
> +               return;
> +
> +       put_task_struct_rcu_user(p);
> +}
> +
> +__diag_pop();
> +
> +BTF_SET8_START(generic_kfunc_btf_ids)
>  #ifdef CONFIG_KEXEC_CORE
>  BTF_ID_FLAGS(func, crash_kexec, KF_DESTRUCTIVE)
>  #endif
> -BTF_SET8_END(tracing_btf_ids)
> +BTF_ID_FLAGS(func, bpf_task_acquire, KF_ACQUIRE | KF_RET_NULL | KF_TRUSTED_ARGS)
> +BTF_ID_FLAGS(func, bpf_task_kptr_get, KF_ACQUIRE | KF_KPTR_GET | KF_RET_NULL)
> +BTF_ID_FLAGS(func, bpf_task_release, KF_RELEASE | KF_TRUSTED_ARGS)

You can remove KF_TRUSTED_ARGS here for bpf_task_release, if this is
required, it would be a bug.