Both blockdev.c and job-qmp.c have TOC/TOU conditions, because they first search for the job and then perform an action on it. Therefore, we need to do the search + action under the same job mutex critical section.
Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito <eespo...@redhat.com> --- blockdev.c | 14 +++++++++++++- job-qmp.c | 13 ++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/blockdev.c b/blockdev.c index c5fba4d157..08408cd44b 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3311,7 +3311,10 @@ out: aio_context_release(aio_context); } -/* Get a block job using its ID and acquire its AioContext */ +/* + * Get a block job using its ID and acquire its AioContext. + * Returns with job_lock held on success. + */ static BlockJob *find_block_job(const char *id, AioContext **aio_context, Error **errp) { @@ -3320,12 +3323,14 @@ static BlockJob *find_block_job(const char *id, AioContext **aio_context, assert(id != NULL); *aio_context = NULL; + job_lock(); job = block_job_get(id); if (!job) { error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE, "Block job '%s' not found", id); + job_unlock(); return NULL; } @@ -3346,6 +3351,7 @@ void qmp_block_job_set_speed(const char *device, int64_t speed, Error **errp) block_job_set_speed(job, speed, errp); aio_context_release(aio_context); + job_unlock(); } void qmp_block_job_cancel(const char *device, @@ -3372,6 +3378,7 @@ void qmp_block_job_cancel(const char *device, job_user_cancel(&job->job, force, errp); out: aio_context_release(aio_context); + job_unlock(); } void qmp_block_job_pause(const char *device, Error **errp) @@ -3386,6 +3393,7 @@ void qmp_block_job_pause(const char *device, Error **errp) trace_qmp_block_job_pause(job); job_user_pause(&job->job, errp); aio_context_release(aio_context); + job_unlock(); } void qmp_block_job_resume(const char *device, Error **errp) @@ -3400,6 +3408,7 @@ void qmp_block_job_resume(const char *device, Error **errp) trace_qmp_block_job_resume(job); job_user_resume(&job->job, errp); aio_context_release(aio_context); + job_unlock(); } void qmp_block_job_complete(const char *device, Error **errp) @@ -3414,6 +3423,7 @@ void qmp_block_job_complete(const char *device, Error **errp) trace_qmp_block_job_complete(job); job_complete(&job->job, errp); aio_context_release(aio_context); + job_unlock(); } void qmp_block_job_finalize(const char *id, Error **errp) @@ -3437,6 +3447,7 @@ void qmp_block_job_finalize(const char *id, Error **errp) aio_context = block_job_get_aio_context(job); job_unref(&job->job); aio_context_release(aio_context); + job_unlock(); } void qmp_block_job_dismiss(const char *id, Error **errp) @@ -3453,6 +3464,7 @@ void qmp_block_job_dismiss(const char *id, Error **errp) job = &bjob->job; job_dismiss(&job, errp); aio_context_release(aio_context); + job_unlock(); } void qmp_change_backing_file(const char *device, diff --git a/job-qmp.c b/job-qmp.c index 06f970f6cf..a08cd7dd36 100644 --- a/job-qmp.c +++ b/job-qmp.c @@ -29,16 +29,21 @@ #include "qapi/error.h" #include "trace/trace-root.h" -/* Get a job using its ID and acquire its AioContext */ +/* + * Get a block job using its ID and acquire its AioContext. + * Returns with job_lock held on success. + */ static Job *find_job(const char *id, AioContext **aio_context, Error **errp) { Job *job; *aio_context = NULL; + job_lock(); job = job_get(id); if (!job) { error_setg(errp, "Job not found"); + job_unlock(); return NULL; } @@ -60,6 +65,7 @@ void qmp_job_cancel(const char *id, Error **errp) trace_qmp_job_cancel(job); job_user_cancel(job, true, errp); aio_context_release(aio_context); + job_unlock(); } void qmp_job_pause(const char *id, Error **errp) @@ -74,6 +80,7 @@ void qmp_job_pause(const char *id, Error **errp) trace_qmp_job_pause(job); job_user_pause(job, errp); aio_context_release(aio_context); + job_unlock(); } void qmp_job_resume(const char *id, Error **errp) @@ -88,6 +95,7 @@ void qmp_job_resume(const char *id, Error **errp) trace_qmp_job_resume(job); job_user_resume(job, errp); aio_context_release(aio_context); + job_unlock(); } void qmp_job_complete(const char *id, Error **errp) @@ -102,6 +110,7 @@ void qmp_job_complete(const char *id, Error **errp) trace_qmp_job_complete(job); job_complete(job, errp); aio_context_release(aio_context); + job_unlock(); } void qmp_job_finalize(const char *id, Error **errp) @@ -125,6 +134,7 @@ void qmp_job_finalize(const char *id, Error **errp) aio_context = job->aio_context; job_unref(job); aio_context_release(aio_context); + job_unlock(); } void qmp_job_dismiss(const char *id, Error **errp) @@ -139,6 +149,7 @@ void qmp_job_dismiss(const char *id, Error **errp) trace_qmp_job_dismiss(job); job_dismiss(&job, errp); aio_context_release(aio_context); + job_unlock(); } static JobInfo *job_query_single(Job *job, Error **errp) -- 2.31.1