Re: [RFC PATCH v2 2/4] liveupdate: Select file handler by name in the preserve ioctl

[tarunsahu@google.com]

I have gone through the discussion in reply to this thread between
@dmatlack and @pasha

I am not sure If I understood the discussion correctly.

userspace IOCTL for FLB compatiblity might not necessary as each FLBs are
registered with the file handler. So when new file handler introduced in the
system, new flb handlers can be registered that is compatible with this
new file handler. I have tried to explain this below.

Pasha Tatashin <pasha.tatashin@soleen.com> writes:

> On Wed, Oct 21, 2026 at 11:25 AM Tarun Sahu <tarunsahu@google.com> wrote:
>>
>> Currently luo_preserve_file() iterates over luo_file_handler_list and
>> picks the first registered handler whose .can_preserve() callback
>> returns true. When a subsystem introduces a new, backwards-incompatible
>> file handler alongside an existing one for the same file type, userspace
>> has no way to choose which handler preserves a file descriptor, nor can
>> the kernel keep the existing rollback-safe handler as the default while
>> allowing callers to opt into the new handler.
>
> That is not how it will work.
> If we are in the .can_preserve() call,
> it means the user has already decided which file handlers to use.

That is not true.

From the documentation of the luo_preserve_file(): 3rd point.

* This function orchestrates the first phase of preserving a file. It performs
 * the following steps:
 *
 * 1. Validates that the @token is not already in use within the file_set.
 * 2. Ensures the file_set's memory for files serialization is allocated
 *    (allocates if needed).
=>* 3. Iterates through registered handlers, calling can_preserve() to find one
 *    compatible with the given @fd.
 * 4. Calls the handler's .preserve() operation, which saves the file's state
 *    and returns an opaque private data handle.
 * 5. Adds the new instance to the file_set's internal list.
 *

luo_preserve_file():

	list_private_for_each_entry(fh, &luo_file_handler_list, list) {
		if (fh->ops->can_preserve(fh, file)) {
			if (try_module_get(fh->ops->owner))
				err = 0;
			break;
		}
	}


the first handler whose can_preserve return true will be selected as the
FH;

later in code:

      	err = fh->ops->preserve(&args);

So can_preserve() is called on every handler for a fd and once it finds
the FH which reutrns true, it uses its preserve call to preserve the file.

> We
> need to do that before any preservation happens; otherwise, we could
> run into conflicts on FLBs i.e. FH of different version require
> different FLBs.

My change below will select the user passed handler directly if it is
valid otherwise it fallbacks to looping .can_preserve based selection.

****For flb****:

Registration of flb handlers happens with original
subsytem fd handler, for example, for iommu

 iommu_liveupdate_register_flb(&iommufd_ser_handler); ->liveupdate_register_flb(fh, flb);

This function takes the FH argument. So every FH has direct relation
with their flbs.

If new FH will be introduced within the system, it must register
compatible flbs handlers with it by liveupdate_register_flb.

Currently, FLB objects are preserved with the first dependent file's
preservation. and each flb handler in fh->flb_list (was associated
using each flb handler registration) will call its ops to preserve itself.

Same on retrieval side, handler name is preserved so the flb name is
also preserved. And they can just call their respects ops to retrieve
themselves.

What about If user try to preserve other dependent but same subsystem
files with different handler. Kernel must reject this. I have not
integrated this support. Which can also be added as part of this series.
Let me know if this sounds good, Then I will add that in the V3?

~Tarun

