Re: [PATCH bpf-next v3 6/7] bpf: Mark linux_binprm->mm as trusted-or-null

[bot+bpf-ci]

> bpf: Mark linux_binprm->mm as trusted-or-null
>
> Mark linux_binprm->mm as a trusted-or-null nested pointer so BPF programs
> can pass it to kfuncs after a NULL check. The linux_binprm owns this mm
> throughout the bprm_check_security hook and clears the pointer only after
> exec_mmap() installs the new address space.

Is the changelog's description of when bprm->mm is cleared accurate?

Enumerating every write to bprm->mm in the tree shows three places where
the pointer is cleared, not just "after exec_mmap()":

  fs/exec.c:bprm_mm_init()
      bprm->mm = mm = mm_alloc();          /* assign */
      ...
      bprm->mm = NULL;                     /* error path, before mmdrop */

  fs/exec.c:begin_new_exec()
      bprm->mm = NULL;                     /* after successful exec_mmap() */

  fs/exec.c:free_bprm()
      bprm->mm = NULL;                     /* before mmput() */

Two of these three are not accounted for by the changelog. The
bprm_mm_init() error path clears the pointer before mmdrop(), and
free_bprm() clears it before mmput(). The free_bprm() case is actually
load-bearing: it's what commit d3633d366b08f ("exec: Clear bprm->mm
before dropping its reference") in this same series was added for, and
that commit's changelog says it's "required for eBPF kfuncs where
bprm->mm is either NULL or points to a live mm_struct."

The real invariant this verifier change depends on is "bprm->mm is
always either NULL or a live mm," established across all of fs/exec.c.
The changelog instead asserts a narrower property that doesn't hold.

This matters because the marking isn't scoped to bprm_check_security.
Any BPF program with a trusted linux_binprm pointer (all five bprm_*
LSM hooks in include/linux/lsm_hook_defs.h, plus fentry/fexit/tp_btf on
any kernel function taking a linux_binprm) now gets a trusted mm.
Justifying it from one hook leaves a reviewer unable to check the other
attach points.

Could the changelog drop "only" and state the actual invariant, perhaps
something like: "bprm->mm is either NULL or points to a live mm_struct
at every point a BPF program can observe a linux_binprm: exec_mmap()
transfers ownership and begin_new_exec() then clears the field, and
free_bprm() clears it before dropping the reference (see the preceding
patch)"?

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 386401fe051d4..584c79326b9c7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -6012,6 +6016,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 linux_binprm));
                 ^^^^
>  	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,

Does the patch ordering break bisect here?

The preceding commit in the series, 146a2bd992c3d ("selftests/bpf: Cover
trusted-or-null BTF pointer reads"), adds a test in
tools/testing/selftests/bpf/progs/verifier_lsm.c that expects this
marking to exist:

  SEC("lsm.s/bprm_check_security")
  __description("store through trusted-or-null bprm->mm is rejected")
  __failure
  __msg("R{{[0-9]+}} invalid mem access 'trusted_ptr_or_null_'")
  int BPF_PROG(store_through_trusted_or_null_bprm_mm,
               struct linux_binprm *bprm)
  {
          bprm->mm->task_size = 0;
          return 0;
  }

At commit 146a2bd992c3d, walking bprm->mm in a sleepable LSM program
takes the final else arm of check_ptr_to_btf_access() (flag =
PTR_UNTRUSTED) because in_rcu_cs() is false. The subsequent store fails
at the 'if (atype != BPF_READ && bpf_may_fault_on_deref(reg->type))'
check with "only read is supported" rather than "invalid mem access
'trusted_ptr_or_null_'". The __msg() assertion doesn't match and
'test_progs -t verifier_lsm' fails at that intermediate commit.

Swapping the order makes every commit in the series independently
testable. The ordering issue is present in the posted series as well
(lore message-ids 20260831092305.42062-6 and -7).


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33379004067