[PATCH v3 bpf-next 0/3] introduce new VFS based BPF kfuncs

[PATCH v3 bpf-next 0/3] introduce new VFS based BPF kfuncs

[Matt Bobrowski]

G'day!

The original cover letter providing background context and motivating
factors around the needs for these new VFS related BPF kfuncs
introduced within this patch series can be found here [0]. Please do
reference that if needed.

The changes contained within this version of the patch series mainly
came at the back of discussions held with Christian at LSFMMBPF
recently. In summary, the primary difference within this patch series
when compared to the last [1] is that I've reduced the number of VFS
related BPF kfuncs being introduced, housed them under fs/, and added
more selftests.

Changes since v2 [1]:

* All new VFS related BPF kfuncs now reside in fs/bpf_fs_kfuncs.c
  rather than kernel/trace/bpf_trace.c. This was something that was
  explicitly requested by Christian after discussing these new VFS
  related BPF kfuncs recently at LSFMMBPF.
  
* Dropped other initially proposed VFS related BPF kfuncs, including
  bpf_get_mm_exe_file(), bpf_get_task_fs_root(),
  bpf_get_task_fs_pwd(), and bpf_put_path().

* bpf_path_d_path() now makes use of __sz argument annotations such
  that the BPF verifier can enforce relevant size checks on the
  supplied buf that ends up being passed to d_path(). Relevant
  selftests have been added to assert __sz checking semantics are
  enforced.

[0] https://lore.kernel.org/bpf/cover.1708377880.git.mattbobrowski@google.com/
[1] https://lore.kernel.org/bpf/cover.1709675979.git.mattbobrowski@google.com/

Matt Bobrowski (3):
  bpf: introduce new VFS based BPF kfuncs
  selftests/bpf: add negative tests for new VFS based BPF kfuncs
  selftests/bpf: add positive tests for new VFS based BPF kfuncs

 fs/Makefile                                   |   1 +
 fs/bpf_fs_kfuncs.c                            | 133 ++++++++++++
 .../testing/selftests/bpf/bpf_experimental.h  |  26 +++
 .../selftests/bpf/prog_tests/verifier.c       |   4 +
 .../selftests/bpf/progs/verifier_vfs_accept.c |  71 +++++++
 .../selftests/bpf/progs/verifier_vfs_reject.c | 196 ++++++++++++++++++
 6 files changed, 431 insertions(+)
 create mode 100644 fs/bpf_fs_kfuncs.c
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_vfs_accept.c
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_vfs_reject.c

-- 
2.46.0.rc1.232.g9752f9e123-goog

[PATCH v3 bpf-next 1/3] bpf: introduce new VFS based BPF kfuncs

[Matt Bobrowski]

Add a new variant of bpf_d_path() named bpf_path_d_path() which takes
the form of a BPF kfunc and enforces KF_TRUSTED_ARGS semantics onto
its arguments.

This new d_path() based BPF kfunc variant is intended to address the
legacy bpf_d_path() BPF helper's susceptibility to memory corruption
issues [0, 1, 2] by ensuring to only operate on supplied arguments
which are deemed trusted by the BPF verifier. Typically, this means
that only pointers to a struct path which have been referenced counted
may be supplied.

In addition to the new bpf_path_d_path() BPF kfunc, we also add a
KF_ACQUIRE based BPF kfunc bpf_get_task_exe_file() and KF_RELEASE
counterpart BPF kfunc bpf_put_file(). This is so that the new
bpf_path_d_path() BPF kfunc can be used more flexibility from within
the context of a BPF LSM program. It's rather common to ascertain the
backing executable file for the calling process by performing the
following walk current->mm->exe_file while instrumenting a given
operation from the context of the BPF LSM program. However, walking
current->mm->exe_file directly is never deemed to be OK, and doing so
from both inside and outside of BPF LSM program context should be
considered as a bug. Using bpf_get_task_exe_file() and in turn
bpf_put_file() will allow BPF LSM programs to reliably get and put
references to current->mm->exe_file.

As of now, all the newly introduced BPF kfuncs within this patch are
limited to sleepable BPF LSM program types. Therefore, they may only
be called when a BPF LSM program is attached to one of the listed
attachment points defined within the sleepable_lsm_hooks BTF ID set.

[0] https://lore.kernel.org/bpf/CAG48ez0ppjcT=QxU-jtCUfb5xQb3mLr=5FcwddF_VKfEBPs_Dg@mail.gmail.com/
[1] https://lore.kernel.org/bpf/20230606181714.532998-1-jolsa@kernel.org/
[2] https://lore.kernel.org/bpf/20220219113744.1852259-1-memxor@gmail.com/

Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
---
 fs/Makefile        |   1 +
 fs/bpf_fs_kfuncs.c | 133 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 134 insertions(+)
 create mode 100644 fs/bpf_fs_kfuncs.c

diff --git a/fs/Makefile b/fs/Makefile
index 6ecc9b0a53f2..61679fd587b7 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -129,3 +129,4 @@ obj-$(CONFIG_EFIVAR_FS)		+= efivarfs/
 obj-$(CONFIG_EROFS_FS)		+= erofs/
 obj-$(CONFIG_VBOXSF_FS)		+= vboxsf/
 obj-$(CONFIG_ZONEFS_FS)		+= zonefs/
+obj-$(CONFIG_BPF_LSM)		+= bpf_fs_kfuncs.o
diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
new file mode 100644
index 000000000000..3813e2a83313
--- /dev/null
+++ b/fs/bpf_fs_kfuncs.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Google LLC. */
+
+#include <linux/bpf.h>
+#include <linux/btf.h>
+#include <linux/btf_ids.h>
+#include <linux/dcache.h>
+#include <linux/err.h>
+#include <linux/fs.h>
+#include <linux/file.h>
+#include <linux/init.h>
+#include <linux/mm.h>
+#include <linux/path.h>
+#include <linux/sched.h>
+
+__bpf_kfunc_start_defs();
+/**
+ * bpf_get_task_exe_file - get a reference on the exe_file struct file member of
+ *                         the mm_struct that is nested within the supplied
+ *                         task_struct
+ * @task: task_struct of which the nested mm_struct exe_file member to get a
+ * reference on
+ *
+ * Get a reference on the exe_file struct file member field of the mm_struct
+ * nested within the supplied *task*. The referenced file pointer acquired by
+ * this BPF kfunc must be released using bpf_put_file(). Failing to call
+ * bpf_put_file() on the returned referenced struct file pointer that has been
+ * acquired by this BPF kfunc will result in the BPF program being rejected by
+ * the BPF verifier.
+ *
+ * This BPF kfunc may only be called from sleepable BPF LSM programs.
+ *
+ * Internally, this BPF kfunc leans on get_task_exe_file(), such that calling
+ * bpf_get_task_exe_file() would be analogous to calling get_task_exe_file()
+ * directly in kernel context.
+ *
+ * Return: A referenced struct file pointer to the exe_file member of the
+ * mm_struct that is nested within the supplied *task*. On error, NULL is
+ * returned.
+ */
+__bpf_kfunc struct file *bpf_get_task_exe_file(struct task_struct *task)
+{
+	return get_task_exe_file(task);
+}
+
+/**
+ * bpf_put_file - put a reference on the supplied file
+ * @file: file to put a reference on
+ *
+ * Put a reference on the supplied *file*. Only referenced file pointers may be
+ * passed to this BPF kfunc. Attempting to pass an unreferenced file pointer, or
+ * any other arbitrary pointer for that matter, will result in the BPF program
+ * being rejected by the BPF verifier.
+ *
+ * This BPF kfunc may only be called from sleepable BPF LSM programs. Though
+ * fput() can be called from IRQ context, we're enforcing sleepability here.
+ */
+__bpf_kfunc void bpf_put_file(struct file *file)
+{
+	fput(file);
+}
+
+/**
+ * bpf_path_d_path - resolve the pathname for the supplied path
+ * @path: path to resolve the pathname for
+ * @buf: buffer to return the resolved pathname in
+ * @buf__sz: length of the supplied buffer
+ *
+ * Resolve the pathname for the supplied *path* and store it in *buf*. This BPF
+ * kfunc is the safer variant of the legacy bpf_d_path() helper and should be
+ * used in place of bpf_d_path() whenever possible. It enforces KF_TRUSTED_ARGS
+ * semantics, meaning that the supplied *path* must itself hold a valid
+ * reference, or else the BPF program will be outright rejected by the BPF
+ * verifier.
+ *
+ * This BPF kfunc may only be called from sleepable BPF LSM programs.
+ *
+ * Return: A positive integer corresponding to the length of the resolved
+ * pathname in *buf*, including the NUL termination character. On error, a
+ * negative integer is returned.
+ */
+__bpf_kfunc int bpf_path_d_path(struct path *path, char *buf, size_t buf__sz)
+{
+	int len;
+	char *ret;
+
+	if (buf__sz <= 0)
+		return -EINVAL;
+
+	/* Usually, d_path() will never involuntarily put the calling thread to
+        * sleep. However, there could be exceptions to this as d_op->d_dname()
+        * has free rein over what it wants to do. Additionally, given that this
+        * new d_path() based BPF kfunc enforces KF_TRUSTED_ARGS, it'll likely
+        * only ever be called alongside or in similar contexts, to other
+        * supporting BPF kfuncs that may end up being put to sleep.
+        */
+	ret = d_path(path, buf, buf__sz);
+	if (IS_ERR(ret))
+		return PTR_ERR(ret);
+
+	len = buf + buf__sz - ret;
+	memmove(buf, ret, len);
+	return len;
+}
+__bpf_kfunc_end_defs();
+
+BTF_KFUNCS_START(bpf_fs_kfunc_set_ids)
+BTF_ID_FLAGS(func, bpf_get_task_exe_file,
+	     KF_ACQUIRE | KF_TRUSTED_ARGS | KF_SLEEPABLE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_put_file, KF_RELEASE | KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_path_d_path, KF_TRUSTED_ARGS | KF_SLEEPABLE)
+BTF_KFUNCS_END(bpf_fs_kfunc_set_ids)
+
+static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id)
+{
+	if (!btf_id_set8_contains(&bpf_fs_kfunc_set_ids, kfunc_id) ||
+	    prog->type == BPF_PROG_TYPE_LSM)
+		return 0;
+	return -EACCES;
+}
+
+static const struct btf_kfunc_id_set bpf_fs_kfunc_set = {
+	.owner = THIS_MODULE,
+	.set = &bpf_fs_kfunc_set_ids,
+	.filter = bpf_fs_kfuncs_filter,
+};
+
+static int __init bpf_fs_kfuncs_init(void)
+{
+	return register_btf_kfunc_id_set(BPF_PROG_TYPE_LSM, &bpf_fs_kfunc_set);
+}
+
+late_initcall(bpf_fs_kfuncs_init);
-- 
2.46.0.rc1.232.g9752f9e123-goog

