linux-kernel - Re: [PATCH v5 2/3] bpf: Add kfuncs for storing struct task

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 18 Oct 2022 07:25:46 +0530
From:   Kumar Kartikeya Dwivedi <memxor@...il.com>
To:     David Vernet <void@...ifault.com>
Cc:     bpf@...r.kernel.org, ast@...nel.org, daniel@...earbox.net,
        andrii@...nel.org, martin.lau@...ux.dev, song@...nel.org,
        yhs@...com, john.fastabend@...il.com, kpsingh@...nel.org,
        sdf@...gle.com, haoluo@...gle.com, jolsa@...nel.org,
        linux-kernel@...r.kernel.org, kernel-team@...com, tj@...nel.org
Subject: Re: [PATCH v5 2/3] bpf: Add kfuncs for storing struct task_struct *
 as a kptr

On Sat, 15 Oct 2022 at 02:51, David Vernet <void@...ifault.com> 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@...ifault.com>
> ---
>  [...]
> +/**
> + * 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.