linux-kernel - Re: flush_dcache_page vs kunmap

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-=wiDjjL50BBU=i8BFz3Rv5+-pGysEyCD+mcc_K_g0140oQ@mail.gmail.com>
Date:   Thu, 4 Nov 2021 10:08:40 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Catalin Marinas <catalin.marinas@....com>
Cc:     Matthew Wilcox <willy@...radead.org>,
        Christoph Hellwig <hch@....de>,
        linux-arch <linux-arch@...r.kernel.org>,
        Ira Weiny <ira.weiny@...el.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: flush_dcache_page vs kunmap_local

On Thu, Nov 4, 2021 at 9:54 AM Catalin Marinas <catalin.marinas@....com> wrote:
>
> We do. flush_dcache_page() is not just about virtual caches. On arm32/64
> (and powerpc), even with PIPT-like caches, we use it to flag a page's
> D-cache as no longer clean. Subsequently in set_pte_at(), if the mapping
> is executable, we do the cache maintenance to ensure the I and D caches
> are coherent with each other.

Ugh,. ok, so we have two very different use-cases for that function.

Perhaps more importantly, they have hugely different semantics. For
you, it's about pages that can be mapped executable, so it's only
relevant for mappable pages.

For the traditional broken pure virtual cache case, it's not about
user mappings at all, it's about any data structure that we might have
in highmem.

Of course, I think we got rid of most of the other uses of highmem,
and we no longer put any "normal" kernel data in highmem pages. There
used to be patches that did inodes and things like that in highmem,
and they actually depended on the "cache the virtual address so that
it's always the same" behavior.

> I wouldn't add this call to kmap/kunmap_local(), it would be a slight
> unnecessary overhead (we had a customer complaining about kmap_atomic()
> breaking write-streaming, I think the new kmap_local() solved this
> problem, if in the right context).

kmap_local() ends up being (I think) fundamentally broken for virtual
cache coherency anyway, because two different CPU's can see two
different virtual addresses at the same time for the same page (in
ways that the old kmap interfaces could not).

So maybe the answer is "let's forget about the old virtual cache
coherence issue, and make it purely about the I$ mapping case".

At that point, kmap is irrelevant from a virtual address standpoint
and so it doesn't make much sense to fliush on kunmap - but anybody
who writes to a page still needs that flush_dcache_page() thing.

                 Linus