Re: [PATCH v3 bpf-next 1/3] bpf: introduce new VFS based BPF kfuncs

[Christian Brauner]

On Fri, Jul 26, 2024 at 08:56:02AM GMT, Matt Bobrowski wrote:
> Add a new variant of bpf_d_path() named bpf_path_d_path() which takes
> the form of a BPF kfunc and enforces KF_TRUSTED_ARGS semantics onto
> its arguments.
> 
> This new d_path() based BPF kfunc variant is intended to address the
> legacy bpf_d_path() BPF helper's susceptibility to memory corruption
> issues [0, 1, 2] by ensuring to only operate on supplied arguments
> which are deemed trusted by the BPF verifier. Typically, this means
> that only pointers to a struct path which have been referenced counted
> may be supplied.
> 
> In addition to the new bpf_path_d_path() BPF kfunc, we also add a
> KF_ACQUIRE based BPF kfunc bpf_get_task_exe_file() and KF_RELEASE
> counterpart BPF kfunc bpf_put_file(). This is so that the new
> bpf_path_d_path() BPF kfunc can be used more flexibility from within
> the context of a BPF LSM program. It's rather common to ascertain the
> backing executable file for the calling process by performing the
> following walk current->mm->exe_file while instrumenting a given
> operation from the context of the BPF LSM program. However, walking
> current->mm->exe_file directly is never deemed to be OK, and doing so
> from both inside and outside of BPF LSM program context should be
> considered as a bug. Using bpf_get_task_exe_file() and in turn
> bpf_put_file() will allow BPF LSM programs to reliably get and put
> references to current->mm->exe_file.
> 
> As of now, all the newly introduced BPF kfuncs within this patch are
> limited to sleepable BPF LSM program types. Therefore, they may only
> be called when a BPF LSM program is attached to one of the listed
> attachment points defined within the sleepable_lsm_hooks BTF ID set.
> 
> [0] https://lore.kernel.org/bpf/CAG48ez0ppjcT=QxU-jtCUfb5xQb3mLr=5FcwddF_VKfEBPs_Dg@mail.gmail.com/
> [1] https://lore.kernel.org/bpf/20230606181714.532998-1-jolsa@kernel.org/
> [2] https://lore.kernel.org/bpf/20220219113744.1852259-1-memxor@gmail.com/
> 
> Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
> ---
>  fs/Makefile        |   1 +
>  fs/bpf_fs_kfuncs.c | 133 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 134 insertions(+)
>  create mode 100644 fs/bpf_fs_kfuncs.c
> 
> diff --git a/fs/Makefile b/fs/Makefile
> index 6ecc9b0a53f2..61679fd587b7 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -129,3 +129,4 @@ obj-$(CONFIG_EFIVAR_FS)		+= efivarfs/
>  obj-$(CONFIG_EROFS_FS)		+= erofs/
>  obj-$(CONFIG_VBOXSF_FS)		+= vboxsf/
>  obj-$(CONFIG_ZONEFS_FS)		+= zonefs/
> +obj-$(CONFIG_BPF_LSM)		+= bpf_fs_kfuncs.o
> diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> new file mode 100644
> index 000000000000..3813e2a83313
> --- /dev/null
> +++ b/fs/bpf_fs_kfuncs.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2024 Google LLC. */
> +
> +#include <linux/bpf.h>
> +#include <linux/btf.h>
> +#include <linux/btf_ids.h>
> +#include <linux/dcache.h>
> +#include <linux/err.h>
> +#include <linux/fs.h>
> +#include <linux/file.h>
> +#include <linux/init.h>
> +#include <linux/mm.h>
> +#include <linux/path.h>
> +#include <linux/sched.h>
> +
> +__bpf_kfunc_start_defs();
> +/**
> + * bpf_get_task_exe_file - get a reference on the exe_file struct file member of
> + *                         the mm_struct that is nested within the supplied
> + *                         task_struct
> + * @task: task_struct of which the nested mm_struct exe_file member to get a
> + * reference on
> + *
> + * Get a reference on the exe_file struct file member field of the mm_struct
> + * nested within the supplied *task*. The referenced file pointer acquired by
> + * this BPF kfunc must be released using bpf_put_file(). Failing to call
> + * bpf_put_file() on the returned referenced struct file pointer that has been
> + * acquired by this BPF kfunc will result in the BPF program being rejected by
> + * the BPF verifier.
> + *
> + * This BPF kfunc may only be called from sleepable BPF LSM programs.
> + *
> + * Internally, this BPF kfunc leans on get_task_exe_file(), such that calling
> + * bpf_get_task_exe_file() would be analogous to calling get_task_exe_file()
> + * directly in kernel context.
> + *
> + * Return: A referenced struct file pointer to the exe_file member of the
> + * mm_struct that is nested within the supplied *task*. On error, NULL is
> + * returned.
> + */
> +__bpf_kfunc struct file *bpf_get_task_exe_file(struct task_struct *task)
> +{
> +	return get_task_exe_file(task);
> +}
> +
> +/**
> + * bpf_put_file - put a reference on the supplied file
> + * @file: file to put a reference on
> + *
> + * Put a reference on the supplied *file*. Only referenced file pointers may be
> + * passed to this BPF kfunc. Attempting to pass an unreferenced file pointer, or
> + * any other arbitrary pointer for that matter, will result in the BPF program
> + * being rejected by the BPF verifier.
> + *
> + * This BPF kfunc may only be called from sleepable BPF LSM programs. Though
> + * fput() can be called from IRQ context, we're enforcing sleepability here.
> + */
> +__bpf_kfunc void bpf_put_file(struct file *file)
> +{
> +	fput(file);
> +}
> +
> +/**
> + * bpf_path_d_path - resolve the pathname for the supplied path
> + * @path: path to resolve the pathname for
> + * @buf: buffer to return the resolved pathname in
> + * @buf__sz: length of the supplied buffer
> + *
> + * Resolve the pathname for the supplied *path* and store it in *buf*. This BPF
> + * kfunc is the safer variant of the legacy bpf_d_path() helper and should be
> + * used in place of bpf_d_path() whenever possible. It enforces KF_TRUSTED_ARGS
> + * semantics, meaning that the supplied *path* must itself hold a valid
> + * reference, or else the BPF program will be outright rejected by the BPF
> + * verifier.
> + *
> + * This BPF kfunc may only be called from sleepable BPF LSM programs.
> + *
> + * Return: A positive integer corresponding to the length of the resolved
> + * pathname in *buf*, including the NUL termination character. On error, a
> + * negative integer is returned.
> + */
> +__bpf_kfunc int bpf_path_d_path(struct path *path, char *buf, size_t buf__sz)
> +{
> +	int len;
> +	char *ret;
> +
> +	if (buf__sz <= 0)
> +		return -EINVAL;

size_t is unsigned so this should just be !buf__sz I can fix that
though. The __sz thing has meaning to the verifier afaict so I guess
that's fine as name then.

Re: [PATCH v3 bpf-next 1/3] bpf: introduce new VFS based BPF kfuncs

[Matt Bobrowski]

On Fri, Jul 26, 2024 at 03:18:25PM +0200, Christian Brauner wrote:
> On Fri, Jul 26, 2024 at 08:56:02AM GMT, Matt Bobrowski wrote:
> > Add a new variant of bpf_d_path() named bpf_path_d_path() which takes
> > the form of a BPF kfunc and enforces KF_TRUSTED_ARGS semantics onto
> > its arguments.
> > 
> > This new d_path() based BPF kfunc variant is intended to address the
> > legacy bpf_d_path() BPF helper's susceptibility to memory corruption
> > issues [0, 1, 2] by ensuring to only operate on supplied arguments
> > which are deemed trusted by the BPF verifier. Typically, this means
> > that only pointers to a struct path which have been referenced counted
> > may be supplied.
> > 
> > In addition to the new bpf_path_d_path() BPF kfunc, we also add a
> > KF_ACQUIRE based BPF kfunc bpf_get_task_exe_file() and KF_RELEASE
> > counterpart BPF kfunc bpf_put_file(). This is so that the new
> > bpf_path_d_path() BPF kfunc can be used more flexibility from within
> > the context of a BPF LSM program. It's rather common to ascertain the
> > backing executable file for the calling process by performing the
> > following walk current->mm->exe_file while instrumenting a given
> > operation from the context of the BPF LSM program. However, walking
> > current->mm->exe_file directly is never deemed to be OK, and doing so
> > from both inside and outside of BPF LSM program context should be
> > considered as a bug. Using bpf_get_task_exe_file() and in turn
> > bpf_put_file() will allow BPF LSM programs to reliably get and put
> > references to current->mm->exe_file.
> > 
> > As of now, all the newly introduced BPF kfuncs within this patch are
> > limited to sleepable BPF LSM program types. Therefore, they may only
> > be called when a BPF LSM program is attached to one of the listed
> > attachment points defined within the sleepable_lsm_hooks BTF ID set.
> > 
> > [0] https://lore.kernel.org/bpf/CAG48ez0ppjcT=QxU-jtCUfb5xQb3mLr=5FcwddF_VKfEBPs_Dg@mail.gmail.com/
> > [1] https://lore.kernel.org/bpf/20230606181714.532998-1-jolsa@kernel.org/
> > [2] https://lore.kernel.org/bpf/20220219113744.1852259-1-memxor@gmail.com/
> > 
> > Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
> > ---
> >  fs/Makefile        |   1 +
> >  fs/bpf_fs_kfuncs.c | 133 +++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 134 insertions(+)
> >  create mode 100644 fs/bpf_fs_kfuncs.c
> > 
> > diff --git a/fs/Makefile b/fs/Makefile
> > index 6ecc9b0a53f2..61679fd587b7 100644
> > --- a/fs/Makefile
> > +++ b/fs/Makefile
> > @@ -129,3 +129,4 @@ obj-$(CONFIG_EFIVAR_FS)		+= efivarfs/
> >  obj-$(CONFIG_EROFS_FS)		+= erofs/
> >  obj-$(CONFIG_VBOXSF_FS)		+= vboxsf/
> >  obj-$(CONFIG_ZONEFS_FS)		+= zonefs/
> > +obj-$(CONFIG_BPF_LSM)		+= bpf_fs_kfuncs.o
> > diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> > new file mode 100644
> > index 000000000000..3813e2a83313
> > --- /dev/null
> > +++ b/fs/bpf_fs_kfuncs.c
> > @@ -0,0 +1,133 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2024 Google LLC. */
> > +
> > +#include <linux/bpf.h>
> > +#include <linux/btf.h>
> > +#include <linux/btf_ids.h>
> > +#include <linux/dcache.h>
> > +#include <linux/err.h>
> > +#include <linux/fs.h>
> > +#include <linux/file.h>
> > +#include <linux/init.h>
> > +#include <linux/mm.h>
> > +#include <linux/path.h>
> > +#include <linux/sched.h>
> > +
> > +__bpf_kfunc_start_defs();
> > +/**
> > + * bpf_get_task_exe_file - get a reference on the exe_file struct file member of
> > + *                         the mm_struct that is nested within the supplied
> > + *                         task_struct
> > + * @task: task_struct of which the nested mm_struct exe_file member to get a
> > + * reference on
> > + *
> > + * Get a reference on the exe_file struct file member field of the mm_struct
> > + * nested within the supplied *task*. The referenced file pointer acquired by
> > + * this BPF kfunc must be released using bpf_put_file(). Failing to call
> > + * bpf_put_file() on the returned referenced struct file pointer that has been
> > + * acquired by this BPF kfunc will result in the BPF program being rejected by
> > + * the BPF verifier.
> > + *
> > + * This BPF kfunc may only be called from sleepable BPF LSM programs.
> > + *
> > + * Internally, this BPF kfunc leans on get_task_exe_file(), such that calling
> > + * bpf_get_task_exe_file() would be analogous to calling get_task_exe_file()
> > + * directly in kernel context.
> > + *
> > + * Return: A referenced struct file pointer to the exe_file member of the
> > + * mm_struct that is nested within the supplied *task*. On error, NULL is
> > + * returned.
> > + */
> > +__bpf_kfunc struct file *bpf_get_task_exe_file(struct task_struct *task)
> > +{
> > +	return get_task_exe_file(task);
> > +}
> > +
> > +/**
> > + * bpf_put_file - put a reference on the supplied file
> > + * @file: file to put a reference on
> > + *
> > + * Put a reference on the supplied *file*. Only referenced file pointers may be
> > + * passed to this BPF kfunc. Attempting to pass an unreferenced file pointer, or
> > + * any other arbitrary pointer for that matter, will result in the BPF program
> > + * being rejected by the BPF verifier.
> > + *
> > + * This BPF kfunc may only be called from sleepable BPF LSM programs. Though
> > + * fput() can be called from IRQ context, we're enforcing sleepability here.
> > + */
> > +__bpf_kfunc void bpf_put_file(struct file *file)
> > +{
> > +	fput(file);
> > +}
> > +
> > +/**
> > + * bpf_path_d_path - resolve the pathname for the supplied path
> > + * @path: path to resolve the pathname for
> > + * @buf: buffer to return the resolved pathname in
> > + * @buf__sz: length of the supplied buffer
> > + *
> > + * Resolve the pathname for the supplied *path* and store it in *buf*. This BPF
> > + * kfunc is the safer variant of the legacy bpf_d_path() helper and should be
> > + * used in place of bpf_d_path() whenever possible. It enforces KF_TRUSTED_ARGS
> > + * semantics, meaning that the supplied *path* must itself hold a valid
> > + * reference, or else the BPF program will be outright rejected by the BPF
> > + * verifier.
> > + *
> > + * This BPF kfunc may only be called from sleepable BPF LSM programs.
> > + *
> > + * Return: A positive integer corresponding to the length of the resolved
> > + * pathname in *buf*, including the NUL termination character. On error, a
> > + * negative integer is returned.
> > + */
> > +__bpf_kfunc int bpf_path_d_path(struct path *path, char *buf, size_t buf__sz)
> > +{
> > +	int len;
> > +	char *ret;
> > +
> > +	if (buf__sz <= 0)
> > +		return -EINVAL;
> 
> size_t is unsigned so this should just be !buf__sz I can fix that
> though.

Sure, that would be great if you wouldn't mind?

> The __sz thing has meaning to the verifier afaict so I guess that's
> fine as name then.

That's right, it's used to signal that a buffer and it's associated
size exists within the BPF kfuncs argument list. Using the __sz
annotation specifically allows the BPF verifier to deduce which size
argument is meant to be bounded to a given buffer.

/M

Re: [PATCH v3 bpf-next 1/3] bpf: introduce new VFS based BPF kfuncs

[Alexei Starovoitov]

On Fri, Jul 26, 2024 at 1:56 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
> +
> +static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id)
> +{
> +       if (!btf_id_set8_contains(&bpf_fs_kfunc_set_ids, kfunc_id) ||
> +           prog->type == BPF_PROG_TYPE_LSM)
> +               return 0;
> +       return -EACCES;
> +}
> +
> +static const struct btf_kfunc_id_set bpf_fs_kfunc_set = {
> +       .owner = THIS_MODULE,
> +       .set = &bpf_fs_kfunc_set_ids,
> +       .filter = bpf_fs_kfuncs_filter,
> +};
> +
> +static int __init bpf_fs_kfuncs_init(void)
> +{
> +       return register_btf_kfunc_id_set(BPF_PROG_TYPE_LSM, &bpf_fs_kfunc_set);
> +}

Aside from buf__sz <= 0 that Christian spotted
the bpf_fs_kfuncs_filter() is a watery water.
It's doing a redundant check that is already covered by

register_btf_kfunc_id_set(BPF_PROG_TYPE_LSM,...

I'll remove it while applying.

Re: [PATCH v3 bpf-next 1/3] bpf: introduce new VFS based BPF kfuncs

[Matt Bobrowski]

On Fri, Jul 26, 2024 at 01:43:45PM -0700, Alexei Starovoitov wrote:
> On Fri, Jul 26, 2024 at 1:56 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
> > +
> > +static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id)
> > +{
> > +       if (!btf_id_set8_contains(&bpf_fs_kfunc_set_ids, kfunc_id) ||
> > +           prog->type == BPF_PROG_TYPE_LSM)
> > +               return 0;
> > +       return -EACCES;
> > +}
> > +
> > +static const struct btf_kfunc_id_set bpf_fs_kfunc_set = {
> > +       .owner = THIS_MODULE,
> > +       .set = &bpf_fs_kfunc_set_ids,
> > +       .filter = bpf_fs_kfuncs_filter,
> > +};
> > +
> > +static int __init bpf_fs_kfuncs_init(void)
> > +{
> > +       return register_btf_kfunc_id_set(BPF_PROG_TYPE_LSM, &bpf_fs_kfunc_set);
> > +}
> 
> Aside from buf__sz <= 0 that Christian spotted

I'm going to fix this up in v2 of this patch, so don't worry about it.

> the bpf_fs_kfuncs_filter() is a watery water.
> It's doing a redundant check that is already covered by
> 
> register_btf_kfunc_id_set(BPF_PROG_TYPE_LSM,...
> 
> I'll remove it while applying.

As discussed, this filter is currently required as without it we
inadvertently allow tracing BPF programs to also use these BPF
kfuncs.

/M

Re: [PATCH v3 bpf-next 1/3] bpf: introduce new VFS based BPF kfuncs

[Song Liu]

On Fri, Jul 26, 2024 at 1:56 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
>
[...]
> +       len = buf + buf__sz - ret;
> +       memmove(buf, ret, len);
> +       return len;
> +}
> +__bpf_kfunc_end_defs();
> +
> +BTF_KFUNCS_START(bpf_fs_kfunc_set_ids)
> +BTF_ID_FLAGS(func, bpf_get_task_exe_file,
> +            KF_ACQUIRE | KF_TRUSTED_ARGS | KF_SLEEPABLE | KF_RET_NULL)
> +BTF_ID_FLAGS(func, bpf_put_file, KF_RELEASE | KF_SLEEPABLE)

Do we really need KF_SLEEPABLE for bpf_put_file?

Thanks,
Song

> +BTF_ID_FLAGS(func, bpf_path_d_path, KF_TRUSTED_ARGS | KF_SLEEPABLE)
> +BTF_KFUNCS_END(bpf_fs_kfunc_set_ids)
> +
[...]

Re: [PATCH v3 bpf-next 1/3] bpf: introduce new VFS based BPF kfuncs

[Matt Bobrowski]

On Fri, Jul 26, 2024 at 02:25:26PM -0700, Song Liu wrote:
> On Fri, Jul 26, 2024 at 1:56 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
> >
> [...]
> > +       len = buf + buf__sz - ret;
> > +       memmove(buf, ret, len);
> > +       return len;
> > +}
> > +__bpf_kfunc_end_defs();
> > +
> > +BTF_KFUNCS_START(bpf_fs_kfunc_set_ids)
> > +BTF_ID_FLAGS(func, bpf_get_task_exe_file,
> > +            KF_ACQUIRE | KF_TRUSTED_ARGS | KF_SLEEPABLE | KF_RET_NULL)
> > +BTF_ID_FLAGS(func, bpf_put_file, KF_RELEASE | KF_SLEEPABLE)
> 
> Do we really need KF_SLEEPABLE for bpf_put_file?

