Skip to content

Commit 6abde43

Browse files
author
Rafael Aquini
committed
fork: avoid inappropriate uprobe access to invalid mm
JIRA: https://issues.redhat.com/browse/RHEL-84184 Conflicts: * kernel/fork.c: minor difference from upstream due to an extra blank line that was left behind when commit d240629 ("fork: use __mt_dup() to duplicate maple tree in dup_mmap()") was backported into RHEL-9 This patch is a backport of the following upstream commit: commit 8ac662f Author: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> Date: Tue Dec 10 17:24:12 2024 +0000 fork: avoid inappropriate uprobe access to invalid mm If dup_mmap() encounters an issue, currently uprobe is able to access the relevant mm via the reverse mapping (in build_map_info()), and if we are very unlucky with a race window, observe invalid XA_ZERO_ENTRY state which we establish as part of the fork error path. This occurs because uprobe_write_opcode() invokes anon_vma_prepare() which in turn invokes find_mergeable_anon_vma() that uses a VMA iterator, invoking vma_iter_load() which uses the advanced maple tree API and thus is able to observe XA_ZERO_ENTRY entries added to dup_mmap() in commit d240629 ("fork: use __mt_dup() to duplicate maple tree in dup_mmap()"). This change was made on the assumption that only process tear-down code would actually observe (and make use of) these values. However this very unlikely but still possible edge case with uprobes exists and unfortunately does make these observable. The uprobe operation prevents races against the dup_mmap() operation via the dup_mmap_sem semaphore, which is acquired via uprobe_start_dup_mmap() and dropped via uprobe_end_dup_mmap(), and held across register_for_each_vma() prior to invoking build_map_info() which does the reverse mapping lookup. Currently these are acquired and dropped within dup_mmap(), which exposes the race window prior to error handling in the invoking dup_mm() which tears down the mm. We can avoid all this by just moving the invocation of uprobe_start_dup_mmap() and uprobe_end_dup_mmap() up a level to dup_mm() and only release this lock once the dup_mmap() operation succeeds or clean up is done. This means that the uprobe code can never observe an incompletely constructed mm and resolves the issue in this case. Link: https://lkml.kernel.org/r/20241210172412.52995-1-lorenzo.stoakes@oracle.com Fixes: d240629 ("fork: use __mt_dup() to duplicate maple tree in dup_mmap()") Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> Reported-by: syzbot+2d788f4f7cb660dac4b7@syzkaller.appspotmail.com Closes: https://lore.kernel.org/all/6756d273.050a0220.2477f.003d.GAE@google.com/ Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> Cc: Ian Rogers <irogers@google.com> Cc: Ingo Molnar <mingo@redhat.com> Cc: Jann Horn <jannh@google.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Kan Liang <kan.liang@linux.intel.com> Cc: Liam R. Howlett <Liam.Howlett@Oracle.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Masami Hiramatsu <mhiramat@kernel.org> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Oleg Nesterov <oleg@redhat.com> Cc: Peng Zhang <zhangpeng.00@bytedance.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Vlastimil Babka <vbabka@suse.cz> Cc: David Hildenbrand <david@redhat.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Rafael Aquini <raquini@redhat.com>
1 parent 0d76045 commit 6abde43

File tree

1 file changed

+6
-8
lines changed

1 file changed

+6
-8
lines changed

kernel/fork.c

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -655,12 +655,8 @@ static __latent_entropy int dup_mmap(struct mm_struct *mm,
655655
LIST_HEAD(uf);
656656
VMA_ITERATOR(vmi, mm, 0);
657657

658-
uprobe_start_dup_mmap();
659-
if (mmap_write_lock_killable(oldmm)) {
660-
retval = -EINTR;
661-
goto fail_uprobe_end;
662-
}
663-
658+
if (mmap_write_lock_killable(oldmm))
659+
return -EINTR;
664660
flush_cache_dup_mm(oldmm);
665661
uprobe_dup_mmap(oldmm, mm);
666662
/*
@@ -799,8 +795,6 @@ static __latent_entropy int dup_mmap(struct mm_struct *mm,
799795
dup_userfaultfd_complete(&uf);
800796
else
801797
dup_userfaultfd_fail(&uf);
802-
fail_uprobe_end:
803-
uprobe_end_dup_mmap();
804798
return retval;
805799

806800
fail_nomem_anon_vma_fork:
@@ -1713,9 +1707,11 @@ static struct mm_struct *dup_mm(struct task_struct *tsk,
17131707
if (!mm_init(mm, tsk, mm->user_ns))
17141708
goto fail_nomem;
17151709

1710+
uprobe_start_dup_mmap();
17161711
err = dup_mmap(mm, oldmm);
17171712
if (err)
17181713
goto free_pt;
1714+
uprobe_end_dup_mmap();
17191715

17201716
mm->hiwater_rss = get_mm_rss(mm);
17211717
mm->hiwater_vm = mm->total_vm;
@@ -1730,6 +1726,8 @@ static struct mm_struct *dup_mm(struct task_struct *tsk,
17301726
mm->binfmt = NULL;
17311727
mm_init_owner(mm, NULL);
17321728
mmput(mm);
1729+
if (err)
1730+
uprobe_end_dup_mmap();
17331731

17341732
fail_nomem:
17351733
return NULL;

0 commit comments

Comments
 (0)