linux-kernel - Re: [PATCH] binfmt_elf: Take the mmap lock when walking the VMA list

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]

Date:   Mon, 31 Jan 2022 16:13:55 +0000
From:   Matthew Wilcox <willy@...radead.org>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Denys Vlasenko <vda.linux@...glemail.com>,
        Kees Cook <keescook@...omium.org>,
        Jann Horn <jannh@...gle.com>, Vlastimil Babka <vbabka@...e.cz>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>
Subject: Re: [PATCH] binfmt_elf: Take the mmap lock when walking the VMA list

On Mon, Jan 31, 2022 at 10:03:31AM -0600, Eric W. Biederman wrote:
> "Matthew Wilcox (Oracle)" <willy@...radead.org> writes:
> 
> > I'm not sure if the VMA list can change under us, but dump_vma_snapshot()
> > is very careful to take the mmap_lock in write mode.  We only need to
> > take it in read mode here as we do not care if the size of the stack
> > VMA changes underneath us.
> >
> > If it can be changed underneath us, this is a potential use-after-free
> > for a multithreaded process which is dumping core.
> 
> The problem is not multi-threaded process so much as processes that
> share their mm.

I don't understand the difference.  I appreciate that another process can
get read access to an mm through, eg, /proc, but how can another process
(that isn't a thread of this process) modify the VMAs?

> I think rather than take a lock we should be using the snapshot captured
> with dump_vma_snapshot.  Otherwise we have the very real chance that the
> two get out of sync.  Which would result in a non-sense core file.
> 
> Probably that means that dump_vma_snapshot needs to call get_file on
> vma->vm_file store it in core_vma_metadata.
> 
> Do you think you can fix it something like that?

Uhh .. that seems like it needs a lot more understanding of binfmt_elf
than I currently possess.  I'd rather spend my time working on folios
than learning much more about binfmt_elf.  I was just trying to fix an
assertion failure with the maple tree patches (we now assert that you're
holding a lock when walking the list of VMAs).