linux-kernel - Re: [PATCH 10/10] mm,sched: conditionally skip lazy TLB mm refcounting

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: <FC4DF151-463C-4CED-8962-9056648BFEDC@amacapital.net>
Date:   Sun, 29 Jul 2018 08:29:27 -0700
From:   Andy Lutomirski <luto@...capital.net>
To:     Rik van Riel <riel@...riel.com>
Cc:     Andy Lutomirski <luto@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-team <kernel-team@...com>,
        Peter Zijlstra <peterz@...radead.org>, X86 ML <x86@...nel.org>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Ingo Molnar <mingo@...nel.org>, Mike Galbraith <efault@....de>,
        Dave Hansen <dave.hansen@...el.com>, will.daecon@....com,
        Catalin Marinas <catalin.marinas@....com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>
Subject: Re: [PATCH 10/10] mm,sched: conditionally skip lazy TLB mm refcounting



> On Jul 29, 2018, at 5:11 AM, Rik van Riel <riel@...riel.com> wrote:
> 
>> On Sat, 2018-07-28 at 21:21 -0700, Andy Lutomirski wrote:
>> On Sat, Jul 28, 2018 at 2:53 PM, Rik van Riel <riel@...riel.com>
>> wrote:
>>> Conditionally skip lazy TLB mm refcounting. When an architecture
>>> has
>>> CONFIG_ARCH_NO_ACTIVE_MM_REFCOUNTING enabled, an mm that is used in
>>> lazy TLB mode anywhere will get shot down from exit_mmap, and there
>>> in no need to incur the cache line bouncing overhead of refcounting
>>> a lazy TLB mm.
>> 
>> Unless I've misunderstood something, this patch results in idle tasks
>> whose active_mm has been freed still having active_mm pointing at
>> freed memory. 
> 
> Patch 9/10 is supposed to ensure that the lazy TLB CPUs get
> switched to init_mm before an mm is freed. No CPU should ever
> have its active_mm pointing at a freed mm.
> 
> Your message made me re-read the code, and now I realize that
> leave_mm does not actually do that.
> 
> Looking at the other callers of leave_mm, I might not be the
> only one surprised by that; xen_drop_mm_ref comes to mind.
> 
> I guess I should some code to leave_mm to have it actually
> clear active_mm and call the conditional refcount drop helper
> function.
> 
> Does that clear up the confusion?
> 

Kind of. But what’s the point of keeping active_mm?  On architectures that opt in to the new mode, there won’t be any code that cares about it’s value.  What’s the benefit of keeping it around?  If you ifdef it out, then it can’t possibly point to freed memory, and there’s nothing to worry about.