linux-kernel - Re: [PATCH] mm: madvise: MADV_DONTNEED

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8080cb2f-09e3-30d2-0f60-160bfe5f9803@oracle.com>
Date:   Fri, 4 Mar 2022 10:45:44 -0800
From:   Mike Kravetz <mike.kravetz@...cle.com>
To:     Johannes Weiner <hannes@...xchg.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Cc:     Michal Hocko <mhocko@...e.com>, Vlastimil Babka <vbabka@...e.cz>,
        Nadav Amit <nadav.amit@...il.com>,
        David Hildenbrand <david@...hat.com>, dgilbert@...hat.com,
        linux-mm@...ck.org, linux-api@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: madvise: MADV_DONTNEED_LOCKED

On 3/4/22 09:19, Johannes Weiner wrote:
> MADV_DONTNEED historically rejects mlocked ranges, but with
> MLOCK_ONFAULT and MCL_ONFAULT allowing to mlock without populating,
> there are valid use cases for depopulating locked ranges as well.
> 
> Users mlock memory to protect secrets. There are allocators for secure
> buffers that want in-use memory generally mlocked, but cleared and
> invalidated memory to give up the physical pages. This could be done
> with explicit munlock -> mlock calls on free -> alloc of course, but
> that adds two unnecessary syscalls, heavy mmap_sem write locks, vma
> splits and re-merges - only to get rid of the backing pages.
> 
> Users also mlockall(MCL_ONFAULT) to suppress sustained paging, but are
> okay with on-demand initial population. It seems valid to selectively
> free some memory during the lifetime of such a process, without having
> to mess with its overall policy.
> 
> Why add a separate flag? Isn't this a pretty niche usecase?
> 
> - MADV_DONTNEED has been bailing on locked vmas forever. It's at least
>   conceivable that someone, somewhere is relying on mlock to protect
>   data from perhaps broader invalidation calls. Changing this behavior
>   now could lead to quiet data corruption.
> 
> - It also clarifies expectations around MADV_FREE and maybe
>   MADV_REMOVE. It avoids the situation where one quietly behaves
>   different than the others. MADV_FREE_LOCKED can be added later.
> 
> - The combination of mlock() and madvise() in the first place is
>   probably niche. But where it happens, I'd say that dropping pages
>   from a locked region once they don't contain secrets or won't page
>   anymore is much saner than relying on mlock to protect memory from
>   speculative or errant invalidation calls. It's just that we can't
>   change the default behavior because of the two previous points.
> 
> Given that, an explicit new flag seems to make the most sense.
> 
> Signed-off-by: Johannes Weiner <hannes@...xchg.org>
> Acked-by: Michal Hocko <mhocko@...e.com>
> ---
>  include/uapi/asm-generic/mman-common.h |  2 ++
>  mm/madvise.c                           | 24 ++++++++++++++----------
>  2 files changed, 16 insertions(+), 10 deletions(-)
> 
> v2:
> - mmap_sem for read is enough for DONTNEED_LOCKED, thanks Nadav
> - rebased on top of Mike's hugetlb DONTNEED patch in -mm

Thanks for rebasing on top of recent changes.

Reviewed-by: Mike Kravetz <mike.kravetz@...cle.com>

Looks like we both will be making madvise man page changes soon.
-- 
Mike Kravetz