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

[Samiullah Khawaja <skhawaja@google.com>]

On Mon, May 18, 2026 at 11:40:58AM +0000, Pranjal Shrivastava wrote:
>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);
>> +

[snip]
>> +
>> +/**
>> + * 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).

I think these are all valid points. I will remove the last sentences for
clarity.
>
>> + *
>> + * 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).

Agreed. Will Update this.

I think there are other LUO APIs that are not exported and can be used
by modules. Let me check those also.
>
>Thanks,
>Praan
>
>[1] https://elixir.bootlin.com/linux/v7.0-rc3/source/kernel/liveupdate/luo_file.c#L560
>