[v2,bpf-next,6/9] bpf: add acquire/release based BPF kfuncs for fs_struct's paths

Commit Message

Matt Bobrowski March 6, 2024, 7:40 a.m. UTC

Add the ability to obtain a reference on the root and pwd paths which
are nested within the fs_struct associated with a supplied
task_struct. Both fs_struct's root and pwd are commonly operated on in
BPF LSM program types and at times are further handed off to BPF
helpers and such. There needs to be a mechanism that supports BPF LSM
program types the ability to obtain stable handles to such paths in
order to avoid possible memory corruption bugs [0].

We provide this mechanism through the introduction of the following
new KF_ACQUIRE/KF_RELEASE BPF kfuncs:

struct path *bpf_get_task_fs_root(struct task_struct *task);
struct path *bpf_get_task_fs_pwd(struct task_struct *task);
void bpf_put_path(struct path *path);

Note that bpf_get_task_fs_root() and bpf_get_task_fs_pwd() are
effectively open-coded variants of the in-kernel helpers get_fs_root()
and get_fs_pwd(). We don't lean on these in-kernel helpers directly
within the newly introduced BPF kfuncs as leaning on them would be
rather awkward as we're wanting to return referenced path pointers
directly BPF LSM program types.

[0] https://lore.kernel.org/bpf/CAG48ez0ppjcT=QxU-jtCUfb5xQb3mLr=[e-mail]/

Signed-off-by: Matt Bobrowski <[e-mail]>
---
 kernel/trace/bpf_trace.c | 83 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

Comments

Christian Brauner March 6, 2024, 11:47 a.m. UTC | #1

On Wed, Mar 06, 2024 at 07:40:12AM +0000, Matt Bobrowski wrote:
> Add the ability to obtain a reference on the root and pwd paths which
> are nested within the fs_struct associated with a supplied
> task_struct. Both fs_struct's root and pwd are commonly operated on in
> BPF LSM program types and at times are further handed off to BPF
> helpers and such. There needs to be a mechanism that supports BPF LSM
> program types the ability to obtain stable handles to such paths in
> order to avoid possible memory corruption bugs [0].
> 
> We provide this mechanism through the introduction of the following
> new KF_ACQUIRE/KF_RELEASE BPF kfuncs:
> 
> struct path *bpf_get_task_fs_root(struct task_struct *task);
> struct path *bpf_get_task_fs_pwd(struct task_struct *task);
> void bpf_put_path(struct path *path);
> 
> Note that bpf_get_task_fs_root() and bpf_get_task_fs_pwd() are

Right now all I'm seeing are requests for exporting a bunch of helpers
with no clear explanation other than "This is common in BPF LSM
programs.". So not going to happen if this is some private users pet bpf
program. Where's that bpf lsm program that has to use this?

> effectively open-coded variants of the in-kernel helpers get_fs_root()
> and get_fs_pwd(). We don't lean on these in-kernel helpers directly
> within the newly introduced BPF kfuncs as leaning on them would be
> rather awkward as we're wanting to return referenced path pointers
> directly BPF LSM program types.
> 
> [0] https://lore.kernel.org/bpf/CAG48ez0ppjcT=QxU-jtCUfb5xQb3mLr=[e-mail]/
> 
> Signed-off-by: Matt Bobrowski <[e-mail]>
> ---
>  kernel/trace/bpf_trace.c | 83 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 539c58db74d7..84fd87ead20c 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -10,6 +10,7 @@
>  #include <linux/bpf_perf_event.h>
>  #include <linux/btf.h>
>  #include <linux/filter.h>
> +#include <linux/fs_struct.h>
>  #include <linux/uaccess.h>
>  #include <linux/ctype.h>
>  #include <linux/kprobes.h>
> @@ -1569,6 +1570,83 @@ __bpf_kfunc void bpf_put_file(struct file *f)
>  	fput(f);
>  }
>  
> +/**
> + * bpf_get_task_fs_root - get a reference on the fs_struct's root path for the
> + * 			  supplied task_struct
> + * @Task: task_struct of which the fs_struct's root path to get a reference on
> + *
> + * Get a reference on the root path nested within the fs_struct of the
> + * associated *task*. The referenced path retruned from this kfunc must be
> + * released using bpf_put_path().
> + *
> + * Return: A referenced path pointer to the root path nested within the
> + * fs_struct of the supplied *task*, or NULL.
> + */
> +__bpf_kfunc struct path *bpf_get_task_fs_root(struct task_struct *task)
> +{
> +	struct path *root;
> +	struct fs_struct *fs;
> +
> +	task_lock(task);
> +	fs = task->fs;
> +	if (unlikely(fs)) {
> +		task_unlock(task);
> +		return NULL;
> +	}
> +
> +	spin_lock(&fs->lock);
> +	root = &fs->root;
> +	path_get(root);
> +	spin_unlock(&fs->lock);
> +	task_unlock(task);
> +
> +	return root;
> +}
> +
> +/**
> + * bpf_get_task_fs_pwd - get a reference on the fs_struct's pwd path for the
> + * 			 supplied task_struct
> + * @task: task_struct of which the fs_struct's pwd path to get a reference on
> + *
> + * Get a reference on the pwd path nested within the fs_struct of the associated
> + * *task*. The referenced path retruned from this kfunc must be released using
> + * bpf_put_path().
> + *
> + * Return: A referenced path pointer to the root path nested within the
> + * fs_struct of the supplied *task*, or NULL.
> + */
> +__bpf_kfunc struct path *bpf_get_task_fs_pwd(struct task_struct *task)
> +{
> +	struct path *pwd;
> +	struct fs_struct *fs;
> +
> +	task_lock(task);
> +	fs = task->fs;
> +	if (unlikely(fs)) {
> +		task_unlock(task);
> +		return NULL;
> +	}
> +
> +	spin_lock(&fs->lock);
> +	pwd = &fs->pwd;
> +	path_get(pwd);
> +	spin_unlock(&fs->lock);
> +	task_unlock(task);
> +
> +	return pwd;
> +}
> +
> +/**
> + * bpf_put_path - put a reference on the supplied path
> + * @path: path of which to put a reference on
> + *
> + * Put a reference on the supplied *path*.
> +  */
> +__bpf_kfunc void bpf_put_path(struct path *path)
> +{
> +	path_put(path);
> +}