>
> Therefore, there should be a luoctl call (i.e via a new ioctl to set
> compatability) that provides the kernel with the specs file of the
> target kernel we are updating to, allowing the kernel to select the
> compatible file handlers. Since .can_preserve() is called when the
> user is already preserving files, the specification must have already
> been supplied, or the kernel should fall back to using the default
> (i.e., latest) file handlers.
>
>>
>> Append char handler[LIVEUPDATE_HNDL_NAME_LENGTH] to struct
>> liveupdate_session_preserve_fd immediately after token, keeping
>> IOCTL_OP min_size at offsetofend(..., token) so callers passing the
>> original 16-byte structure continue to work:
>> - When handler is empty (all-zero, or zero-filled when userspace
>>   passes the 16-byte structure), luo_preserve_file() selects the first
>>   registered handler whose .can_preserve() callback accepts the fd.
>> - When handler is non-empty, it must be null-terminated within
>>   LIVEUPDATE_HNDL_NAME_LENGTH bytes (returning -EINVAL otherwise);
>>   luo_preserve_file() looks up the registered handler with that exact
>>   name (returning -ENOENT if unknown) and verifies that its
>>   .can_preserve() callback accepts the fd (returning -EINVAL if not).
>>
>> The retrieve ioctl is unchanged because the handler is identified by the
>> name stored in the serialized payload from the source kernel.
>>
>> Signed-off-by: Tarun Sahu <tarunsahu@google.com>
>> ---
>>  include/uapi/linux/liveupdate.h  | 12 +++++++++---
>>  kernel/liveupdate/luo_file.c     | 21 ++++++++++++++++++---
>>  kernel/liveupdate/luo_internal.h |  3 ++-
>>  kernel/liveupdate/luo_session.c  |  7 ++++++-
>>  4 files changed, 35 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/uapi/linux/liveupdate.h b/include/uapi/linux/liveupdate.h
>> index 509c6236720d..2413b19dabc1 100644
>> --- a/include/uapi/linux/liveupdate.h
>> +++ b/include/uapi/linux/liveupdate.h
>> @@ -126,9 +126,14 @@ struct liveupdate_ioctl_retrieve_session {
>>
>>  /**
>>   * struct liveupdate_session_preserve_fd - ioctl(LIVEUPDATE_SESSION_PRESERVE_FD)
>> - * @size:  Input; sizeof(struct liveupdate_session_preserve_fd)
>> - * @fd:    Input; The user-space file descriptor to be preserved.
>> - * @token: Input; An opaque, unique token for preserved resource.
>> + * @size:    Input; sizeof(struct liveupdate_session_preserve_fd)
>> + * @fd:      Input; The user-space file descriptor to be preserved.
>> + * @token:   Input; An opaque, unique token for preserved resource.
>> + * @handler: Input; Optional null-terminated file handler name (max length
>> + *           %LIVEUPDATE_HNDL_NAME_LENGTH including null terminator) selecting
>> + *           which registered handler preserves @fd. If empty (all-zero, or
>> + *           omitted when passing the 16-byte structure), the kernel selects
>> + *           the first registered handler whose .can_preserve() accepts @fd.
>>   *
>>   * Holds parameters for preserving a file descriptor.
>>   *
>> @@ -152,6 +157,7 @@ struct liveupdate_session_preserve_fd {
>>         __u32           size;
>>         __s32           fd;
>>         __aligned_u64   token;
>> +       char            handler[LIVEUPDATE_HNDL_NAME_LENGTH];
>>  };
>>
>>  #define LIVEUPDATE_SESSION_PRESERVE_FD                                 \
>> diff --git a/kernel/liveupdate/luo_file.c b/kernel/liveupdate/luo_file.c
>> index fb81239bedf3..be20e0f1e0aa 100644
>> --- a/kernel/liveupdate/luo_file.c
>> +++ b/kernel/liveupdate/luo_file.c
>> @@ -207,6 +207,8 @@ static bool luo_token_is_used(struct luo_file_set *file_set, u64 token)
>>   * @file_set: The file_set to which the preserved file will be added.
>>   * @token:    A unique, user-provided identifier for the file.
>>   * @fd:       The file descriptor to be preserved.
>> + * @handler:  Optional NUL-terminated handler name. If NULL or empty, the first
>> + *            registered handler whose .can_preserve() accepts @fd is selected.
>>   *
>>   * This function orchestrates the first phase of preserving a file. Upon entry,
>>   * it takes a reference to the 'struct file' via fget(), effectively making LUO
>> @@ -238,11 +240,13 @@ static bool luo_token_is_used(struct luo_file_set *file_set, u64 token)
>>   *         -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 for the file type.
>> + *         -ENOENT if no matching handler is registered or accepts @fd.
>> + *         -EINVAL if the named handler cannot preserve @fd.
>>   *         -ENOMEM on memory allocation failure.
>>   *         Other erros might be returned by .preserve().
>>   */
>> -int luo_preserve_file(struct luo_file_set *file_set, u64 token, int fd)
>> +int luo_preserve_file(struct luo_file_set *file_set, u64 token, int fd,
>> +                     const char *handler)
>>  {
>>         struct liveupdate_file_op_args args = {0};
>>         struct liveupdate_file_handler *fh;
>> @@ -266,6 +270,18 @@ int luo_preserve_file(struct luo_file_set *file_set, u64 token, int fd)
>>         err = -ENOENT;
>>         down_read(&luo_register_rwlock);
>>         list_private_for_each_entry(fh, &luo_file_handler_list, list) {
>> +               if (handler && handler[0]) {
>> +                       if (strcmp(fh->name, handler))
>> +                               continue;
>> +                       if (!fh->ops->can_preserve(fh, file)) {
>> +                               err = -EINVAL;
>> +                               break;
>> +                       }
>> +                       if (try_module_get(fh->ops->owner))
>> +                               err = 0;
>> +                       break;
>> +               }
>> +
>>                 if (fh->ops->can_preserve(fh, file)) {
>>                         if (try_module_get(fh->ops->owner))
>>                                 err = 0;
>> @@ -274,7 +290,6 @@ int luo_preserve_file(struct luo_file_set *file_set, u64 token, int fd)
>>         }
>>         up_read(&luo_register_rwlock);
>>
>> -       /* err is still -ENOENT if no handler was found */
>>         if (err)
>>                 goto err_fput;
>>
>> diff --git a/kernel/liveupdate/luo_internal.h b/kernel/liveupdate/luo_internal.h
>> index dac6644bfb18..911ffedec9f3 100644
>> --- a/kernel/liveupdate/luo_internal.h
>> +++ b/kernel/liveupdate/luo_internal.h
>> @@ -101,7 +101,8 @@ int __init luo_session_setup_incoming(u64 sessions_pa);
>>  int luo_session_serialize(void);
>>  int luo_session_deserialize(void);
>>
>> -int luo_preserve_file(struct luo_file_set *file_set, u64 token, int fd);
>> +int luo_preserve_file(struct luo_file_set *file_set, u64 token, int fd,
>> +                     const char *handler);
>>  void luo_file_unpreserve_files(struct luo_file_set *file_set);
>>  int luo_file_freeze(struct luo_file_set *file_set,
>>                     struct luo_file_set_ser *file_set_ser);
>> diff --git a/kernel/liveupdate/luo_session.c b/kernel/liveupdate/luo_session.c
>> index f48e9a4185f9..194ef45907c1 100644
>> --- a/kernel/liveupdate/luo_session.c
>> +++ b/kernel/liveupdate/luo_session.c
>> @@ -266,8 +266,13 @@ static int luo_session_preserve_fd(struct luo_session *session,
>>         struct liveupdate_session_preserve_fd *argp = ucmd->cmd;
>>         int err;
>>
>> +       if (strnlen(argp->handler, sizeof(argp->handler)) ==
>> +           sizeof(argp->handler))
>> +               return -EINVAL;
>> +
>>         guard(mutex)(&session->mutex);
>> -       err = luo_preserve_file(&session->file_set, argp->token, argp->fd);
>> +       err = luo_preserve_file(&session->file_set, argp->token, argp->fd,
>> +                               argp->handler);
>>         if (err)
>>                 return err;
>>
>> --
>> 2.56.0.212.g129ede20d9-goog
>>