Well, the guts of fput() is annotated w/ might_sleep(), so the calling
thread may presumably be involuntarily put to sleep? You can also see
the guts of fput() invoking various indirect function calls
i.e. ->release(), and depending on the implementation of those, they
could be initiating resource release related actions which
consequently could result in waiting for some I/O to be done? fput()
also calls dput() and mntput() and these too can also do a bunch of
teardown.

Please correct me if I've misunderstood something.

/M

Re: [PATCH v3 bpf-next 1/3] bpf: introduce new VFS based BPF kfuncs

[Song Liu]

On Fri, Jul 26, 2024 at 2:49 PM Matt Bobrowski <mattbobrowski@google.com> wrote:
>
> On Fri, Jul 26, 2024 at 02:25:26PM -0700, Song Liu wrote:
> > On Fri, Jul 26, 2024 at 1:56 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
> > >
> > [...]
> > > +       len = buf + buf__sz - ret;
> > > +       memmove(buf, ret, len);
> > > +       return len;
> > > +}
> > > +__bpf_kfunc_end_defs();
> > > +
> > > +BTF_KFUNCS_START(bpf_fs_kfunc_set_ids)
> > > +BTF_ID_FLAGS(func, bpf_get_task_exe_file,
> > > +            KF_ACQUIRE | KF_TRUSTED_ARGS | KF_SLEEPABLE | KF_RET_NULL)
> > > +BTF_ID_FLAGS(func, bpf_put_file, KF_RELEASE | KF_SLEEPABLE)
> >
> > Do we really need KF_SLEEPABLE for bpf_put_file?
>
> Well, the guts of fput() is annotated w/ might_sleep(), so the calling
> thread may presumably be involuntarily put to sleep? You can also see
> the guts of fput() invoking various indirect function calls
> i.e. ->release(), and depending on the implementation of those, they
> could be initiating resource release related actions which
> consequently could result in waiting for some I/O to be done? fput()
> also calls dput() and mntput() and these too can also do a bunch of
> teardown.
>
> Please correct me if I've misunderstood something.

