On Wed, Mar 06, 2024 at 07:39:31AM +0000, Matt Bobrowski wrote:
> A BPF LSM program will at times introspect the mm_struct that is
> nested within a given task_struct. Such introspection performed by a
> BPF LSM program may involve reading virtual addresses out from fields
> like arg_start/arg_end and env_start/env_end, or reading fields
> directly out from the backing exe_file. In order to perform reliable
> reads against fields contained within mm_struct, we need to introduce
> a new set of BPF kfuncs that have the ability to acquire and release
> references on the mm_struct that is nested within a task_struct.
>
> The following BPF kfuncs have been added in order to support this
> capability:
>
> struct mm_struct *bpf_task_mm_grab(struct task_struct *task);
> void bpf_mm_drop(struct mm_struct *mm);
>
> These new BPF kfuncs are pretty self-explanatory, but in kernel terms
> bpf_task_mm_grab() effectively allows you to get a reference on the
> mm_struct nested within a supplied task_struct. Whereas, bpf_mm_drop()
> allows you put a reference on a previously gotten mm_struct
> reference. Both BPF kfuncs are also backed by BPF's respective
> KF_ACQUIRE/KF_RELEASE semantics, ensuring that the BPF program behaves
> in accordance to the constraints enforced upon it when operating on
> reference counted in-kernel data structures.
>
> Notably, these newly added BPF kfuncs are simple wrappers around the
> mmgrab() and mmdrop() in-kernel helpers. Both mmgrab() and mmdrop()
> are used in favour of their somewhat similar counterparts mmget() and
> mmput() as they're considered to be the more lightweight variants in
> comparison, and there's no requirement to also pin the underlying
> address spaces just yet.
>
> Signed-off-by: Matt Bobrowski <mattbobrowski@xxxxxxxxxx>
> ---
That's not something I can in any way ACK or NAK. That's clearly mm.
And same question as in the other mail. What's the user of this? I find
it extremly strange that the justification is "some LSM program" needs
this. This is really an annoying justification when we can't even see
the users. With LSMs we can at least see what they're doing with this in
their hooks.
> kernel/trace/bpf_trace.c | 47 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index f639663ac339..801808b6efb0 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1473,10 +1473,57 @@ __bpf_kfunc int bpf_get_file_xattr(struct file *file, const char *name__str,
> return __vfs_getxattr(dentry, dentry->d_inode, name__str, value, value_len);
> }
>
> +/**
> + * bpf_task_mm_grab - get a reference on the mm_struct nested within the
> + * supplied task_struct
> + * @task: task_struct nesting the mm_struct that is to be referenced
> + *
> + * Grab a reference on the mm_struct that is nested within the supplied
> + * *task*. This kfunc will return NULL for threads that do not possess a valid
> + * mm_struct. For example, those that are flagged as PF_KTHREAD. A reference on
> + * a mm_struct acquired by this kfunc must be released using bpf_mm_drop().
> + *
> + * This helper only pins the mm_struct and not necessarily the address space
> + * associated with the referenced mm_struct that is returned from this
> + * kfunc. Internally, this kfunc leans on mmgrab(), such that calling
> + * bpf_task_mm_grab() would be analogous to calling mmgrab() outside of BPF
> + * program context.
> + *
> + * Return: A referenced pointer to the mm_struct nested within the supplied
> + * *task*, or NULL.
> + */
> +__bpf_kfunc struct mm_struct *bpf_task_mm_grab(struct task_struct *task)
> +{
> + struct mm_struct *mm;
> +
> + task_lock(task);
> + mm = task->mm;
> + if (likely(mm))
> + mmgrab(mm);
> + task_unlock(task);
> +
> + return mm;
> +}
> +
> +/**
> + * bpf_mm_drop - put a reference on the supplied mm_struct
> + * @mm: mm_struct of which to put a reference on
> + *
> + * Put a reference on the supplied *mm*. This kfunc internally leans on
> + * mmdrop(), such that calling bpf_mm_drop() would be analogous to calling
> + * mmdrop() outside of BPF program context.
> + */
> +__bpf_kfunc void bpf_mm_drop(struct mm_struct *mm)
> +{
> + mmdrop(mm);
> +}
> +
> __bpf_kfunc_end_defs();
>
> BTF_KFUNCS_START(lsm_kfunc_set_ids)
> BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
> +BTF_ID_FLAGS(func, bpf_task_mm_grab, KF_ACQUIRE | KF_TRUSTED_ARGS | KF_RET_NULL);
> +BTF_ID_FLAGS(func, bpf_mm_drop, KF_RELEASE);
> BTF_KFUNCS_END(lsm_kfunc_set_ids)
>
> static int bpf_lsm_kfunc_filter(const struct bpf_prog *prog, u32 kfunc_id)
> --
> 2.44.0.278.ge034bb2e1d-goog
>
> /M
