| Message ID | 20230203152202.49054-3-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: Lock the graph, part 2 (BlockDriver callbacks) | expand |
On 03.02.23 18:21, Kevin Wolf wrote: > bdrv_mirror_top_pwritev() accesses the job object when active mirroring > is enabled. It disables this code during early initialisation while > s->job isn't set yet. > > However, s->job is still set way too early when the job object isn't > fully initialised. For example, &s->ops_in_flight isn't initialised yet > and the in_flight bitmap doesn't exist yet. This causes crashes when a > write request comes in too early. > > Move the assignment of s->job to when the mirror job is actually fully > initialised to make sure that the mirror_top driver doesn't access it > too early. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> Probably bitmap initialization and (maybe) some other things in mirror_run() should actually be done in mirror_start_job(). Still: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
diff --git a/block/mirror.c b/block/mirror.c index ab326b67c9..fbbb4f619e 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -896,6 +896,7 @@ static int coroutine_fn mirror_run(Job *job, Error **errp) { MirrorBlockJob *s = container_of(job, MirrorBlockJob, common.job); BlockDriverState *bs = s->mirror_top_bs->backing->bs; + MirrorBDSOpaque *mirror_top_opaque = s->mirror_top_bs->opaque; BlockDriverState *target_bs = blk_bs(s->target); bool need_drain = true; BlockDeviceIoStatus iostatus; @@ -985,6 +986,12 @@ static int coroutine_fn mirror_run(Job *job, Error **errp) } } + /* + * Only now the job is fully initialised and mirror_top_bs should start + * accessing it. + */ + mirror_top_opaque->job = s; + assert(!s->dbi); s->dbi = bdrv_dirty_iter_new(s->dirty_bitmap); for (;;) { @@ -1704,7 +1711,6 @@ static BlockJob *mirror_start_job( if (!s) { goto fail; } - bs_opaque->job = s; /* The block job now has a reference to this node */ bdrv_unref(mirror_top_bs);
bdrv_mirror_top_pwritev() accesses the job object when active mirroring is enabled. It disables this code during early initialisation while s->job isn't set yet. However, s->job is still set way too early when the job object isn't fully initialised. For example, &s->ops_in_flight isn't initialised yet and the in_flight bitmap doesn't exist yet. This causes crashes when a write request comes in too early. Move the assignment of s->job to when the mirror job is actually fully initialised to make sure that the mirror_top driver doesn't access it too early. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/mirror.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)