__fput() is annotated with might_sleep(). However, fput() doesn't not
call __fput() directly. Instead, it schedules a worker to call __fput().
Therefore, it is safe to call fput() from a non-sleepable context.

Thanks,
Song

Re: [PATCH v3 bpf-next 1/3] bpf: introduce new VFS based BPF kfuncs

[Matt Bobrowski]

On Fri, Jul 26, 2024 at 03:48:45PM -0700, Song Liu wrote:
> On Fri, Jul 26, 2024 at 2:49 PM Matt Bobrowski <mattbobrowski@google.com> wrote:
> >
> > On Fri, Jul 26, 2024 at 02:25:26PM -0700, Song Liu wrote:
> > > On Fri, Jul 26, 2024 at 1:56 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
> > > >
> > > [...]
> > > > +       len = buf + buf__sz - ret;
> > > > +       memmove(buf, ret, len);
> > > > +       return len;
> > > > +}
> > > > +__bpf_kfunc_end_defs();
> > > > +
> > > > +BTF_KFUNCS_START(bpf_fs_kfunc_set_ids)
> > > > +BTF_ID_FLAGS(func, bpf_get_task_exe_file,
> > > > +            KF_ACQUIRE | KF_TRUSTED_ARGS | KF_SLEEPABLE | KF_RET_NULL)
> > > > +BTF_ID_FLAGS(func, bpf_put_file, KF_RELEASE | KF_SLEEPABLE)
> > >
> > > Do we really need KF_SLEEPABLE for bpf_put_file?
> >
> > Well, the guts of fput() is annotated w/ might_sleep(), so the calling
> > thread may presumably be involuntarily put to sleep? You can also see
> > the guts of fput() invoking various indirect function calls
> > i.e. ->release(), and depending on the implementation of those, they
> > could be initiating resource release related actions which
> > consequently could result in waiting for some I/O to be done? fput()
> > also calls dput() and mntput() and these too can also do a bunch of
> > teardown.
> >
> > Please correct me if I've misunderstood something.
> 
> __fput() is annotated with might_sleep(). However, fput() doesn't not
> call __fput() directly. Instead, it schedules a worker to call __fput().
> Therefore, it is safe to call fput() from a non-sleepable context.

Oh, yes, you're absolutely right. I failed to realize that, so my
apologies. In that case, yes, technically bpf_put_file() does not need
to be annotated w/ KF_SLEEPABLE. Now that I also think of it, one of
the other and only reasons why we made this initially sleepable is
because bpf_put_file() at the time was meant to be used exclusively
within the same context as bpf_path_d_path(), and that is currently
marked as sleepable. Although technically speaking, I think we could
also make bpf_path_d_path() not restricted to only sleepable BPF LSM
program types, and in turn that could mean that
bpf_get_task_exe_file() also doesn't need to be restricted to
sleepable BPF LSM programs.

