Message-ID: <20210211112702.GI242749@kernel.org>
Date:   Thu, 11 Feb 2021 13:27:02 +0200
From:   Mike Rapoport <rppt@...nel.org>
To:     David Hildenbrand <david@...hat.com>
Cc:     Michal Hocko <mhocko@...e.com>, Mike Rapoport <rppt@...ux.ibm.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Andy Lutomirski <luto@...nel.org>,
        Arnd Bergmann <arnd@...db.de>, Borislav Petkov <bp@...en8.de>,
        Catalin Marinas <catalin.marinas@....com>,
        Christopher Lameter <cl@...ux.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Elena Reshetova <elena.reshetova@...el.com>,
        "H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
        James Bottomley <jejb@...ux.ibm.com>,
        "Kirill A. Shutemov" <kirill@...temov.name>,
        Matthew Wilcox <willy@...radead.org>,
        Mark Rutland <mark.rutland@....com>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Rick Edgecombe <rick.p.edgecombe@...el.com>,
        Roman Gushchin <guro@...com>,
        Shakeel Butt <shakeelb@...gle.com>,
        Shuah Khan <shuah@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Tycho Andersen <tycho@...ho.ws>, Will Deacon <will@...nel.org>,
        linux-api@...r.kernel.org, linux-arch@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-nvdimm@...ts.01.org, linux-riscv@...ts.infradead.org,
        x86@...nel.org, Hagen Paul Pfeifer <hagen@...u.net>,
        Palmer Dabbelt <palmerdabbelt@...gle.com>
Subject: Re: [PATCH v17 07/10] mm: introduce memfd_secret system call to
 create "secret" memory areas

On Thu, Feb 11, 2021 at 10:01:32AM +0100, David Hildenbrand wrote:
> On 11.02.21 09:39, Michal Hocko wrote:
> > On Thu 11-02-21 09:13:19, Mike Rapoport wrote:
> > > On Tue, Feb 09, 2021 at 02:17:11PM +0100, Michal Hocko wrote:
> > > > On Tue 09-02-21 11:09:38, Mike Rapoport wrote:
> > [...]
> > > > > Citing my older email:
> > > > > 
> > > > >      I've hesitated whether to continue to use new flags to memfd_create() or to
> > > > >      add a new system call and I've decided to use a new system call after I've
> > > > >      started to look into man pages update. There would have been two completely
> > > > >      independent descriptions and I think it would have been very confusing.
> > > > 
> > > > Could you elaborate? Unmapping from the kernel address space can work
> > > > both for sealed or hugetlb memfds, no? Those features are completely
> > > > orthogonal AFAICS. With a dedicated syscall you will need to introduce
> > > > this functionality on top if that is required. Have you considered that?
> > > > I mean hugetlb pages are used to back guest memory very often. Is this
> > > > something that will be a secret memory usecase?
> > > > 
> > > > Please be really specific when giving arguments to back a new syscall
> > > > decision.
> > > 
> > > Isn't "syscalls have completely independent description" specific enough?
> > 
> > No, it's not as you can see from questions I've had above. More on that
> > below.
> > 
> > > We are talking about API here, not the implementation details whether
> > > secretmem supports large pages or not.
> > > 
> > > The purpose of memfd_create() is to create a file-like access to memory.
> > > The purpose of memfd_secret() is to create a way to access memory hidden
> > > from the kernel.
> > > 
> > > I don't think overloading memfd_create() with the secretmem flags because
> > > they happen to return a file descriptor will be better for users, but
> > > rather will be more confusing.
> > 
> > This is quite a subjective conclusion. I could very well argue that it
> > would be much better to have a single syscall to get a fd backed memory
> > with spedific requirements (sealing, unmapping from the kernel address
> > space). Neither of us would be clearly right or wrong. A more important
> > point is a future extensibility and usability, though. So let's just
> > think of few usecases I have outlined above. Is it unrealistic to expect
> > that secret memory should be sealable? What about hugetlb? Because if
> > the answer is no then a new API is a clear win as the combination of
> > flags would never work and then we would just suffer from the syscall
> > multiplexing without much gain. On the other hand if combination of the
> > functionality is to be expected then you will have to jam it into
> > memfd_create and copy the interface likely causing more confusion. See
> > what I mean?
> > 
> > I by no means do not insist one way or the other but from what I have
> > seen so far I have a feeling that the interface hasn't been thought
> > through enough. Sure you have landed with fd based approach and that
> > seems fair. But how to get that fd seems to still have some gaps IMHO.
> > 
> 
> I agree with Michal. This has been raised by different
> people already, including on LWN (https://lwn.net/Articles/835342/).
> 
> I can follow Mike's reasoning (man page), and I am also fine if there is
> a valid reason. However, IMHO the basic description seems to match quite good:
> 
>        memfd_create() creates an anonymous file and returns a file descriptor that refers to it.  The
>        file behaves like a regular file, and so can be modified, truncated, memory-mapped, and so on.
>        However,  unlike a regular file, it lives in RAM and has a volatile backing storage.  Once all
>        references to the file are dropped, it is automatically released.  Anonymous  memory  is  used
>        for  all  backing pages of the file.  Therefore, files created by memfd_create() have the same
>        semantics as other anonymous memory allocations such as those allocated using mmap(2) with the
>        MAP_ANONYMOUS flag.

Even despite my laziness and huge amount of copy-paste you can spot the
differences (this is a very old version, update is due):

       memfd_secret()  creates an anonymous file and returns a file descriptor
       that refers to it.  The file can only be memory-mapped; the  memory  in
       such  mapping  will  have  stronger protection than usual memory mapped
       files, and so it can be used to store application  secrets.   Unlike  a
       regular file, a file created with memfd_secret() lives in RAM and has a
       volatile backing storage.  Once all references to the file are dropped,
       it  is  automatically released.  The initial size of the file is set to
       0.  Following the call, the file size should be set using ftruncate(2).

       The memory areas obtained with mmap(2) from the file descriptor are ex‐
       clusive to the owning context.  These areas are removed from the kernel
       page tables and only the page table of the process holding the file de‐
       scriptor maps the corresponding physical memory.
 
> AFAIKS, we would need MFD_SECRET and disallow
> MFD_ALLOW_SEALING and MFD_HUGETLB.

So here we start to multiplex.

> In addition, we could add MFD_SECRET_NEVER_MAP, which could disallow any kind of
> temporary mappings (eor migration). TBC.

Never map is the default. When we'll need to map we'll add an explicit flag
for it.

-- 
Sincerely yours,
Mike.

Powered by blists - more mailing lists