linux-kernel - Re: [BUG?] X86 arch_tlbbatch_flush() seems to be lacking mm_tlb_flush

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: <CAHk-=wgu9x3ds+Mxpz6-BxuQgPt-YJ75J8q0q38w+QR-fA2kqA@mail.gmail.com>
Date:   Fri, 14 Oct 2022 12:08:44 -0700
From:   Linus Torvalds <torvalds@...uxfoundation.org>
To:     Jann Horn <jannh@...gle.com>
Cc:     Andy Lutomirski <luto@...nel.org>, Linux-MM <linux-mm@...ck.org>,
        Mel Gorman <mgorman@...e.de>, Rik van Riel <riel@...hat.com>,
        kernel list <linux-kernel@...r.kernel.org>,
        Kees Cook <keescook@...omium.org>,
        Ingo Molnar <mingo@...nel.org>,
        Sasha Levin <sasha.levin@...cle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Will Deacon <will@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [BUG?] X86 arch_tlbbatch_flush() seems to be lacking
 mm_tlb_flush_nested() integration

On Fri, Oct 14, 2022 at 11:20 AM Jann Horn <jannh@...gle.com> wrote:\
>
> The effect would be that after process B removes a mapping with
> munmap() and creates a new mapping in its place, it would still see
> data from the old mapping when trying to access the new mapping.
>
> Am I missing something that protects against this scenario?

While I don't think that scenario is something you can trigger in
practice without enormously bad luck, I don't see anything that would
protect against it.

Afaik, the whole vmscan thing never takes the vm lock, only the file
lock (to protect mapping->i_map) or the anonvma lock (to protect
anon_vma->rb_root).

And none of that ends up serializing with a new mmap() that doesn't
even install a new page in the page tables (and just gets an old TLB
entry). There are zero shared data structures outside of the mm
itself.

Now, munmap() *could* serialize with it, because at least munmap has
access to the data structures and their locks. But it doesn't do any
deferred flushes that I can see, so while it's serialized, it doesn't
help.

And it wouldn't help to do try_to_unmap_flush() from munmap either,
since the deferred flushing is per-thread, and the munmap is done from
a different thread.

So if you're missing something, then I am too.

All this flushing is very careful to flush before actually releasing
the page, which is our really traditional TLB flush bug. But yeah,
that's not the only race - we should flush before replacing the
mapping too.

Mel? I think the batched flushing goes back to you many many years
ago. I hope Jann and me are just being stupid and missing something
obvious.

                  Linus