Re: [PATCH v5 08/22] liveupdate: luo_file: implement file systems callbacks

Pratyush Yadav
2025-11-10T17:27:41Z

Hi Pasha,

Caught a small bug during some of my testing.

On Fri, Nov 07 2025, Pasha Tatashin wrote:

> This patch implements the core mechanism for managing preserved
> files throughout the live update lifecycle. It provides the logic to
> invoke the file handler callbacks (preserve, unpreserve, freeze,
> unfreeze, retrieve, and finish) at the appropriate stages.
>
> During the reboot phase, luo_file_freeze() serializes the final
> metadata for each file (handler compatible string, token, and data
> handle) into a memory region preserved by KHO. In the new kernel,
> luo_file_deserialize() reconstructs the in-memory file list from this
> data, preparing the session for retrieval.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
[...]
> +int luo_preserve_file(struct luo_session *session, u64 token, int fd)
> +{
> + struct liveupdate_file_op_args args = {0};
> + struct liveupdate_file_handler *fh;
> + struct luo_file *luo_file;
> + struct file *file;
> + int err = -ENOENT;
> +
> + lockdep_assert_held(&session->mutex);
> +
> + if (luo_token_is_used(session, token))
> + return -EEXIST;
> +
> + file = fget(fd);
> + if (!file)
> + return -EBADF;
> +
> + err = luo_session_alloc_files_mem(session);

err gets set to 0 here...

> + if (err)
> + goto exit_err;
> +
> + if (session->count == LUO_FILE_MAX) {
> + err = -ENOSPC;
> + goto exit_err;
> + }
> +
> + list_for_each_entry(fh, &luo_file_handler_list, list) {
> + if (fh->ops->can_preserve(fh, file)) {
> + err = 0;
> + break;
> + }
> + }

... say no file handler can preserve this file ...

> +
> + /* err is still -ENOENT if no handler was found */
> + if (err)

... err is not ENOENT, but 0. So this function does not error but, but
goes ahead with fh == luo_file_handler_list (since end of list). This
causes an out-of-bounds access. It eventually causes a kernel fault and
panic.

You should drop the ENOENT at initialization time and set it right
before list_for_each_entry().

> + goto exit_err;
> +
> + luo_file = kzalloc(sizeof(*luo_file), GFP_KERNEL);
> + if (!luo_file) {
> + err = -ENOMEM;
> + goto exit_err;
> + }
> +
> + luo_file->file = file;
> + luo_file->fh = fh;
> + luo_file->token = token;
> + luo_file->retrieved = false;
> + mutex_init(&luo_file->mutex);
> +
> + args.handler = fh;
> + args.session = (struct liveupdate_session *)session;
> + args.file = file;
> + err = fh->ops->preserve(&args);
> + if (err) {
> + mutex_destroy(&luo_file->mutex);
> + kfree(luo_file);
> + goto exit_err;
> + } else {
> + luo_file->serialized_data = args.serialized_data;
> + list_add_tail(&luo_file->list, &session->files_list);
> + session->count++;
> + }
> +
> + return 0;
> +
> +exit_err:
> + fput(file);
> + luo_session_free_files_mem(session);
> +
> + return err;
> +}
[...]

--
Regards,
Pratyush Yadav