linux-kernel - Re: [PATCH v2] fork: avoid inappropriate uprobe access to invalid mm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3bd43f87-571a-4bc6-b068-69056ec18d85@redhat.com>
Date: Tue, 10 Dec 2024 20:35:30 +0100
From: David Hildenbrand <david@...hat.com>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 Peter Zijlstra <peterz@...radead.org>
Cc: Oleg Nesterov <oleg@...hat.com>, Ingo Molnar <mingo@...hat.com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
 Adrian Hunter <adrian.hunter@...el.com>,
 Kan Liang <kan.liang@...ux.intel.com>, Masami Hiramatsu
 <mhiramat@...nel.org>, linux-perf-users@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
 Andrew Morton <akpm@...ux-foundation.org>,
 "Liam R . Howlett" <Liam.Howlett@...cle.com>,
 Vlastimil Babka <vbabka@...e.cz>, Jann Horn <jannh@...gle.com>,
 linux-mm@...ck.org, Peng Zhang <zhangpeng.00@...edance.com>,
 syzbot+2d788f4f7cb660dac4b7@...kaller.appspotmail.com
Subject: Re: [PATCH v2] fork: avoid inappropriate uprobe access to invalid mm

On 10.12.24 18:24, Lorenzo Stoakes wrote:
> 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
> d24062914837 ("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.

What I understand is: we need to perform the uprobe_end_dup_mmap() after 
the mmput().

I assume/hope that we cannot see another mmget() before we return here. 
In that case, this LGTM.

-- 
Cheers,

David / dhildenb