On Wed, Mar 06, 2024 at 07:40:00AM +0000, Matt Bobrowski wrote: > It is rather common for BPF LSM program types to perform the struct > walk current->mm->exe_file and subsequently operate on fields of the > backing file. At times, some of these operations involve passing a > exe_file's field on to BPF helpers and such > i.e. bpf_d_path(¤t->mm->exe_file->f_path). However, doing so > isn't necessarily always reliable as the backing file that exe_file is > pointing to may be in the midst of being torn down and handing > anything contained within this file to BPF helpers and such can lead > to memory corruption issues [0]. > > To alleviate possibly operating on semi-torn down instances of > current->mm->exe_file we introduce a set of BPF kfuncs that posses > KF_ACQUIRE/KF_RELEASE based semantics. Such BPF kfuncs will allow BPF > LSM program types to reliably get/put a reference on a > current->mm->exe_file. > > The following new BPF kfuncs have been added: > > struct file *bpf_get_task_exe_file(struct task_struct *task); > struct file *bpf_get_mm_exe_file(struct mm_struct *mm); > void bpf_put_file(struct file *f); > > Internally, these new BPF kfuncs simply call the preexisting in-kernel > functions get_task_exe_file(), get_mm_exe_file(), and fput() > accordingly. From a technical standpoint, there's absolutely no need > to re-implement such helpers just for BPF as they're currently scoped > to BPF LSM program types. > > Note that we explicitly do not explicitly rely on the use of very low > level in-kernel functions like get_file_rcu() and get_file_active() to > acquire a reference on current->mm->exe_file and such. This is super > subtle code and we probably want to avoid exposing any such subtleties > to BPF in the form of BPF kfuncs. Additionally, the usage of a double > pointer i.e. struct file **, isn't something that the BPF verifier > currently recognizes nor has any intention to recognize for the > foreseeable future. > > [0] https://lore.kernel.org/bpf/CAG48ez0ppjcT=QxU-jtCUfb5xQb3mLr=5FcwddF_VKfEBPs_Dg@xxxxxxxxxxxxxx/ > > Signed-off-by: Matt Bobrowski <mattbobrowski@xxxxxxxxxx> > --- get_mm_exe_file() and get_task_exe_file() aren't even exported to modules which makes me a lot more hesitant. > kernel/trace/bpf_trace.c | 56 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 56 insertions(+) > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 801808b6efb0..539c58db74d7 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -1518,12 +1518,68 @@ __bpf_kfunc void bpf_mm_drop(struct mm_struct *mm) > mmdrop(mm); > } > > +/** > + * bpf_get_task_exe_file - get a reference on the exe_file associated with the > + * mm_struct that is nested within the supplied > + * task_struct > + * @task: task_struct of which the nested mm_struct's exe_file is to be > + * referenced > + * > + * Get a reference on the exe_file that is associated with the mm_struct nested > + * within the supplied *task*. A reference on a file pointer acquired by this > + * kfunc must be released using bpf_put_file(). Internally, this kfunc leans on > + * get_task_exe_file(), such that calling bpf_get_task_exe_file() would be > + * analogous to calling get_task_exe_file() outside of BPF program context. > + * > + * Return: A referenced pointer to the exe_file associated with the mm_struct > + * nested in the supplied *task*, or NULL. > + */ > +__bpf_kfunc struct file *bpf_get_task_exe_file(struct task_struct *task) > +{ > + return get_task_exe_file(task); > +} > + > +/** > + * bpf_get_mm_exe_file - get a reference on the exe_file for the supplied > + * mm_struct. > + * @mm: mm_struct of which the exe_file to get a reference on > + * > + * Get a reference on the exe_file associated with the supplied *mm*. A > + * reference on a file pointer acquired by this kfunc must be released using > + * bpf_put_file(). Internally, this kfunc leans on get_mm_exe_file(), such that > + * calling bpf_get_mm_exe_file() would be analogous to calling get_mm_exe_file() > + * outside of BPF program context. > + * > + * Return: A referenced file pointer to the exe_file for the supplied *mm*, or > + * NULL. > + */ > +__bpf_kfunc struct file *bpf_get_mm_exe_file(struct mm_struct *mm) > +{ > + return get_mm_exe_file(mm); > +} > + > +/** > + * bpf_put_file - put a reference on the supplied file > + * @f: file of which to put a reference on > + * > + * Put a reference on the supplied *f*. > + */ > +__bpf_kfunc void bpf_put_file(struct file *f) > +{ > + fput(f); > +} That's probably reasonable because this is already exported to modules. But that's not a generic argument. So there'll never be kfuncs for __fput_sync(), flushed_delayed_fput() and other special purpose file put or get helpers. Conditio sine qua non is that all such kfunc definitions must live in the respective file in fs/. They won't be hidden somewhere in kernel/bpf/.
