linux-kernel - Re: [PATCH bpf-next 2/3] bpf: Remove KF_KPTR

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] [day] [month] [year] [list]

Message-ID: <20230415230831.pkpk5qtgjynap62c@macbook-pro-6.dhcp.thefacebook.com>
Date:   Sat, 15 Apr 2023 16:08:31 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...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@...a.com,
        memxor@...il.com
Subject: Re: [PATCH bpf-next 2/3] bpf: Remove KF_KPTR_GET kfunc flag

On Sat, Apr 15, 2023 at 05:32:30AM -0500, David Vernet wrote:
> We've managed to improve the UX for kptrs significantly over the last 9
> months. All of the existing use cases which previously had KF_KPTR_GET
> kfuncs (struct bpf_cpumask *, struct task_struct *, and struct cgroup *)
> have all been updated to be synchronized using RCU. In other words,
> their KF_KPTR_GET kfuncs have been removed in favor of KF_RU |

typo: KF_RCU.
same typo is in cover letter.

> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 495250162422..7721c90ead5b 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -18,7 +18,6 @@
>  #define KF_ACQUIRE	(1 << 0) /* kfunc is an acquire function */
>  #define KF_RELEASE	(1 << 1) /* kfunc is a release function */
>  #define KF_RET_NULL	(1 << 2) /* kfunc returns a pointer that may be NULL */
> -#define KF_KPTR_GET	(1 << 3) /* kfunc returns reference to a kptr */
>  /* Trusted arguments are those which are guaranteed to be valid when passed to
>   * the kfunc. It is used to enforce that pointers obtained from either acquire
>   * kfuncs, or from the main kernel on a tracepoint or struct_ops callback
> @@ -67,14 +66,14 @@
>   *	return 0;
>   * }
>   */
> -#define KF_TRUSTED_ARGS (1 << 4) /* kfunc only takes trusted pointer arguments */
> -#define KF_SLEEPABLE    (1 << 5) /* kfunc may sleep */
> -#define KF_DESTRUCTIVE  (1 << 6) /* kfunc performs destructive actions */
> -#define KF_RCU          (1 << 7) /* kfunc takes either rcu or trusted pointer arguments */
> +#define KF_TRUSTED_ARGS (1 << 3) /* kfunc only takes trusted pointer arguments */
> +#define KF_SLEEPABLE    (1 << 4) /* kfunc may sleep */
> +#define KF_DESTRUCTIVE  (1 << 5) /* kfunc performs destructive actions */
> +#define KF_RCU          (1 << 6) /* kfunc takes either rcu or trusted pointer arguments */
>  /* only one of KF_ITER_{NEW,NEXT,DESTROY} could be specified per kfunc */
> -#define KF_ITER_NEW     (1 << 8) /* kfunc implements BPF iter constructor */
> -#define KF_ITER_NEXT    (1 << 9) /* kfunc implements BPF iter next method */
> -#define KF_ITER_DESTROY (1 << 10) /* kfunc implements BPF iter destructor */
> +#define KF_ITER_NEW     (1 << 7) /* kfunc implements BPF iter constructor */
> +#define KF_ITER_NEXT    (1 << 8) /* kfunc implements BPF iter next method */
> +#define KF_ITER_DESTROY (1 << 9) /* kfunc implements BPF iter destructor */

Great cleanup! but let's not renumber flags to reduce the churn.
Just delete #define KF_KPTR_GET  (1 << 3)
I bet soon enough we will add another KF_ flag in that place.