Re: [PATCH v17 07/10] mm: introduce memfd_secret system call to create "secret" memory areas

[David Hildenbrand <david@redhat.com>]

On 11.02.21 12:27, Mike Rapoport wrote:
> 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.
>   

So let's talk about the main user-visible differences to other memfd 
files (especially, other purely virtual files like hugetlbfs). With 
secretmem:

- File content can only be read/written via memory mappings.
- File content cannot be swapped out.

I think there are still valid ways to modify file content using 
syscalls: e.g., fallocate(PUNCH_HOLE). Things like truncate also seems 
to work just fine.

What else?


>> AFAIKS, we would need MFD_SECRET and disallow
>> MFD_ALLOW_SEALING and MFD_HUGETLB.
> 
> So here we start to multiplex.

Yes. And as Michal said, maybe we can support combinations in the future.

> 
>> 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.

No strong opinion. (I'd try to hurt the kernel less as default)

-- 
Thanks,

David / dhildenb