Alexei, what do you think about relaxing the sleepable annotation
across this entire set of BPF kfuncs? From a technical perspective I
think it's OK, but I'd also like someone like Christian to confirm
that d_path() can't actually end up sleeping. Glancing over it, I
believe this to be true, but I may also be naively missing something.

/M

Re: [PATCH v3 bpf-next 1/3] bpf: introduce new VFS based BPF kfuncs

[Christian Brauner]

> think it's OK, but I'd also like someone like Christian to confirm
> that d_path() can't actually end up sleeping. Glancing over it, I

We annotated ->d_dname() as non-sleepable in locking.rst so even
->d_dname() shouldn't and curently doesn't sleep. There's a few
codepaths that end up calling d_path() under spinlocks but none of them
should end up calling anything related to ->d_name() and so wouldn't be
affected even if it did sleep.

Re: [PATCH v3 bpf-next 1/3] bpf: introduce new VFS based BPF kfuncs

[Matt Bobrowski]

On Mon, Jul 29, 2024 at 12:56:54PM +0200, Christian Brauner wrote:
> > think it's OK, but I'd also like someone like Christian to confirm
> > that d_path() can't actually end up sleeping. Glancing over it, I
> 
> We annotated ->d_dname() as non-sleepable in locking.rst so even
> ->d_dname() shouldn't and curently doesn't sleep. There's a few
> codepaths that end up calling d_path() under spinlocks but none of them
> should end up calling anything related to ->d_name() and so wouldn't be
> affected even if it did sleep.

Wonderful, exactly what I had also concluded. In that case, I think we
can relax the sleepable requirement across this suite of BPF
kfuncs. Does anyone object?

/M

Re: [PATCH v3 bpf-next 1/3] bpf: introduce new VFS based BPF kfuncs

[Song Liu]

On Fri, Jul 26, 2024 at 1:56 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
>
> Add a new variant of bpf_d_path() named bpf_path_d_path() which takes
> the form of a BPF kfunc and enforces KF_TRUSTED_ARGS semantics onto
> its arguments.
>
> This new d_path() based BPF kfunc variant is intended to address the
> legacy bpf_d_path() BPF helper's susceptibility to memory corruption
> issues [0, 1, 2] by ensuring to only operate on supplied arguments
> which are deemed trusted by the BPF verifier. Typically, this means
> that only pointers to a struct path which have been referenced counted
> may be supplied.
>
> In addition to the new bpf_path_d_path() BPF kfunc, we also add a
> KF_ACQUIRE based BPF kfunc bpf_get_task_exe_file() and KF_RELEASE
> counterpart BPF kfunc bpf_put_file(). This is so that the new
> bpf_path_d_path() BPF kfunc can be used more flexibility from within
> the context of a BPF LSM program. It's rather common to ascertain the
> backing executable file for the calling process by performing the
> following walk current->mm->exe_file while instrumenting a given
> operation from the context of the BPF LSM program. However, walking
> current->mm->exe_file directly is never deemed to be OK, and doing so
> from both inside and outside of BPF LSM program context should be
> considered as a bug. Using bpf_get_task_exe_file() and in turn
> bpf_put_file() will allow BPF LSM programs to reliably get and put
> references to current->mm->exe_file.
>
> As of now, all the newly introduced BPF kfuncs within this patch are
> limited to sleepable BPF LSM program types. Therefore, they may only
> be called when a BPF LSM program is attached to one of the listed
> attachment points defined within the sleepable_lsm_hooks BTF ID set.
>
> [0] https://lore.kernel.org/bpf/CAG48ez0ppjcT=QxU-jtCUfb5xQb3mLr=5FcwddF_VKfEBPs_Dg@mail.gmail.com/
> [1] https://lore.kernel.org/bpf/20230606181714.532998-1-jolsa@kernel.org/
> [2] https://lore.kernel.org/bpf/20220219113744.1852259-1-memxor@gmail.com/
>
> Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>

checkpatch reported a few syntax issues on this one:

https://netdev.bots.linux.dev/static/nipa/874023/13742510/checkpatch/stdout

Re: [PATCH v3 bpf-next 1/3] bpf: introduce new VFS based BPF kfuncs

[Matt Bobrowski]

On Fri, Jul 26, 2024 at 04:52:50PM -0700, Song Liu wrote:
> On Fri, Jul 26, 2024 at 1:56 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
> >
> > Add a new variant of bpf_d_path() named bpf_path_d_path() which takes
> > the form of a BPF kfunc and enforces KF_TRUSTED_ARGS semantics onto
> > its arguments.
> >
> > This new d_path() based BPF kfunc variant is intended to address the
> > legacy bpf_d_path() BPF helper's susceptibility to memory corruption
> > issues [0, 1, 2] by ensuring to only operate on supplied arguments
> > which are deemed trusted by the BPF verifier. Typically, this means
> > that only pointers to a struct path which have been referenced counted
> > may be supplied.
> >
> > In addition to the new bpf_path_d_path() BPF kfunc, we also add a
> > KF_ACQUIRE based BPF kfunc bpf_get_task_exe_file() and KF_RELEASE
> > counterpart BPF kfunc bpf_put_file(). This is so that the new
> > bpf_path_d_path() BPF kfunc can be used more flexibility from within
> > the context of a BPF LSM program. It's rather common to ascertain the
> > backing executable file for the calling process by performing the
> > following walk current->mm->exe_file while instrumenting a given
> > operation from the context of the BPF LSM program. However, walking
> > current->mm->exe_file directly is never deemed to be OK, and doing so
> > from both inside and outside of BPF LSM program context should be
> > considered as a bug. Using bpf_get_task_exe_file() and in turn
> > bpf_put_file() will allow BPF LSM programs to reliably get and put
> > references to current->mm->exe_file.
> >
> > As of now, all the newly introduced BPF kfuncs within this patch are
> > limited to sleepable BPF LSM program types. Therefore, they may only
> > be called when a BPF LSM program is attached to one of the listed
> > attachment points defined within the sleepable_lsm_hooks BTF ID set.
> >
> > [0] https://lore.kernel.org/bpf/CAG48ez0ppjcT=QxU-jtCUfb5xQb3mLr=5FcwddF_VKfEBPs_Dg@mail.gmail.com/
> > [1] https://lore.kernel.org/bpf/20230606181714.532998-1-jolsa@kernel.org/
> > [2] https://lore.kernel.org/bpf/20220219113744.1852259-1-memxor@gmail.com/
> >
> > Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
> 
> checkpatch reported a few syntax issues on this one:
> 
> https://netdev.bots.linux.dev/static/nipa/874023/13742510/checkpatch/stdout

Thanks for making aware, all has been addressed.

/M

[PATCH v3 bpf-next 2/3] selftests/bpf: add negative tests for new VFS based BPF kfuncs

[Matt Bobrowski]

Add a bunch of negative selftests responsible for asserting that the
BPF verifier successfully rejects a BPF program load when the
underlying BPF program misuses one of the newly introduced VFS based
BPF kfuncs.

The following VFS based BPF kfuncs are extensively tested within this
new selftest:

* struct file *bpf_get_task_exe_file(struct task_struct *);
* void bpf_put_file(struct file *);
* int bpf_path_d_path(struct path *, char *, size_t);

Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
---
 .../testing/selftests/bpf/bpf_experimental.h  |  26 +++
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../selftests/bpf/progs/verifier_vfs_reject.c | 196 ++++++++++++++++++
 3 files changed, 224 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_vfs_reject.c

diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
index 828556cdc2f0..8a1ed62b4ed1 100644
--- a/tools/testing/selftests/bpf/bpf_experimental.h
+++ b/tools/testing/selftests/bpf/bpf_experimental.h
@@ -195,6 +195,32 @@ extern void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it) __ksym;
  */
 extern void bpf_throw(u64 cookie) __ksym;
 
