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

[Pasha Tatashin]

On Sat, Mar 21, 2026 at 1:58 PM Pasha Tatashin
<pasha.tatashin@xxxxxxxxxx> wrote:
>
> Currently, LUO does not prevent the same file from being managed twice
> across different active sessions.
>
> Add a new i_state flag I_LUO_MANAGED and update luo_preserve_file()
> to check and set this flag when a file is preserved, and clear it in
> luo_file_unpreserve_files() when it is released.
>
> Additionally, set this flag in luo_retrieve_file() after a file is
> successfully restored in the new kernel, and clear it in
> luo_file_finish() when the LUO session is finalized.
>
> This ensures that the same file (inode) cannot be managed by multiple
> sessions. If another session attempts to preserve an already managed
> file, it will now fail with -EBUSY.
>
> Acked-by: Pratyush Yadav (Google) <pratyush@xxxxxxxxxx>
> Acked-by: Jan Kara <jack@xxxxxxx>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@xxxxxxxxxx>
> ---
>  include/linux/fs.h           |  5 ++++-
>  kernel/liveupdate/luo_file.c | 27 ++++++++++++++++++++++++---
>  2 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 23f36a2613a3..692a8be56f3c 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -712,6 +712,8 @@ is_uncached_acl(struct posix_acl *acl)
>   * I_LRU_ISOLATING     Inode is pinned being isolated from LRU without holding
>   *                     i_count.
>   *
> + * I_LUO_MANAGED       Inode is being managed by a live update session.
> + *
>   * Q: What is the difference between I_WILL_FREE and I_FREEING?
>   *
>   * __I_{SYNC,NEW,LRU_ISOLATING} are used to derive unique addresses to wait
> @@ -744,7 +746,8 @@ enum inode_state_flags_enum {
>         I_CREATING              = (1U << 15),
>         I_DONTCACHE             = (1U << 16),
>         I_SYNC_QUEUED           = (1U << 17),
> -       I_PINNING_NETFS_WB      = (1U << 18)
> +       I_PINNING_NETFS_WB      = (1U << 18),
> +       I_LUO_MANAGED           = (1U << 19),
>  };
>
>  #define I_DIRTY_INODE (I_DIRTY_SYNC | I_DIRTY_DATASYNC)
> diff --git a/kernel/liveupdate/luo_file.c b/kernel/liveupdate/luo_file.c
> index 5acee4174bf0..86911beeff71 100644
> --- a/kernel/liveupdate/luo_file.c
> +++ b/kernel/liveupdate/luo_file.c
> @@ -248,6 +248,7 @@ static bool luo_token_is_used(struct luo_file_set *file_set, u64 token)
>   * Context: Can be called from an ioctl handler during normal system operation.
>   * Return: 0 on success. Returns a negative errno on failure:
>   *         -EEXIST if the token is already used.
> + *         -EBUSY if the file descriptor is already preserved by another session.
>   *         -EBADF if the file descriptor is invalid.
>   *         -ENOSPC if the file_set is full.
>   *         -ENOENT if no compatible handler is found.
> @@ -276,6 +277,14 @@ int luo_preserve_file(struct luo_file_set *file_set, u64 token, int fd)
>         if (err)
>                 goto  err_fput;
>
> +       scoped_guard(spinlock, &file_inode(file)->i_lock) {
> +               if (inode_state_read(file_inode(file)) & I_LUO_MANAGED) {
> +                       err = -EBUSY;
> +                       goto err_free_files_mem;
> +               }
> +               inode_state_set(file_inode(file), I_LUO_MANAGED);
> +       }
> +
>         err = -ENOENT;
>         list_private_for_each_entry(fh, &luo_file_handler_list, list) {
>                 if (fh->ops->can_preserve(fh, file)) {
> @@ -286,11 +295,11 @@ int luo_preserve_file(struct luo_file_set *file_set, u64 token, int fd)
>
>         /* err is still -ENOENT if no handler was found */
>         if (err)
> -               goto err_free_files_mem;
> +               goto err_unpreserve_inode;
>
>         err = luo_flb_file_preserve(fh);
>         if (err)
> -               goto err_free_files_mem;
> +               goto err_unpreserve_inode;
>
>         luo_file = kzalloc_obj(*luo_file);
>         if (!luo_file) {
> @@ -320,6 +329,9 @@ int luo_preserve_file(struct luo_file_set *file_set, u64 token, int fd)
>         kfree(luo_file);
>  err_flb_unpreserve:
>         luo_flb_file_unpreserve(fh);
> +err_unpreserve_inode:
> +       scoped_guard(spinlock, &file_inode(file)->i_lock)
> +               inode_state_clear(file_inode(file), I_LUO_MANAGED);
>  err_free_files_mem:
>         luo_free_files_mem(file_set);
>  err_fput:
> @@ -363,6 +375,9 @@ void luo_file_unpreserve_files(struct luo_file_set *file_set)
>                 luo_file->fh->ops->unpreserve(&args);
>                 luo_flb_file_unpreserve(luo_file->fh);
>
> +               scoped_guard(spinlock, &file_inode(luo_file->file)->i_lock)
> +                       inode_state_clear(file_inode(luo_file->file), I_LUO_MANAGED);
> +
>                 list_del(&luo_file->list);
>                 file_set->count--;
>
> @@ -609,6 +624,9 @@ int luo_retrieve_file(struct luo_file_set *file_set, u64 token,
>         *filep = luo_file->file;
>         luo_file->retrieve_status = 1;
>
> +       scoped_guard(spinlock, &file_inode(luo_file->file)->i_lock)
> +               inode_state_set(file_inode(luo_file->file), I_LUO_MANAGED);
> +
>         return 0;
>  }
>
> @@ -701,8 +719,11 @@ int luo_file_finish(struct luo_file_set *file_set)
>
>                 luo_file_finish_one(file_set, luo_file);
>
> -               if (luo_file->file)
> +               if (luo_file->file) {
> +                       scoped_guard(spinlock, &file_inode(luo_file->file)->i_lock)
> +                               inode_state_clear(file_inode(luo_file->file), I_LUO_MANAGED);
>                         fput(luo_file->file);
> +               }
>                 list_del(&luo_file->list);
>                 file_set->count--;
>                 mutex_destroy(&luo_file->mutex);
> --
> 2.43.0
>

> Sashiko: https://sashiko.dev/#/patchset/20260321175808.57942-1-pasha.tatashin@xxxxxxxxxx

Sashiko reported two problems:

1. Are there any issues with mixing goto-based error handling and scope-based
cleanups like scoped_guard() in the same function?

Initially, I thought that there should not be any problems, however,
after looking this up  I found in include/linux/cleanup.h the
following comment:

 * Lastly, given that the benefit of cleanup helpers is removal of
 * "goto", and that the "goto" statement can jump between scopes, the
 * expectation is that usage of "goto" and cleanup helpers is never
 * mixed in the same function.

Well, good to know, will not use goto inside scoped_guards.

2. Additionally, does setting I_LUO_MANAGED on the inode break the preservation
of anonymous inodes? Many file types (like eventfd, epoll, timerfd,
signalfd)

This is actually a very good point. It looks like everyone who uses
anon_inode_getfd() has one shared inode. This is not a problem for the
existing LUO user memfd, or for the upcoming vfiofd and memfd, but
kvm-vmfd and kvm-cpufd also use it, and that might be a problem in the
future once we add support for Orphaned VMs.

Therefore, we have two choices: either use a hash table, which adds
performance and memory overhead, or delegate this double-check to the
LUO file handlers, as they can use a private context to know if the FD
is already preserved.

Pasha