Re: [PATCH v1 0/7] liveupdate: tmpfs preservation and early freeze

[Pasha Tatashin <pasha.tatashin@soleen.com>]

> > >  - memfd_pin_folios() doesn't require the inode lock, so iommufd can
> > >    pin folios of the target while they're being added or truncated
> > >    away. v1 starts with two patches that make iommufd take the lock
> > >    and make memfd_pin_folios() assert it.
> > 
> > Hm, isn't this mostly orthogonal to the change we're making here?
> 
> Kind of. Today .retrieve() fills a brand-new memfd that nobody else can
> reach until the ioctl returns. Retrieve-into fills a file that other
> processes can already open by path and pass to iommufd, so pinning can
> now race with the folios being added, or truncated away on failure.
> 
> In the failure case a folio can stay pinned (and mapped for DMA) after
> it's been removed from the file. Retrieve-into introduces the race, and
> the fix is two small prep patches.



> > We originally agreed that if retrieve() fails, we fail permanently and
> > leak resources until reboot to avoid memory corruption from ongoing DMA
> > activity. However, now that we have the retryable option, we can start
> > relaxing what is considered a permanent failure vs. a recoverable/early
> > failure that .finish() can clean up.
> 
> Ok I think we are ligned that we need to keep the preserved folios
> around until finish(). I think this point was saying there are bugs in
> the existing memfd_luo code and in this series that leak the folios even
> after finish(). Either way I'll fix them up.

I think we are solving this problem incorrectly, LUO should still always
re-create files, even the tmpfs files.

What if instead of retrieve-into an existing file FD, userspace passes
a dir_fd on the target tmpfs mount where the file should be restored?

During preserve we can record the file's basename if the file was linked
(or mark that it was unlinked). On retrieve, creates a brand-new file in
dir_fd (e.g. starts as an unlinked O_TMPFILE on that tmpfs superblock),
populates all the folios and metadata while the file is still unreachable
by anyone else, links it into dir_fd under the preserved name at the end
(or leaves it unlinked if the original file was unlinked), and returns
the FD of the new file to userspace.

That way:
1. Just like memfd today, .retrieve() always populates a brand-new
   file that no other process can open by path or pass to iommufd until
   retrieve succeeds, so the memfd_pin_folios() / truncate race goes
   away completely.
2. Userspace gets back a single new FD from RETRIEVE_FD instead of
   ending up with two FDs (the pre-created one and the retrieved one)
   referencing the same struct file.
3. Unlinked tmpfs files are also supported on the target tmpfs mount
   (preserving mount options and size/inode accounting) instead of
   silently falling back to the internal shm_mnt.

We can even postpone linking the file into dir_fd until finish(), so
that it only appears in the filesystem once it is fully populated.

Actually, I think, this is the right way to do tmpfs preservation,
instead of relying on user to provide the FD of an open file to us.

> > I do not want LUO to change the type of a preserved file across live
> > update. Take seals, for example: a memfd can be sealed with the
> > guarantee that it will never be modified, whereas a regular tmpfs file
> > does not support sealing. Allowing a preserved memfd to be retrieved
> > into a tmpfs file would create a loophole to strip seals (or bypass
> > mount/superblock boundaries). I strongly prefer keeping the file type
> > strictly the same across preserve and retrieve.
> 
> I agree that seals must not be strippable. I am not proposing to allow
> that. Regular tmpfs files are created with F_SEAL_SEAL, and v1 refuses
> to retrieve into a target that has F_SEAL_SEAL if the preserved file has
> any seals the target doesn't have. So a sealed memfd can only be
> retrieved into a memfd that allows sealing, and gets its seals back
> there the same way F_ADD_SEALS would add them. Only unsealed memfds can
> be retrieved into regular tmpfs files.
> 
> On mount boundaries, retrieving into a tmpfs file charges the blocks
> to that mount, so its size limit and quota apply. That's stricter than
> the unlimited internal shm mount that memfds live on, not looser.
> 
> This is what I mean about enforcing the fundamental requirements, rather
> than enforcing something artifical like MEMFD_LUO_FLAG_TMPFS.
> 
> BTW, MEMFD_LUO_FLAG_TMPFS is set based on i_nlink, so it doesn't really
> track the file type. An O_TMPFILE or unlinked tmpfs file gets preserved
> without the flag and comes back as an anonymous memfd on the internal
> shm mount. That changes its type and drops its mount's size limit and
> quota, which is the boundary crossing you're worried about.
> 
> If you still want the type to stay the same, we can define it by
> superblock instead of i_nlink:
> 
>  - memfds (on the internal shm mount): plain retrieve only, as today.
>  - files on a tmpfs mount, linked or not: retrieve-into only, into any
>    empty tmpfs file, including O_TMPFILE files.

I would like file type to stay exactly the same, I really don't want to 
play the game where LUO can magically change file from one type to 
another even if from Linux internal implementation they share a lot of 
code, they are still a different file types from the user perspective.