+/* Description
+ *	Acquire a reference on the exe_file member field belonging to the
+ *	mm_struct that is nested within the supplied task_struct. The supplied
+ *	task_struct must be trusted/referenced.
+ * Returns
+ *	A referenced file pointer pointing to the exe_file member field of the
+ *	mm_struct nested in the supplied task_struct, or NULL.
+ */
+extern struct file *bpf_get_task_exe_file(struct task_struct *task) __ksym;
+
+/* Description
+ *	Release a reference on the supplied file. The supplied file must be
+ *	trusted/referenced.
+ */
+extern void bpf_put_file(struct file *file) __ksym;
+
+/* Description
+ *	Resolve a pathname for the supplied path and store it in the supplied
+ *	buffer. The supplied path must be trusted/referenced.
+ * Returns
+ *	A positive integer corresponding to the length of the resolved pathname,
+ *	including the NULL termination character, stored in the supplied
+ *	buffer. On error, a negative integer is returned.
+ */
+extern int bpf_path_d_path(struct path *path, char *buf, size_t buf__sz) __ksym;
+
 /* This macro must be used to mark the exception callback corresponding to the
  * main program. For example:
  *
diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index 67a49d12472c..14d74ba2188e 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -85,6 +85,7 @@
 #include "verifier_value_or_null.skel.h"
 #include "verifier_value_ptr_arith.skel.h"
 #include "verifier_var_off.skel.h"
+#include "verifier_vfs_reject.skel.h"
 #include "verifier_xadd.skel.h"
 #include "verifier_xdp.skel.h"
 #include "verifier_xdp_direct_packet_access.skel.h"
@@ -205,6 +206,7 @@ void test_verifier_value(void)                { RUN(verifier_value); }
 void test_verifier_value_illegal_alu(void)    { RUN(verifier_value_illegal_alu); }
 void test_verifier_value_or_null(void)        { RUN(verifier_value_or_null); }
 void test_verifier_var_off(void)              { RUN(verifier_var_off); }
+void test_verifier_vfs_reject(void)	      { RUN(verifier_vfs_reject); }
 void test_verifier_xadd(void)                 { RUN(verifier_xadd); }
 void test_verifier_xdp(void)                  { RUN(verifier_xdp); }
 void test_verifier_xdp_direct_packet_access(void) { RUN(verifier_xdp_direct_packet_access); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
new file mode 100644
index 000000000000..27666a8ef78a
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Google LLC. */
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <linux/limits.h>
+
+#include "bpf_misc.h"
+#include "bpf_experimental.h"
+
+static char buf[PATH_MAX];
+
+SEC("lsm.s/file_open")
+__failure __msg("Possibly NULL pointer passed to trusted arg0")
+int BPF_PROG(get_task_exe_file_kfunc_null)
+{
+	struct file *acquired;
+
+	/* Can't pass a NULL pointer to bpf_get_task_exe_file(). */
+	acquired = bpf_get_task_exe_file(NULL);
+	if (!acquired)
+		return 0;
+
+	bpf_put_file(acquired);
+	return 0;
+}
+
+SEC("lsm.s/inode_getxattr")
+__failure __msg("arg#0 pointer type STRUCT task_struct must point to scalar, or struct with scalar")
+int BPF_PROG(get_task_exe_file_kfunc_fp)
+{
+	u64 x;
+	struct file *acquired;
+	struct task_struct *fp;
+
+	fp = (struct task_struct *)&x;
+	/* Can't pass random frame pointer to bpf_get_task_exe_file(). */
+	acquired = bpf_get_task_exe_file(fp);
+	if (!acquired)
+		return 0;
+
+	bpf_put_file(acquired);
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("R1 must be referenced or trusted")
+int BPF_PROG(get_task_exe_file_kfunc_untrusted)
+{
+	struct file *acquired;
+	struct task_struct *parent;
+
+	/* Walking a trusted struct task_struct returned from
+	 * bpf_get_current_task_btf() yields an untrusted pointer.
+	 */
+	parent = bpf_get_current_task_btf()->parent;
+	/* Can't pass untrusted pointer to bpf_get_task_exe_file(). */
+	acquired = bpf_get_task_exe_file(parent);
+	if (!acquired)
+		return 0;
+
+	bpf_put_file(acquired);
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("Unreleased reference")
+int BPF_PROG(get_task_exe_file_kfunc_unreleased)
+{
+	struct file *acquired;
+
+	acquired = bpf_get_task_exe_file(bpf_get_current_task_btf());
+	if (!acquired)
+		return 0;
+
+	/* Acquired but never released. */
+	return 0;
+}
+
+SEC("lsm/file_open")
+__failure __msg("program must be sleepable to call sleepable kfunc bpf_get_task_exe_file")
+int BPF_PROG(put_file_kfunc_not_sleepable, struct file *file)
+{
+	struct file *acquired;
+
+	/* Can't call bpf_get_task_file() from non-sleepable BPF LSM program
+	 * types.
+	 */
+	acquired = bpf_get_task_exe_file(bpf_get_current_task_btf());
+	if (!acquired)
+		return 0;
+
+	bpf_put_file(acquired);
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("release kernel function bpf_put_file expects")
+int BPF_PROG(put_file_kfunc_unacquired, struct file *file)
+{
+	/* Can't release an unacquired pointer. */
+	bpf_put_file(file);
+	return 0;
+}
+
+SEC("tp_btf/task_newtask")
+__failure __msg("calling kernel function bpf_get_task_exe_file is not allowed")
+int BPF_PROG(get_task_exe_file_kfunc_not_lsm_prog, struct task_struct *task)
+{
+	struct file *acquired;
+
+	/* bpf_get_task_exe_file() can only be called from a sleepable BPF LSM
+	 * program type.
+	 */
+	acquired = bpf_get_task_exe_file(bpf_get_current_task_btf());
+	if (!acquired)
+		return 0;
+
+	bpf_put_file(acquired);
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("Possibly NULL pointer passed to trusted arg0")
+int BPF_PROG(path_d_path_kfunc_null)
+{
+	/* Can't pass NULL value to bpf_path_d_path() kfunc. */
+	bpf_path_d_path(NULL, buf, sizeof(buf));
+	return 0;
+}
+
+SEC("fentry/vfs_open")
+__failure __msg("calling kernel function bpf_path_d_path is not allowed")
+int BPF_PROG(path_d_path_kfunc_non_lsm, struct path *path, struct file *f)
+{
+	/* Calling bpf_path_d_path() from a non-sleepable and non-LSM
+	 * BPF program isn't permitted.
+	 */
+	bpf_path_d_path(path, buf, sizeof(buf));
+	return 0;
+}
+
+SEC("lsm.s/task_alloc")
+__failure __msg("R1 must be referenced or trusted")
+int BPF_PROG(path_d_path_kfunc_untrusted_from_argument, struct task_struct *task)
+{
+	struct path *root;
+
+	/* Walking a trusted argument typically yields an untrusted
+	 * pointer. This is one example of that.
+	 */
+	root = &task->fs->root;
+	bpf_path_d_path(root, buf, sizeof(buf));
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("R1 must be referenced or trusted")
+int BPF_PROG(path_d_path_kfunc_untrusted_from_current)
+{
+	struct path *pwd;
+	struct task_struct *current;
+
+	current = bpf_get_current_task_btf();
+	/* Walking a trusted pointer returned from bpf_get_current_task_btf()
+	 * yields an untrusted pointer.
+	 */
+	pwd = &current->fs->pwd;
+	bpf_path_d_path(pwd, buf, sizeof(buf));
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("kernel function bpf_path_d_path args#0 expected pointer to STRUCT path but R1 has a pointer to STRUCT file")
+int BPF_PROG(path_d_path_kfunc_type_mismatch, struct file *file)
+{
+	bpf_path_d_path((struct path *)&file->f_task_work, buf, sizeof(buf));
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("invalid access to map value, value_size=4096 off=0 size=8192")
+int BPF_PROG(path_d_path_kfunc_invalid_buf_sz, struct file *file)
+{
+	/* bpf_path_d_path() enforces a constraint on the buffer size supplied
+	 * by the BPF LSM program via the __sz annotation. buf here is set to
+	 * PATH_MAX, so let's ensure that the BPF verifier rejects BPF_PROG_LOAD
+	 * attempts if the supplied size and the actual size of the buffer
+	 * mismatches.
+	 */
+	bpf_path_d_path(&file->f_path, buf, PATH_MAX * 2);
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.46.0.rc1.232.g9752f9e123-goog

Re: [PATCH v3 bpf-next 2/3] selftests/bpf: add negative tests for new VFS based BPF kfuncs

[Song Liu]

On Fri, Jul 26, 2024 at 1:56 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
>
> Add a bunch of negative selftests responsible for asserting that the
> BPF verifier successfully rejects a BPF program load when the
> underlying BPF program misuses one of the newly introduced VFS based
> BPF kfuncs.

Negative tests are great. Thanks for adding them.

A few nitpicks below.

[...]

> diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
> index 828556cdc2f0..8a1ed62b4ed1 100644
> --- a/tools/testing/selftests/bpf/bpf_experimental.h
> +++ b/tools/testing/selftests/bpf/bpf_experimental.h
> @@ -195,6 +195,32 @@ extern void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it) __ksym;
>   */
>  extern void bpf_throw(u64 cookie) __ksym;
>
> +/* Description
> + *     Acquire a reference on the exe_file member field belonging to the
> + *     mm_struct that is nested within the supplied task_struct. The supplied
> + *     task_struct must be trusted/referenced.
> + * Returns
> + *     A referenced file pointer pointing to the exe_file member field of the
> + *     mm_struct nested in the supplied task_struct, or NULL.
> + */
> +extern struct file *bpf_get_task_exe_file(struct task_struct *task) __ksym;
> +
> +/* Description
> + *     Release a reference on the supplied file. The supplied file must be
> + *     trusted/referenced.

Probably replace "trusted/referenced" with "acquired".

> + */
> +extern void bpf_put_file(struct file *file) __ksym;
> +
> +/* Description
> + *     Resolve a pathname for the supplied path and store it in the supplied
> + *     buffer. The supplied path must be trusted/referenced.
> + * Returns
> + *     A positive integer corresponding to the length of the resolved pathname,
> + *     including the NULL termination character, stored in the supplied
> + *     buffer. On error, a negative integer is returned.
> + */
> +extern int bpf_path_d_path(struct path *path, char *buf, size_t buf__sz) __ksym;
> +

In my environment, we already have these declarations in vmlinux.h.
So maybe we don't need to add them manually?


>  /* This macro must be used to mark the exception callback corresponding to the
>   * main program. For example:
>   *
> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> index 67a49d12472c..14d74ba2188e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> @@ -85,6 +85,7 @@
>  #include "verifier_value_or_null.skel.h"
>  #include "verifier_value_ptr_arith.skel.h"
>  #include "verifier_var_off.skel.h"
> +#include "verifier_vfs_reject.skel.h"
>  #include "verifier_xadd.skel.h"
>  #include "verifier_xdp.skel.h"
>  #include "verifier_xdp_direct_packet_access.skel.h"
> @@ -205,6 +206,7 @@ void test_verifier_value(void)                { RUN(verifier_value); }
>  void test_verifier_value_illegal_alu(void)    { RUN(verifier_value_illegal_alu); }
>  void test_verifier_value_or_null(void)        { RUN(verifier_value_or_null); }
>  void test_verifier_var_off(void)              { RUN(verifier_var_off); }
> +void test_verifier_vfs_reject(void)          { RUN(verifier_vfs_reject); }
>  void test_verifier_xadd(void)                 { RUN(verifier_xadd); }
>  void test_verifier_xdp(void)                  { RUN(verifier_xdp); }
>  void test_verifier_xdp_direct_packet_access(void) { RUN(verifier_xdp_direct_packet_access); }
> diff --git a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
> new file mode 100644
> index 000000000000..27666a8ef78a
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2024 Google LLC. */
> +
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include <linux/limits.h>
> +
> +#include "bpf_misc.h"
> +#include "bpf_experimental.h"
> +
> +static char buf[PATH_MAX];
> +
> +SEC("lsm.s/file_open")
> +__failure __msg("Possibly NULL pointer passed to trusted arg0")
> +int BPF_PROG(get_task_exe_file_kfunc_null)
> +{
> +       struct file *acquired;
> +
> +       /* Can't pass a NULL pointer to bpf_get_task_exe_file(). */
> +       acquired = bpf_get_task_exe_file(NULL);
> +       if (!acquired)
> +               return 0;
> +
> +       bpf_put_file(acquired);
> +       return 0;
> +}
> +
> +SEC("lsm.s/inode_getxattr")
> +__failure __msg("arg#0 pointer type STRUCT task_struct must point to scalar, or struct with scalar")
> +int BPF_PROG(get_task_exe_file_kfunc_fp)
> +{
> +       u64 x;
> +       struct file *acquired;
> +       struct task_struct *fp;

"fp" is a weird name for a task_struct pointer.

Other than these:

Acked-by: Song Liu <song@kernel.org>

> +
> +       fp = (struct task_struct *)&x;
> +       /* Can't pass random frame pointer to bpf_get_task_exe_file(). */
> +       acquired = bpf_get_task_exe_file(fp);
> +       if (!acquired)
> +               return 0;
> +
> +       bpf_put_file(acquired);
> +       return 0;
> +}
> +
[...]

Re: [PATCH v3 bpf-next 2/3] selftests/bpf: add negative tests for new VFS based BPF kfuncs

[Matt Bobrowski]

On Fri, Jul 26, 2024 at 04:38:32PM -0700, Song Liu wrote:
> On Fri, Jul 26, 2024 at 1:56 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
> >
> > Add a bunch of negative selftests responsible for asserting that the
> > BPF verifier successfully rejects a BPF program load when the
> > underlying BPF program misuses one of the newly introduced VFS based
> > BPF kfuncs.
> 
> Negative tests are great. Thanks for adding them.
> 
> A few nitpicks below.

Thanks for the review!

> > diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
> > index 828556cdc2f0..8a1ed62b4ed1 100644
> > --- a/tools/testing/selftests/bpf/bpf_experimental.h
> > +++ b/tools/testing/selftests/bpf/bpf_experimental.h
> > @@ -195,6 +195,32 @@ extern void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it) __ksym;
> >   */
> >  extern void bpf_throw(u64 cookie) __ksym;
> >
> > +/* Description
> > + *     Acquire a reference on the exe_file member field belonging to the
> > + *     mm_struct that is nested within the supplied task_struct. The supplied
> > + *     task_struct must be trusted/referenced.
> > + * Returns
> > + *     A referenced file pointer pointing to the exe_file member field of the
> > + *     mm_struct nested in the supplied task_struct, or NULL.
> > + */
> > +extern struct file *bpf_get_task_exe_file(struct task_struct *task) __ksym;
> > +
> > +/* Description
> > + *     Release a reference on the supplied file. The supplied file must be
> > + *     trusted/referenced.
> 
> Probably replace "trusted/referenced" with "acquired".

Done.

> > + */
> > +extern void bpf_put_file(struct file *file) __ksym;
> > +
> > +/* Description
> > + *     Resolve a pathname for the supplied path and store it in the supplied
> > + *     buffer. The supplied path must be trusted/referenced.
> > + * Returns
> > + *     A positive integer corresponding to the length of the resolved pathname,
> > + *     including the NULL termination character, stored in the supplied
> > + *     buffer. On error, a negative integer is returned.
> > + */
> > +extern int bpf_path_d_path(struct path *path, char *buf, size_t buf__sz) __ksym;
> > +
> 
> In my environment, we already have these declarations in vmlinux.h.
> So maybe we don't need to add them manually?

Right, but that's probably when building vmlinux.h using the latest
pahole I imagine? Those not using the latest pahole will probably
won't already see these BPF kfuncs within the generated vmlinux.h.

> >  /* This macro must be used to mark the exception callback corresponding to the
> >   * main program. For example:
> >   *
> > diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> > index 67a49d12472c..14d74ba2188e 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> > @@ -85,6 +85,7 @@
> >  #include "verifier_value_or_null.skel.h"
> >  #include "verifier_value_ptr_arith.skel.h"
> >  #include "verifier_var_off.skel.h"
> > +#include "verifier_vfs_reject.skel.h"
> >  #include "verifier_xadd.skel.h"
> >  #include "verifier_xdp.skel.h"
> >  #include "verifier_xdp_direct_packet_access.skel.h"
> > @@ -205,6 +206,7 @@ void test_verifier_value(void)                { RUN(verifier_value); }
> >  void test_verifier_value_illegal_alu(void)    { RUN(verifier_value_illegal_alu); }
> >  void test_verifier_value_or_null(void)        { RUN(verifier_value_or_null); }
> >  void test_verifier_var_off(void)              { RUN(verifier_var_off); }
> > +void test_verifier_vfs_reject(void)          { RUN(verifier_vfs_reject); }
> >  void test_verifier_xadd(void)                 { RUN(verifier_xadd); }
> >  void test_verifier_xdp(void)                  { RUN(verifier_xdp); }
> >  void test_verifier_xdp_direct_packet_access(void) { RUN(verifier_xdp_direct_packet_access); }
> > diff --git a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
> > new file mode 100644
> > index 000000000000..27666a8ef78a
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
> > @@ -0,0 +1,196 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2024 Google LLC. */
> > +
> > +#include <vmlinux.h>
> > +#include <bpf/bpf_helpers.h>
> > +#include <bpf/bpf_tracing.h>
> > +#include <linux/limits.h>
> > +
> > +#include "bpf_misc.h"
> > +#include "bpf_experimental.h"
> > +
> > +static char buf[PATH_MAX];
> > +
> > +SEC("lsm.s/file_open")
> > +__failure __msg("Possibly NULL pointer passed to trusted arg0")
> > +int BPF_PROG(get_task_exe_file_kfunc_null)
> > +{
> > +       struct file *acquired;
> > +
> > +       /* Can't pass a NULL pointer to bpf_get_task_exe_file(). */
> > +       acquired = bpf_get_task_exe_file(NULL);
> > +       if (!acquired)
> > +               return 0;
> > +
> > +       bpf_put_file(acquired);
> > +       return 0;
> > +}
> > +
> > +SEC("lsm.s/inode_getxattr")
> > +__failure __msg("arg#0 pointer type STRUCT task_struct must point to scalar, or struct with scalar")
> > +int BPF_PROG(get_task_exe_file_kfunc_fp)
> > +{
> > +       u64 x;
> > +       struct file *acquired;
> > +       struct task_struct *fp;
> 
> "fp" is a weird name for a task_struct pointer.

OK, just want to make it clear that it was a pointer to something that
exists on the current stack frame. Happy to change the name to task or
something. Done.

> Other than these:
> 
> Acked-by: Song Liu <song@kernel.org>
> 
> > +
> > +       fp = (struct task_struct *)&x;
> > +       /* Can't pass random frame pointer to bpf_get_task_exe_file(). */
> > +       acquired = bpf_get_task_exe_file(fp);
> > +       if (!acquired)
> > +               return 0;
> > +
> > +       bpf_put_file(acquired);
> > +       return 0;
> > +}
> > +
> [...]
/M

[PATCH v3 bpf-next 3/3] selftests/bpf: add positive tests for new VFS based BPF kfuncs

[Matt Bobrowski]

Add a bunch of positive selftests which extensively cover the various
contexts and parameters in which the new VFS based BPF kfuncs may be
used from.

Again, the following VFS based BPF kfuncs are thoroughly tested within
this new selftest:
* struct file *bpf_get_task_exe_file(struct task_struct *);
* void bpf_put_file(struct file *);
* int bpf_path_d_path(struct path *, char *, size_t);

Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
---
 .../selftests/bpf/prog_tests/verifier.c       |  2 +
 .../selftests/bpf/progs/verifier_vfs_accept.c | 71 +++++++++++++++++++
 2 files changed, 73 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_vfs_accept.c

diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index 14d74ba2188e..f8f546eba488 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -85,6 +85,7 @@
 #include "verifier_value_or_null.skel.h"
 #include "verifier_value_ptr_arith.skel.h"
 #include "verifier_var_off.skel.h"
+#include "verifier_vfs_accept.skel.h"
 #include "verifier_vfs_reject.skel.h"
 #include "verifier_xadd.skel.h"
 #include "verifier_xdp.skel.h"
@@ -206,6 +207,7 @@ void test_verifier_value(void)                { RUN(verifier_value); }
 void test_verifier_value_illegal_alu(void)    { RUN(verifier_value_illegal_alu); }
 void test_verifier_value_or_null(void)        { RUN(verifier_value_or_null); }
 void test_verifier_var_off(void)              { RUN(verifier_var_off); }
+void test_verifier_vfs_accept(void)	      { RUN(verifier_vfs_accept); }
 void test_verifier_vfs_reject(void)	      { RUN(verifier_vfs_reject); }
 void test_verifier_xadd(void)                 { RUN(verifier_xadd); }
 void test_verifier_xdp(void)                  { RUN(verifier_xdp); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_vfs_accept.c b/tools/testing/selftests/bpf/progs/verifier_vfs_accept.c
new file mode 100644
index 000000000000..55deb96a4421
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_vfs_accept.c
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Google LLC. */
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+#include "bpf_misc.h"
+#include "bpf_experimental.h"
+
+static char buf[64];
+
+SEC("lsm.s/file_open")
+__success
+int BPF_PROG(get_task_exe_file_and_put_kfunc_from_current)
+{
+	struct file *acquired;
+
+	acquired = bpf_get_task_exe_file(bpf_get_current_task_btf());
+	if (!acquired)
+		return 0;
+
+	bpf_put_file(acquired);
+	return 0;
+}
+
+SEC("lsm.s/task_alloc")
+__success
+int BPF_PROG(get_task_exe_file_and_put_kfunc_from_argument,
+	     struct task_struct *task)
+{
+	struct file *acquired;
+
+	acquired = bpf_get_task_exe_file(task);
+	if (!acquired)
+		return 0;
+
+	bpf_put_file(acquired);
+	return 0;
+}
+
+SEC("lsm.s/inode_getattr")
+__success
+int BPF_PROG(path_d_path_from_path_argument, struct path *path)
+{
+	int ret;
+
+	ret = bpf_path_d_path(path, buf, sizeof(buf));
+	__sink(ret);
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__success
+int BPF_PROG(path_d_path_from_file_argument, struct file *file)
+{
+	int ret;
+	struct path *path;
+
+	/* The f_path member is a path which is embedded directly within a
+	 * file. Therefore, a pointer to such embedded members are still
+	 * recognized by the BPF verifier as being PTR_TRUSTED as it's
+	 * essentially PTR_TRUSTED w/ a non-zero fixed offset.
+	 */
+	path = &file->f_path;
+	ret = bpf_path_d_path(path, buf, sizeof(buf));
+	__sink(ret);
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.46.0.rc1.232.g9752f9e123-goog

Re: [PATCH v3 bpf-next 3/3] selftests/bpf: add positive tests for new VFS based BPF kfuncs

[Song Liu]

On Fri, Jul 26, 2024 at 1:56 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
>
> Add a bunch of positive selftests which extensively cover the various
> contexts and parameters in which the new VFS based BPF kfuncs may be
> used from.
>
> Again, the following VFS based BPF kfuncs are thoroughly tested within
> this new selftest:
> * struct file *bpf_get_task_exe_file(struct task_struct *);
> * void bpf_put_file(struct file *);
> * int bpf_path_d_path(struct path *, char *, size_t);
>
> Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>

Acked-by: Song Liu <song@kernel.org>

Re: [PATCH v3 bpf-next 0/3] introduce new VFS based BPF kfuncs

[Christian Brauner]

On Fri, Jul 26, 2024 at 08:56:01AM GMT, Matt Bobrowski wrote:
> G'day!
> 
> The original cover letter providing background context and motivating
> factors around the needs for these new VFS related BPF kfuncs
> introduced within this patch series can be found here [0]. Please do
> reference that if needed.
> 
> The changes contained within this version of the patch series mainly
> came at the back of discussions held with Christian at LSFMMBPF
> recently. In summary, the primary difference within this patch series
> when compared to the last [1] is that I've reduced the number of VFS
> related BPF kfuncs being introduced, housed them under fs/, and added
> more selftests.

I have no complaints about this now that it's been boiled down.
So as far as I'm concerned I'm happy to pick this up. (I also wouldn't
mind follow-up patches that move the xattr bpf kfuncs under fs/ as
well.)

Re: [PATCH v3 bpf-next 0/3] introduce new VFS based BPF kfuncs

[Matt Bobrowski]

On Fri, Jul 26, 2024 at 03:22:09PM +0200, Christian Brauner wrote:
> On Fri, Jul 26, 2024 at 08:56:01AM GMT, Matt Bobrowski wrote:
> > G'day!
> > 
> > The original cover letter providing background context and motivating
> > factors around the needs for these new VFS related BPF kfuncs
> > introduced within this patch series can be found here [0]. Please do
> > reference that if needed.
> > 
> > The changes contained within this version of the patch series mainly
> > came at the back of discussions held with Christian at LSFMMBPF
> > recently. In summary, the primary difference within this patch series
> > when compared to the last [1] is that I've reduced the number of VFS
> > related BPF kfuncs being introduced, housed them under fs/, and added
> > more selftests.
> 
> I have no complaints about this now that it's been boiled down.
> So as far as I'm concerned I'm happy to pick this up. (I also wouldn't
> mind follow-up patches that move the xattr bpf kfuncs under fs/ as
> well.)

Wonderful, thank you Christian!

I agree, those should also reside in alongside these newly added BPF
kfuncs. I'll send through a patch addressing this
separately. Generally, I think the same applies for any other VFS
related BPF kfuncs that end up getting introduced moving forward.

/M

Re: [PATCH v3 bpf-next 0/3] introduce new VFS based BPF kfuncs

[Alexei Starovoitov]

On Fri, Jul 26, 2024 at 6:22 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Fri, Jul 26, 2024 at 08:56:01AM GMT, Matt Bobrowski wrote:
> > G'day!
> >
> > The original cover letter providing background context and motivating
> > factors around the needs for these new VFS related BPF kfuncs
> > introduced within this patch series can be found here [0]. Please do
> > reference that if needed.
> >
> > The changes contained within this version of the patch series mainly
> > came at the back of discussions held with Christian at LSFMMBPF
> > recently. In summary, the primary difference within this patch series
> > when compared to the last [1] is that I've reduced the number of VFS
> > related BPF kfuncs being introduced, housed them under fs/, and added
> > more selftests.
>
> I have no complaints about this now that it's been boiled down.
> So as far as I'm concerned I'm happy to pick this up.

We very much prefer to go standard route via bpf-next
like we do for all kfuncs to avoid conflicts in selftests,
and where these patches will be actively tested by CI and developers.

So please provide an Ack.
I can fix up <= while applying.

> (I also wouldn't
> mind follow-up patches that move the xattr bpf kfuncs under fs/ as
> well.)

np. I'm sure Song can move xattr kfunc to this newly added file.

Re: [PATCH v3 bpf-next 0/3] introduce new VFS based BPF kfuncs

[Christian Brauner]

Acked-by: Christian Brauner <brauner@kernel.org>