Skip to content

Commit

Permalink
block: Fix use after free in blockdev_mark_auto_del()
Browse files Browse the repository at this point in the history
job_cancel_locked() drops the job list lock temporarily and it may call
aio_poll(). We must assume that the list has changed after this call.
Also, with unlucky timing, it can end up freeing the job during
job_completed_txn_abort_locked(), making the job pointer invalid, too.

For both reasons, we can't just continue at block_job_next_locked(job).
Instead, start at the head of the list again after job_cancel_locked()
and skip those jobs that we already cancelled (or that are completing
anyway).

Cc: [email protected]
Signed-off-by: Kevin Wolf <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Stefan Hajnoczi <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
  • Loading branch information
kevmw committed May 10, 2023
1 parent 3edf660 commit e262687
Showing 1 changed file with 14 additions and 4 deletions.
18 changes: 14 additions & 4 deletions blockdev.c
Original file line number Diff line number Diff line change
Expand Up @@ -153,12 +153,22 @@ void blockdev_mark_auto_del(BlockBackend *blk)

JOB_LOCK_GUARD();

for (job = block_job_next_locked(NULL); job;
job = block_job_next_locked(job)) {
if (block_job_has_bdrv(job, blk_bs(blk))) {
do {
job = block_job_next_locked(NULL);
while (job && (job->job.cancelled ||
job->job.deferred_to_main_loop ||
!block_job_has_bdrv(job, blk_bs(blk))))
{
job = block_job_next_locked(job);
}
if (job) {
/*
* This drops the job lock temporarily and polls, so we need to
* restart processing the list from the start after this.
*/
job_cancel_locked(&job->job, false);
}
}
} while (job);

dinfo->auto_del = 1;
}
Expand Down

0 comments on commit e262687

Please sign in to comment.