Re: [PATCH v2 01/16] liveupdate: luo_file: Add internal APIs for file preservation

[Pranjal Shrivastava <praan@google.com>]

On Mon, Apr 27, 2026 at 05:56:18PM +0000, Samiullah Khawaja wrote:
> From: Pasha Tatashin <pasha.tatashin@soleen.com>
> 
> The core liveupdate mechanism allows userspace to preserve file
> descriptors. However, kernel subsystems often manage struct file
> objects directly and need to participate in the preservation process
> programmatically without relying solely on userspace interaction.
> 
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>

[..]

> @@ -924,3 +931,65 @@ void liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh)
>  	luo_flb_unregister_all(fh);
>  	list_del(&ACCESS_PRIVATE(fh, list));
>  }
> +EXPORT_SYMBOL_GPL(liveupdate_unregister_file_handler);
> +
> +/**
> + * liveupdate_get_token_outgoing - Get the token for a preserved file.
> + * @s:      The outgoing liveupdate session.
> + * @file:   The file object to search for.
> + * @tokenp: Output parameter for the found token.
> + *
> + * Searches the list of preserved files in an outgoing session for a matching
> + * file object. If found, the corresponding user-provided token is returned.
> + *
> + * This function is intended for in-kernel callers that need to correlate a
> + * file with its liveupdate token.
> + *
> + * Context: It must be called with session mutex acquired.
> + * Return: 0 on success, -ENOENT if the file is not preserved in this session.
> + */
> +int liveupdate_get_token_outgoing(struct liveupdate_session *s,
> +				  struct file *file, u64 *tokenp)
> +{
> +	struct luo_file_set *file_set = luo_file_set_from_session_locked(s);
> +	struct luo_file *luo_file;
> +	int err = -ENOENT;
> +
> +	list_for_each_entry(luo_file, &file_set->files_list, list) {
> +		if (luo_file->file == file) {
> +			if (tokenp)
> +				*tokenp = luo_file->token;
> +			err = 0;
> +			break;
> +		}
> +	}
> +
> +	return err;
> +}
> +
> +/**
> + * liveupdate_get_file_incoming - Retrieves a preserved file for in-kernel use.
> + * @s:      The incoming liveupdate session (restored from the previous kernel).
> + * @token:  The unique token identifying the file to retrieve.
> + * @filep:  On success, this will be populated with a pointer to the retrieved
> + *          'struct file'.
> + *
> + * Provides a kernel-internal API for other subsystems to retrieve their
> + * preserved files after a live update. This function is a simple wrapper
> + * around luo_retrieve_file(), allowing callers to find a file by its token.
> + *
> + * The caller receives a new reference to the file and must call fput() when it
> + * is no longer needed. The file's lifetime is managed by LUO and any userspace
> + * file descriptors. If the caller needs to hold a reference to the file beyond
> + * the immediate scope, it must call get_file() itself.

Thanks for re-wording this and I'm sorry for being a stickler here, I'm
a bit concerned that the last part here might lead to reference leaks in
downstream drivers.

Looking at the underlying luo_retrieve_file() implementation [1], it 
explicitly calls get_file() before returning the pointer (both on the
initial retrieve and on cached ones). This means the caller inherently
receives a reference that they own & the caller is responsible for 
exactly one fput().

However, that last part of the comment can be misunderstood as the caller
doesn't hold a lasting reference unless they call get_file() themselves. 
This makes the reader assume that LUO is going to automatically reap
that initial reference from them.

If a driver author assumes LUO is going to reap it, they will follow that
last sentence and call get_file() to stash the pointer safely. They might
end up holding two references (thinking one of them will be reaped), and
could ultimately leak the struct file when they only call fput() once 
during teardown.

Should we just drop that last sentence to make the lifecycle contract 
unambiguous? (i.e., The caller gets a newly bumped reference, and they
are responsible for exactly one fput() per call).

> + *
> + * Context: It must be called with session mutex acquired of a restored session.
> + * Return: 0 on success. Returns -ENOENT if no file with the matching token is
> + *         found, or any other negative errno on failure.
> + */
> +int liveupdate_get_file_incoming(struct liveupdate_session *s, u64 token,
> +				 struct file **filep)
> +{
> +	return luo_retrieve_file(luo_file_set_from_session_locked(s),
> +				 token, filep);
> +}

Nit: Shouldn't we export both of these functions via EXPORT_SYMBOL_GPL?
Since, these new APIs are intended for kernel subsystems to participate
programmatically, there could be IOMMU drivers (or others) that can be
compiled as loadable modules. Thus we should export these APIs via 
EXPORT_SYMBOL_GPL(). If they aren't exported, any loadable module 
attempting to use them will compile successfully (due to the header), but
will fail to load at runtime with an Unknown symbol error.

IIUC, if a function isn't exported with EXPORT_SYMBOL, it remains hidden
inside vmlinux, (i.e. it isn't in the kernel's global symbol table used
during modprobe).

Thanks,
Praan

[1] https://elixir.bootlin.com/linux/v7.0-rc3/source/kernel/liveupdate/luo_file.c#L560