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

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