Re: [PATCH bpf-next 7/9] fork: Let the new task point to its mm before uprobes can find it
flat view
From: sashiko-bot@kernel.org
Date: 2026-10-22 02:23:47
Also in:
bpf, linux-mm, linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] fork: clear p->mm under task_lock() before mmput() in bad_fork_cleanup_mm -- commit ec1e952207350f8b07e6038358df3fabd0496801 Author: Alexei Starovoitov [off-list ref] fork: Let the new task point to its mm before uprobes can find it This commit sets tsk->mm under task_lock() at the end of dup_mmap() before uprobe_end_dup_mmap() releases dup_mmap_sem, so that uprobe filters on inherited perf events can match the child's mm. It also clears tsk->mm under task_lock() if dup_mm() fails after dup_mmap() succeeds.
quoted hunk ↗ jump to hunk
diff --git a/kernel/fork.c b/kernel/fork.c index 2e125623cdbc3..a1042e370bdb8 100644 --- a/kernel/fork.c +++ b/kernel/fork.c@@ -1537,7 +1537,7 @@ static struct mm_struct *dup_mm(struct task_struct *tsk, if (!mm_init(mm, tsk)) goto fail_nomem; - err = dup_mmap(mm, oldmm); + err = dup_mmap(tsk, mm, oldmm); if (err) goto free_pt;@@ -1550,6 +1550,13 @@ static struct mm_struct *dup_mm(struct task_struct *tsk, return mm; free_pt: + /* + * dup_mmap() has set it if it succeeded. get_task_mm() must not find + * it after the mmput() below. + */ + task_lock(tsk); + tsk->mm = NULL; + task_unlock(tsk); /* don't put binfmt in mmput, we haven't got module yet */ mm->binfmt = NULL; mm_init_owner(mm, NULL);
[Severity: High]
dup_mm() clears tsk->mm under task_lock(tsk) before calling mmput(mm) so
that get_task_mm() cannot find a freed mm when dup_mm() fails. However,
what happens if copy_process() fails after copy_mm() has already succeeded?
In kernel/fork.c:copy_process(), perf_event_init_task() runs before
copy_mm() and links inherited perf events (with event->hw.target = p) onto
parent_event->child_list and uprobe filter->perf_events:
kernel/fork.c:copy_process() {
...
retval = perf_event_init_task(p, clone_flags);
if (retval)
goto bad_fork_sched_cancel_fork;
...
retval = copy_mm(clone_flags, p);
if (retval)
goto bad_fork_cleanup_signal;
If a later step in copy_process() fails (for example, copy_namespaces(),
copy_io(), copy_thread(), alloc_pid(), cgroup_can_fork(), or
fatal_signal_pending(current) after waiting in dup_userfaultfd_complete()),
execution jumps to bad_fork_cleanup_mm:
bad_fork_cleanup_mm:
sched_cache_fork_cleanup(p);
if (p->mm) {
mm_clear_owner(p->mm, p);
mmput(p->mm);
}
...
bad_fork_cleanup_security:
security_task_free(p);
bad_fork_cleanup_audit:
audit_free(p);
bad_fork_cleanup_perf:
perf_event_free_task(p);
Here mmput(p->mm) drops the reference and frees the mm, but p->mm is left
non-null while teardown continues through exit_fs(p), exit_files(p), and
security_task_free(p) before perf_event_free_task(p) detaches the inherited
events.
During this window, can a concurrent caller of get_task_mm(p) (such as
perf_event_set_addr_filter() -> perf_event_for_each_child() ->
perf_event_addr_filters_apply() in kernel/events/core.c) or bpf helpers like
bpf_find_vma() and bpf_iter_task_vma_new() in kernel/bpf/task_iter.c (for
instance from security_task_free(p)) observe the non-null p->mm under
task_lock(p) and call mmget() on the already-freed mm_struct?
Similarly, when perf_event_free_task(p) -> uprobe_perf_close() runs (or a
concurrent uprobe_perf_filter() runs), trace_uprobe_filter_remove() and
__uprobe_perf_filter() in kernel/trace/trace_uprobe.c read
event->hw.target->mm and compare that dangling pointer against active
mm_structs.
Should bad_fork_cleanup_mm in copy_process() also clear p->mm under
task_lock(p) before calling mmput()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261022015712.1297331-1-alexei.starovoitov@gmail.com?part=7