linux-kernel - Re: [RFC 20/20] mm/rmap: avoid potential races

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, 23 Aug 2021 15:50:22 +0000
From:   Nadav Amit <namit@...are.com>
To:     "Huang, Ying" <ying.huang@...el.com>
CC:     Linux-MM <linux-mm@...ck.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Mel Gorman <mgorman@...hsingularity.net>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Will Deacon <will@...nel.org>, Yu Zhao <yuzhao@...gle.com>,
        "x86@...nel.org" <x86@...nel.org>
Subject: Re: [RFC 20/20] mm/rmap: avoid potential races



> On Aug 23, 2021, at 1:05 AM, Huang, Ying <ying.huang@...el.com> wrote:
> 
> Hi, Nadav,
> 
> Nadav Amit <nadav.amit@...il.com> writes:
> 
>> From: Nadav Amit <namit@...are.com>
>> 
>> flush_tlb_batched_pending() appears to have a theoretical race:
>> tlb_flush_batched is being cleared after the TLB flush, and if in
>> between another core calls set_tlb_ubc_flush_pending() and sets the
>> pending TLB flush indication, this indication might be lost. Holding the
>> page-table lock when SPLIT_LOCK is set cannot eliminate this race.
> 
> Recently, when I read the corresponding code, I find the exact same race
> too.  Do you still think the race is possible at least in theory?  If
> so, why hasn't your fix been merged?

I think the race is possible. It didn’t get merged, IIRC, due to some
addressable criticism and lack of enthusiasm from other people, and
my laziness/busy-ness.

> 
>> The current batched TLB invalidation scheme therefore does not seem
>> viable or easily repairable.
> 
> I have some idea to fix this without too much code.  If necessary, I
> will send it out.

Arguably, it would be preferable to have a small back-portable fix for
this issue specifically. Just try to ensure that you do not introduce
performance overheads. Any solution should be clear about its impact
on additional TLB flushes on the worst-case scenario and the number
of additional atomic operations that would be required.