Re: [PATCH v3 1/2] liveupdate: prevent double management of files

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

On Thu, Mar 26, 2026 at 5:04 AM Mike Rapoport <rppt@kernel.org> wrote:
>
> On Wed, Mar 25, 2026 at 05:08:57PM -0400, Pasha Tatashin wrote:
> > On Wed, Mar 25, 2026 at 4:34 PM David Matlack <dmatlack@google.com> wrote:
> > >
> > > On Wed, Mar 25, 2026 at 1:20 PM Pratyush Yadav <pratyush@kernel.org> wrote:
> > >
> > > > For memfd and hugetlb at least, we serialize the _inode_ not the file.
> > > > The inode has the contents that we care to preserve.
> > > >
> > > > So if two FDs point to the same inode, this will break. You can do this
> > > > by first creating a memfd and then by opening "/proc/self/fd/<fd>". Then
> > > > you would be able to trigger the preservation twice, causing all sorts
> > > > of problems. Same on the retrieve side.
> >
> > Hm.
> >
> > >
> > > > So unless I am missing something, I don't think this approach will work.
> > > > As much as I hate to suggest it, I think we need to move this check to
> > > > each caller so they can find out the object they need to serialize and
> > > > check if it already is.
> > >
> > > I think LUO can still enforce that the file is not preserved twice.
> > > HugeTLB and memfd's preserve() functions just need to also check that
> > > the associated inode has not already been preserved?
> >
> > For memfd/hugetlbs the true state is in inode
> > For vfio/kvm the shared anonymous inode is just a dummy wrapper, and
> > the true state is in file->private_data.
> >
> > I wonder if we could use the XArray to track inodes for standard
> > files, but track the struct file itself for anonymous files (we would
> > need a new function from FS that allows us to determine if "struct
> > file" has anonymous inode or not).
>
> Don't all files we preserve use anon inodes?

No for memfd_create(), the inode allocation path depends on the
presence of the MFD_HUGETLB flag:

1. Regular memfd (shmem):
memfd_alloc_file() calls shmem_file_setup(), which leads to:
shmem_get_inode() -> __shmem_get_inode() -> new_inode().
The allocated inode is a struct shmem_inode_info.

2. HugeTLB memfd (hugetlbfs):
memfd_alloc_file() calls hugetlb_file_setup(), which leads to:
hugetlbfs_get_inode() -> new_inode().
The allocated inode is a struct hugetlbfs_inode_info.

> How about we extend the fh->ops with a method that will return "unique"
> object?

Yeap, this is exactly what I am proposing. Adding an optional
get_id(struct file *file) to fh->ops allows each handler to define
what constitutes a "unique" identity for its supported file types.

luo_file.c will have a helper like this:

static inline unsigned long luo_get_id(struct liveupdate_file_handler
*fh, struct file *file)
{
     return (fh->ops->get_id) ? fh->ops->get_id(file) : (unsigned long)file;
}

For memfd, the get_id implementation will return the inode pointer.

Pasha


>
>         list_private_for_each_entry(fh, &luo_file_handler_list, list) {
>                 if (fh->ops->can_preserve(fh, file)) {
>                         unique_handle = fh->ops->unique_handle(fh, file);
>                         err = 0;
>                         break;
>                 }
>         }
>
>         xa_insert(&luo_preserved_objects, unique_handle,
>                   (unsigned long)unique_handle, GFP_KERNEL);
>
> > Pasha
>
> --
> Sincerely yours,
> Mike.
>