From: Pasha Tatashin Add a retryable output flag to struct liveupdate_file_op_args, which a handler's .retrieve() can set on error to tell LUO that it hasn't consumed the preserved state. LUO then lets userspace retry the retrieve, and .finish() still releases the preserved state if the file is never retrieved. When luo_retrieve_file() invokes a handler's .retrieve() callback and it returns an error, LUO currently records the error in luo_file->retrieve_status and blocks any subsequent retrieve attempts. This is necessary when the handler has already partially consumed or freed the preserved KHO state. However, with user arguments passed to .retrieve(), a handler may fail early while validating caller-supplied parameters, such as a missing or invalid argument, before touching any preserved KHO state. In such cases, userspace should be able to fix the argument and retry LIVEUPDATE_SESSION_RETRIEVE_FD. Signed-off-by: Pasha Tatashin Signed-off-by: David Matlack --- include/linux/liveupdate.h | 11 ++++++++--- include/uapi/linux/liveupdate.h | 8 +++++--- kernel/liveupdate/luo_file.c | 20 ++++++++++++++------ 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/include/linux/liveupdate.h b/include/linux/liveupdate.h index 7543f5d648d3..e4ad0bf78a5c 100644 --- a/include/linux/liveupdate.h +++ b/include/linux/liveupdate.h @@ -58,9 +58,13 @@ struct liveupdate_arg_spec { * @session: The session this file belongs to. * @retrieve_status: The retrieve status for the 'can_finish / finish' * operation. A value of 0 means the retrieve has not been - * attempted, a positive value means the retrieve was - * successful, and a negative value means the retrieve failed, - * and the value is the error code of the call. + * attempted (or only failed with a retryable error), a + * positive value means the retrieve was successful, and a + * negative value means the retrieve permanently failed, and + * the value is the error code of the call. + * @retryable: For .retrieve(): [OUT] Set to true by the callback on + * error when the preserved state has not been consumed and + * retrieval may be retried. * @file: The file object. For retrieve: [OUT] The callback sets * this to the new file. For other ops: [IN] The caller sets * this to the file being operated on. @@ -83,6 +87,7 @@ struct liveupdate_file_op_args { struct liveupdate_file_handler *handler; struct liveupdate_session *session; int retrieve_status; + bool retryable; struct file *file; u64 serialized_data; void *private_data; diff --git a/include/uapi/linux/liveupdate.h b/include/uapi/linux/liveupdate.h index 87e77c3bfd82..7420e2c5434b 100644 --- a/include/uapi/linux/liveupdate.h +++ b/include/uapi/linux/liveupdate.h @@ -174,9 +174,11 @@ struct liveupdate_session_preserve_fd { * associated with the token and populates the @fd field with a new file * descriptor referencing the restored resource in the current (new) kernel. * This operation must be performed *before* signaling completion via - * %LIVEUPDATE_IOCTL_FINISH. If a retrieve of a token fails, subsequent - * attempts to retrieve the token fail with the same error code. Failed - * retrieves are not retried. + * %LIVEUPDATE_IOCTL_FINISH. If a retrieve of a token fails after consuming + * preserved state, subsequent attempts to retrieve the token fail with the + * same error code. A file handler may instead leave the token intact on errors + * that occur before it consumes the preserved state, e.g. an invalid @arg, so + * that the retrieve can be retried. See the documentation of the file handler. * * Retrieving a token that was already retrieved successfully returns a new file * descriptor for the same file. Passing a non-zero @arg in that case fails diff --git a/kernel/liveupdate/luo_file.c b/kernel/liveupdate/luo_file.c index 1437630d435e..0c9a3b6921d1 100644 --- a/kernel/liveupdate/luo_file.c +++ b/kernel/liveupdate/luo_file.c @@ -152,9 +152,10 @@ static DEFINE_XARRAY(luo_preserved_files); * @retrieve_status: Status code indicating whether a user/kernel in the new kernel has * successfully called retrieve() on this file. This prevents * multiple retrieval attempts. A value of 0 means a retrieve() - * has not been attempted, a positive value means the retrieve() - * was successful, and a negative value means the retrieve() - * failed, and the value is the error code of the call. + * has not been attempted (or only failed with a retryable + * error), a positive value means the retrieve() was successful, + * and a negative value means the retrieve() permanently failed, + * and the value is the error code of the call. * @mutex: A mutex that protects the fields of this specific instance * (e.g., @retrieved, @file), ensuring that operations like * retrieving or finishing a file are atomic. @@ -588,7 +589,9 @@ void luo_file_unfreeze(struct luo_file_set *file_set, * The operation is idempotent: if a file has already been successfully * retrieved, this function will simply return a pointer to the existing * 'struct file' and report success without re-executing the retrieve - * operation. This is handled by checking the 'retrieved' flag under a lock. + * operation. If a previous retrieve failed permanently, it returns that + * saved error code; if a previous retrieve failed with a retryable error + * before consuming preserved state, the retrieve operation is re-attempted. * * File retrieval can happen in any order; it is not bound by the order of * preservation. @@ -655,8 +658,13 @@ int luo_retrieve_file(struct luo_file_set *file_set, u64 token, err = luo_file->fh->ops->retrieve(&args); kfree(args.arg); if (err) { - /* Keep the error code for later use. */ - luo_file->retrieve_status = err; + /* + * Keep the error code for later use unless the handler marked + * the failure as retryable (e.g. invalid user argument before + * preserved state was consumed). + */ + if (!args.retryable) + luo_file->retrieve_status = err; return err; } -- 2.56.0.542.g3ef89b7020-goog