Message-ID: <mafs0pla5cuml.fsf@kernel.org>
Date: Wed, 29 Oct 2025 21:37:06 +0100
From: Pratyush Yadav <pratyush@...nel.org>
To: Pasha Tatashin <pasha.tatashin@...een.com>
Cc: pratyush@...nel.org,  jasonmiu@...gle.com,  graf@...zon.com,
  changyuanl@...gle.com,  rppt@...nel.org,  dmatlack@...gle.com,
  rientjes@...gle.com,  corbet@....net,  rdunlap@...radead.org,
  ilpo.jarvinen@...ux.intel.com,  kanie@...ux.alibaba.com,
  ojeda@...nel.org,  aliceryhl@...gle.com,  masahiroy@...nel.org,
  akpm@...ux-foundation.org,  tj@...nel.org,  yoann.congal@...le.fr,
  mmaurer@...gle.com,  roman.gushchin@...ux.dev,  chenridong@...wei.com,
  axboe@...nel.dk,  mark.rutland@....com,  jannh@...gle.com,
  vincent.guittot@...aro.org,  hannes@...xchg.org,
  dan.j.williams@...el.com,  david@...hat.com,  joel.granados@...nel.org,
  rostedt@...dmis.org,  anna.schumaker@...cle.com,  song@...nel.org,
  zhangguopeng@...inos.cn,  linux@...ssschuh.net,
  linux-kernel@...r.kernel.org,  linux-doc@...r.kernel.org,
  linux-mm@...ck.org,  gregkh@...uxfoundation.org,  tglx@...utronix.de,
  mingo@...hat.com,  bp@...en8.de,  dave.hansen@...ux.intel.com,
  x86@...nel.org,  hpa@...or.com,  rafael@...nel.org,  dakr@...nel.org,
  bartosz.golaszewski@...aro.org,  cw00.choi@...sung.com,
  myungjoo.ham@...sung.com,  yesanishhere@...il.com,
  Jonathan.Cameron@...wei.com,  quic_zijuhu@...cinc.com,
  aleksander.lobakin@...el.com,  ira.weiny@...el.com,
  andriy.shevchenko@...ux.intel.com,  leon@...nel.org,  lukas@...ner.de,
  bhelgaas@...gle.com,  wagi@...nel.org,  djeffery@...hat.com,
  stuart.w.hayes@...il.com,  lennart@...ttering.net,  brauner@...nel.org,
  linux-api@...r.kernel.org,  linux-fsdevel@...r.kernel.org,
  saeedm@...dia.com,  ajayachandra@...dia.com,  jgg@...dia.com,
  parav@...dia.com,  leonro@...dia.com,  witu@...dia.com,
  hughd@...gle.com,  skhawaja@...gle.com,  chrisl@...nel.org,
  steven.sistare@...cle.com
Subject: Re: [PATCH v4 14/30] liveupdate: luo_session: Add ioctls for file
 preservation and state management

Hi Pasha,

On Mon, Sep 29 2025, Pasha Tatashin wrote:

> Introducing the userspace interface and internal logic required to
> manage the lifecycle of file descriptors within a session. Previously, a
> session was merely a container; this change makes it a functional
> management unit.
>
> The following capabilities are added:
>
> A new set of ioctl commands are added, which operate on the file
> descriptor returned by CREATE_SESSION. This allows userspace to:
> - LIVEUPDATE_SESSION_PRESERVE_FD: Add a file descriptor to a session
>   to be preserved across the live update.
> - LIVEUPDATE_SESSION_UNPRESERVE_FD: Remove a previously added file
>   descriptor from the session.
> - LIVEUPDATE_SESSION_RESTORE_FD: Retrieve a preserved file in the
>   new kernel using its unique token.
>
> A state machine for each individual session, distinct from the global
> LUO state. This enables more granular control, allowing userspace to
> prepare or freeze specific sessions independently. This is managed via:
> - LIVEUPDATE_SESSION_SET_EVENT: An ioctl to send PREPARE, FREEZE,
>   CANCEL, or FINISH events to a single session.
> - LIVEUPDATE_SESSION_GET_STATE: An ioctl to query the current state
>   of a single session.
>
> The global subsystem callbacks (luo_session_prepare, luo_session_freeze)
> are updated to iterate through all existing sessions. They now trigger
> the appropriate per-session state transitions for any sessions that
> haven't already been transitioned individually by userspace.
>
> The session's .release handler is enhanced to be state-aware. When a
> session's file descriptor is closed, it now correctly cancels or
> finishes the session based on its current state before freeing all
> associated file resources, preventing resource leaks.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@...een.com>
> ---
[...]
> +static int luo_session_restore_fd(struct luo_session *session,
> +				  struct luo_ucmd *ucmd)
> +{
> +	struct liveupdate_session_restore_fd *argp = ucmd->cmd;
> +	struct file *file;
> +	int ret;
> +
> +	guard(rwsem_read)(&luo_state_rwsem);
> +	if (!liveupdate_state_updated())
> +		return -EBUSY;
> +
> +	argp->fd = get_unused_fd_flags(O_CLOEXEC);
> +	if (argp->fd < 0)
> +		return argp->fd;
> +
> +	guard(mutex)(&session->mutex);
> +
> +	/* Session might have already finished independatly from global state */
> +	if (session->state != LIVEUPDATE_STATE_UPDATED)
> +		return -EBUSY;
> +
> +	ret = luo_retrieve_file(session, argp->token, &file);

The retrieve behaviour here causes some nastiness.

When the session is deserialized by luo_session_deserialize(), all the
files get added to the session's files_list. Now when a process
retrieves the session after kexec and restores a file, the file
handler's retrieve callback is invoked, deserializing and restoring the
file. Once deserialization is done, the callback usually frees up the
metadata. All this is fine.

The problem is that the file stays on on the files_list. When the
process closes the session FD, the unpreserve callback is invoked for
all files.

The unpreserve callback should undo what preserve did. That is, free up
serialization data. After a file is restored post-kexec, the things to
free up are different. For example, on a memfd, the folios won't be
pinned anymore. So invoking unpreserve on a retrieved file doesn't work
and causes UAF or other invalid behaviour.

I think you should treat retrieve as a unpreserve as well, and remove
the file from the session's list.

Side note: I see that a lot of code in luo_file.c works with the session
data structures directly. For example, luo_file_deserialize() adds the
file to session->files_list. I think the code would be a lot cleaner and
maintainable if the concerns were clearly separated.
luo_file_deserialize() should focus on deserializing a file given a
compatible and data, and all the dealing with the session's state should
be done by luo_session_deserialize().

luo_file_deserialize() is just an example, but I think the idea can be
applied in more places.

[...]

-- 
Regards,
Pratyush Yadav

Powered by blists - more mailing lists