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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Nested acquisition of `luo_file->mutex` causes a lockdep splat when subsystems retrieve files programmatically.
- [Low] Missing EXPORT_SYMBOL_GPL for `liveupdate_get_token_outgoing` and `liveupdate_get_file_incoming` causes module build failures.
--

commit 3b62824c5e4a0e08095576efe5b452875a92a685
Author: Pasha Tatashin <pasha.tatashin@soleen.com>

liveupdate: luo_file: Add internal APIs for file preservation

This commit introduces internal APIs to allow kernel subsystems to
programmatically preserve and retrieve file descriptors during a live update.
This enables subsystems managing struct file objects directly to participate
without solely relying on userspace interaction.

> diff --git a/kernel/liveupdate/luo_file.c b/kernel/liveupdate/luo_file.c
> index a0a419085e283..0aa0b4e5339f3 100644
> --- a/kernel/liveupdate/luo_file.c
> +++ b/kernel/liveupdate/luo_file.c
[ ... ]
> @@ -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.
[ ... ]
> + */
> +int liveupdate_get_token_outgoing(struct liveupdate_session *s,
> +				  struct file *file, u64 *tokenp)
> +{

[Severity: Low]
Should this function be exported?

Since this API is intended for kernel subsystems that might be built as
loadable modules, will leaving out EXPORT_SYMBOL_GPL() cause module link
errors?

> +	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.
[ ... ]
> + */
> +int liveupdate_get_file_incoming(struct liveupdate_session *s, u64 token,
> +				 struct file **filep)
> +{

[Severity: Low]
Similar to liveupdate_get_token_outgoing(), should this function also be
exported with EXPORT_SYMBOL_GPL() to allow loadable modules to use it?

> +	return luo_retrieve_file(luo_file_set_from_session_locked(s),
> +				 token, filep);

[Severity: High]
Will this cause a lockdep splat if a subsystem uses this API during retrieval?

When a subsystem's .retrieve() callback is invoked from luo_retrieve_file(),
it already holds the target file's luo_file->mutex:

luo_retrieve_file() {
    ...
    guard(mutex)(&luo_file->mutex);
    ...
    err = luo_file->fh->ops->retrieve(&args);
}

If the subsystem callback then calls liveupdate_get_file_incoming(), it
recursively calls luo_retrieve_file() for the dependency file, attempting
to acquire a second luo_file->mutex.

Since all luo_file->mutex instances share the same lock class, won't this
trigger a 'possible recursive locking detected' warning?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1780676742.git.tarunsahu@google.com?part=1