Re: [PATCH v10 08/21] jobs: add job lock in find_* functions

[Kevin Wolf]

Am 25.07.2022 um 09:38 hat Emanuele Giuseppe Esposito geschrieben:
> 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*.
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

> diff --git a/job-qmp.c b/job-qmp.c
> index 829a28aa70..ac11a6c23c 100644
> --- a/job-qmp.c
> +++ b/job-qmp.c
> @@ -29,14 +29,19 @@
>  #include "qapi/error.h"
>  #include "trace/trace-root.h"
>  
> -/* Get a job using its ID and acquire its AioContext */
> -static Job *find_job(const char *id, AioContext **aio_context, Error **errp)
> +/*
> + * Get a block job using its ID and acquire its AioContext.

This should still be just a job, not specifically a block job.

> + * Called with job_mutex held.
> + */
> +static Job *find_job_locked(const char *id,
> +                            AioContext **aio_context,
> +                            Error **errp)
>  {
>      Job *job;
>  
>      *aio_context = NULL;
>  
> -    job = job_get(id);
> +    job = job_get_locked(id);
>      if (!job) {
>          error_setg(errp, "Job not found");
>          return NULL;

With this fixed:

Reviewed-by: Kevin Wolf <kwolf@redhat.com>