On Thu, Sep 11, 2025 at 01:30:52PM -0400, Liam R. Howlett wrote:
> * Yafang Shao <laoar.shao@xxxxxxxxx> [250909 22:46]:
> > The vma->vm_mm might be NULL and it can be accessed outside of RCU. Thus,
> > we can mark it as trusted_or_null. With this change, BPF helpers can safely
> > access vma->vm_mm to retrieve the associated mm_struct from the VMA.
> > Then we can make policy decision from the VMA.
>
> I don't agree with any of that statement.
>
> How are you getting a vma outside an rcu lock safely?
I'm guessing he means that kernel code might access it outside of RCU?
vma->vm_mm can be NULL for 'special' mappings, no not that special, not the
other special, the VDSO special, yeah that one.
get_vma_name() in fs/proc/task_mmu.c does:
if (!vma->vm_mm) {
*name = "[vdso]";
return;
}
Not sure you'd ever find a way to bump into that in THP code paths though ofc.
I was reassured in the last version of the series that the MM is definitely safe
to access safe to access
E.g. https://lore.kernel.org/linux-mm/299e12dc-259b-45c2-8662-2f3863479939@lucifer.local/
https://lore.kernel.org/linux-mm/5fb8bd8d-cdd9-42e0-b62d-eb5a517a35c2@lucifer.local/
And it _seems_ BPF can already access VMA's.
I think everything's under RCU, and there's automatically an RCU lock applied
for anything BPF-ish.
So my A-b was all baed on this kind of hand waving...
>
> vmas are RCU type safe so I don't think you can make the statement of
> null or trusted. You can get a vma that has moved to another mm if you
> are not careful.
>
> What am I missing? Surely there is more context to add to this commit
> message.
Suspect it's the BPF-magic that's the confusing bit...
>
> >
> > The lsm selftest must be modified because it directly accesses vma->vm_mm
> > without a NULL pointer check; otherwise it will break due to this
> > change.
> >
> > For the VMA based THP policy, the use case is as follows,
> >
> > @mm = @vma->vm_mm; // vm_area_struct::vm_mm is trusted or null
> > if (!@mm)
> > return;
> > bpf_rcu_read_lock(); // rcu lock must be held to dereference the owner
> > @owner = @mm->owner; // mm_struct::owner is rcu trusted or null
> > if (!@owner)
> > goto out;
> > @cgroup1 = bpf_task_get_cgroup1(@owner, MEMCG_HIERARCHY_ID);
> >
> > /* make the decision based on the @cgroup1 attribute */
> >
> > bpf_cgroup_release(@cgroup1); // release the associated cgroup
> > out:
> > bpf_rcu_read_unlock();
> >
> > PSI memory information can be obtained from the associated cgroup to inform
> > policy decisions. Since upstream PSI support is currently limited to cgroup
> > v2, the following example demonstrates cgroup v2 implementation:
> >
> > @owner = @mm->owner;
> > if (@owner) {
> > // @ancestor_cgid is user-configured
> > @ancestor = bpf_cgroup_from_id(@ancestor_cgid);
> > if (bpf_task_under_cgroup(@owner, @ancestor)) {
> > @psi_group = @ancestor->psi;
> >
> > /* Extract PSI metrics from @psi_group and
> > * implement policy logic based on the values
> > */
> >
> > }
> > }
> >
> > Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> > Cc: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
> > ---
> > kernel/bpf/verifier.c | 5 +++++
> > tools/testing/selftests/bpf/progs/lsm.c | 8 +++++---
> > 2 files changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index d400e18ee31e..b708b98f796c 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -7165,6 +7165,10 @@ BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct socket) {
> > struct sock *sk;
> > };
> >
> > +BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct vm_area_struct) {
> > + struct mm_struct *vm_mm;
> > +};
> > +
> > static bool type_is_rcu(struct bpf_verifier_env *env,
> > struct bpf_reg_state *reg,
> > const char *field_name, u32 btf_id)
> > @@ -7206,6 +7210,7 @@ static bool type_is_trusted_or_null(struct bpf_verifier_env *env,
> > {
> > BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct socket));
> > BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct dentry));
> > + BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct vm_area_struct));
> >
> > return btf_nested_type_is_trusted(&env->log, reg, field_name, btf_id,
> > "__safe_trusted_or_null");
> > diff --git a/tools/testing/selftests/bpf/progs/lsm.c b/tools/testing/selftests/bpf/progs/lsm.c
> > index 0c13b7409947..7de173daf27b 100644
> > --- a/tools/testing/selftests/bpf/progs/lsm.c
> > +++ b/tools/testing/selftests/bpf/progs/lsm.c
> > @@ -89,14 +89,16 @@ SEC("lsm/file_mprotect")
> > int BPF_PROG(test_int_hook, struct vm_area_struct *vma,
> > unsigned long reqprot, unsigned long prot, int ret)
> > {
> > - if (ret != 0)
> > + struct mm_struct *mm = vma->vm_mm;
> > +
> > + if (ret != 0 || !mm)
> > return ret;
> >
> > __s32 pid = bpf_get_current_pid_tgid() >> 32;
> > int is_stack = 0;
> >
> > - is_stack = (vma->vm_start <= vma->vm_mm->start_stack &&
> > - vma->vm_end >= vma->vm_mm->start_stack);
> > + is_stack = (vma->vm_start <= mm->start_stack &&
> > + vma->vm_end >= mm->start_stack);
> >
> > if (is_stack && monitored_pid == pid) {
> > mprotect_count++;
> > --
> > 2.47.3
> >
> >