Probably ok since it's exported to modules but same condition as
mentioned in my earlier mail.

Patch

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 539c58db74d7..84fd87ead20c 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -10,6 +10,7 @@ 
 #include <linux/bpf_perf_event.h>
 #include <linux/btf.h>
 #include <linux/filter.h>
+#include <linux/fs_struct.h>
 #include <linux/uaccess.h>
 #include <linux/ctype.h>
 #include <linux/kprobes.h>
@@ -1569,6 +1570,83 @@  __bpf_kfunc void bpf_put_file(struct file *f)
 	fput(f);
 }
 
+/**
+ * bpf_get_task_fs_root - get a reference on the fs_struct's root path for the
+ * 			  supplied task_struct
+ * @Task: task_struct of which the fs_struct's root path to get a reference on
+ *
+ * Get a reference on the root path nested within the fs_struct of the
+ * associated *task*. The referenced path retruned from this kfunc must be
+ * released using bpf_put_path().
+ *
+ * Return: A referenced path pointer to the root path nested within the
+ * fs_struct of the supplied *task*, or NULL.
+ */
+__bpf_kfunc struct path *bpf_get_task_fs_root(struct task_struct *task)
+{
+	struct path *root;
+	struct fs_struct *fs;
+
+	task_lock(task);
+	fs = task->fs;
+	if (unlikely(fs)) {
+		task_unlock(task);
+		return NULL;
+	}
+
+	spin_lock(&fs->lock);
+	root = &fs->root;
+	path_get(root);
+	spin_unlock(&fs->lock);
+	task_unlock(task);
+
+	return root;
+}
+
+/**
+ * bpf_get_task_fs_pwd - get a reference on the fs_struct's pwd path for the
+ * 			 supplied task_struct
+ * @task: task_struct of which the fs_struct's pwd path to get a reference on
+ *
+ * Get a reference on the pwd path nested within the fs_struct of the associated
+ * *task*. The referenced path retruned from this kfunc must be released using
+ * bpf_put_path().
+ *
+ * Return: A referenced path pointer to the root path nested within the
+ * fs_struct of the supplied *task*, or NULL.
+ */
+__bpf_kfunc struct path *bpf_get_task_fs_pwd(struct task_struct *task)
+{
+	struct path *pwd;
+	struct fs_struct *fs;
+
+	task_lock(task);
+	fs = task->fs;
+	if (unlikely(fs)) {
+		task_unlock(task);
+		return NULL;
+	}
+
+	spin_lock(&fs->lock);
+	pwd = &fs->pwd;
+	path_get(pwd);
+	spin_unlock(&fs->lock);
+	task_unlock(task);
+
+	return pwd;
+}
+
+/**
+ * bpf_put_path - put a reference on the supplied path
+ * @path: path of which to put a reference on
+ *
+ * Put a reference on the supplied *path*.
+  */
+__bpf_kfunc void bpf_put_path(struct path *path)
+{
+	path_put(path);
+}
+
 __bpf_kfunc_end_defs();
 
 BTF_KFUNCS_START(lsm_kfunc_set_ids)
@@ -1580,6 +1658,11 @@  BTF_ID_FLAGS(func, bpf_get_task_exe_file,
 BTF_ID_FLAGS(func, bpf_get_mm_exe_file,
 	     KF_ACQUIRE | KF_TRUSTED_ARGS | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_put_file, KF_RELEASE | KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_get_task_fs_root,
+	     KF_ACQUIRE | KF_TRUSTED_ARGS | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_get_task_fs_pwd,
+	     KF_ACQUIRE | KF_TRUSTED_ARGS | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_put_path, KF_RELEASE | KF_SLEEPABLE)
 BTF_KFUNCS_END(lsm_kfunc_set_ids)
 
 static int bpf_lsm_kfunc_filter(const struct bpf_prog *prog, u32 kfunc_id)