| Message ID | 20180703023758.14422-2-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | backup: Use copy offloading | expand |
On Tue, Jul 03, 2018 at 10:37:56AM +0800, Fam Zheng wrote: > src may be NULL if BDRV_REQ_ZERO_WRITE flag is set, in this case only > check dst and dst->bs. This bug was introduced when moving in the > request tracking code from bdrv_co_copy_range, in 37aec7d75eb. > > This especially fixes the possible segfault when initializing src_bs > with a NULL src. > > Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> > --- > block/io.c | 29 +++++++++++++++-------------- > 1 file changed, 15 insertions(+), 14 deletions(-) > > diff --git a/block/io.c b/block/io.c > index 7035b78a20..b8845708d7 100644 > --- a/block/io.c > +++ b/block/io.c > @@ -2897,18 +2897,11 @@ static int coroutine_fn bdrv_co_copy_range_internal(BdrvChild *src, > bool recurse_src) > { > BdrvTrackedRequest src_req, dst_req; > - BlockDriverState *src_bs = src->bs; > - BlockDriverState *dst_bs = dst->bs; > int ret; > > - if (!src || !dst || !src->bs || !dst->bs) { > + if (!dst || !dst->bs) { > return -ENOMEDIUM; > } > - ret = bdrv_check_byte_request(src->bs, src_offset, bytes); > - if (ret) { > - return ret; > - } > - > ret = bdrv_check_byte_request(dst->bs, dst_offset, bytes); > if (ret) { > return ret; > @@ -2917,16 +2910,24 @@ static int coroutine_fn bdrv_co_copy_range_internal(BdrvChild *src, > return bdrv_co_pwrite_zeroes(dst, dst_offset, bytes, flags); > } > > + if (!src || !src->bs) { > + return -ENOMEDIUM; > + } > + ret = bdrv_check_byte_request(src->bs, src_offset, bytes); > + if (ret) { > + return ret; > + } > + > if (!src->bs->drv->bdrv_co_copy_range_from > || !dst->bs->drv->bdrv_co_copy_range_to > || src->bs->encrypted || dst->bs->encrypted) { > return -ENOTSUP; > } > - bdrv_inc_in_flight(src_bs); > - bdrv_inc_in_flight(dst_bs); > - tracked_request_begin(&src_req, src_bs, src_offset, > + bdrv_inc_in_flight(src->bs); > + bdrv_inc_in_flight(dst->bs); > + tracked_request_begin(&src_req, src->bs, src_offset, > bytes, BDRV_TRACKED_READ); > - tracked_request_begin(&dst_req, dst_bs, dst_offset, > + tracked_request_begin(&dst_req, dst->bs, dst_offset, > bytes, BDRV_TRACKED_WRITE); > > wait_serialising_requests(&src_req); > @@ -2944,8 +2945,8 @@ static int coroutine_fn bdrv_co_copy_range_internal(BdrvChild *src, > } > tracked_request_end(&src_req); > tracked_request_end(&dst_req); > - bdrv_dec_in_flight(src_bs); > - bdrv_dec_in_flight(dst_bs); > + bdrv_dec_in_flight(src->bs); > + bdrv_dec_in_flight(dst->bs); > return ret; > } > > -- > 2.17.1 >
diff --git a/block/io.c b/block/io.c index 7035b78a20..b8845708d7 100644 --- a/block/io.c +++ b/block/io.c @@ -2897,18 +2897,11 @@ static int coroutine_fn bdrv_co_copy_range_internal(BdrvChild *src, bool recurse_src) { BdrvTrackedRequest src_req, dst_req; - BlockDriverState *src_bs = src->bs; - BlockDriverState *dst_bs = dst->bs; int ret; - if (!src || !dst || !src->bs || !dst->bs) { + if (!dst || !dst->bs) { return -ENOMEDIUM; } - ret = bdrv_check_byte_request(src->bs, src_offset, bytes); - if (ret) { - return ret; - } - ret = bdrv_check_byte_request(dst->bs, dst_offset, bytes); if (ret) { return ret; @@ -2917,16 +2910,24 @@ static int coroutine_fn bdrv_co_copy_range_internal(BdrvChild *src, return bdrv_co_pwrite_zeroes(dst, dst_offset, bytes, flags); } + if (!src || !src->bs) { + return -ENOMEDIUM; + } + ret = bdrv_check_byte_request(src->bs, src_offset, bytes); + if (ret) { + return ret; + } + if (!src->bs->drv->bdrv_co_copy_range_from || !dst->bs->drv->bdrv_co_copy_range_to || src->bs->encrypted || dst->bs->encrypted) { return -ENOTSUP; } - bdrv_inc_in_flight(src_bs); - bdrv_inc_in_flight(dst_bs); - tracked_request_begin(&src_req, src_bs, src_offset, + bdrv_inc_in_flight(src->bs); + bdrv_inc_in_flight(dst->bs); + tracked_request_begin(&src_req, src->bs, src_offset, bytes, BDRV_TRACKED_READ); - tracked_request_begin(&dst_req, dst_bs, dst_offset, + tracked_request_begin(&dst_req, dst->bs, dst_offset, bytes, BDRV_TRACKED_WRITE); wait_serialising_requests(&src_req); @@ -2944,8 +2945,8 @@ static int coroutine_fn bdrv_co_copy_range_internal(BdrvChild *src, } tracked_request_end(&src_req); tracked_request_end(&dst_req); - bdrv_dec_in_flight(src_bs); - bdrv_dec_in_flight(dst_bs); + bdrv_dec_in_flight(src->bs); + bdrv_dec_in_flight(dst->bs); return ret; }
src may be NULL if BDRV_REQ_ZERO_WRITE flag is set, in this case only check dst and dst->bs. This bug was introduced when moving in the request tracking code from bdrv_co_copy_range, in 37aec7d75eb. This especially fixes the possible segfault when initializing src_bs with a NULL src. Signed-off-by: Fam Zheng <famz@redhat.com> --- block/io.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)