From 45db54d58de0b7171fe1f0d1dfc14378e464949e Mon Sep 17 00:00:00 2001 From: Kent Overstreet Date: Tue, 8 May 2018 21:33:54 -0400 Subject: [PATCH] block: Split out bio_list_copy_data() Found a bug (with ASAN) where we were passing a bio to bio_copy_data() with bi_next not NULL, when it should have been - a driver had left bi_next set to something after calling bio_endio(). Since the normal case is only copying single bios, split out bio_list_copy_data() to avoid more bugs like this in the future. Signed-off-by: Kent Overstreet Signed-off-by: Jens Axboe --- block/bio.c | 97 ++++++++++++++++++++++++----------------- drivers/block/pktcdvd.c | 2 +- include/linux/bio.h | 5 ++- 3 files changed, 62 insertions(+), 42 deletions(-) diff --git a/block/bio.c b/block/bio.c index e849efff4616..858f56f3edcb 100644 --- a/block/bio.c +++ b/block/bio.c @@ -971,13 +971,67 @@ void bio_advance(struct bio *bio, unsigned bytes) } EXPORT_SYMBOL(bio_advance); -void bio_copy_data_iter(struct bio *dst, struct bvec_iter dst_iter, - struct bio *src, struct bvec_iter src_iter) +void bio_copy_data_iter(struct bio *dst, struct bvec_iter *dst_iter, + struct bio *src, struct bvec_iter *src_iter) { struct bio_vec src_bv, dst_bv; void *src_p, *dst_p; unsigned bytes; + while (src_iter->bi_size && dst_iter->bi_size) { + src_bv = bio_iter_iovec(src, *src_iter); + dst_bv = bio_iter_iovec(dst, *dst_iter); + + bytes = min(src_bv.bv_len, dst_bv.bv_len); + + src_p = kmap_atomic(src_bv.bv_page); + dst_p = kmap_atomic(dst_bv.bv_page); + + memcpy(dst_p + dst_bv.bv_offset, + src_p + src_bv.bv_offset, + bytes); + + kunmap_atomic(dst_p); + kunmap_atomic(src_p); + + bio_advance_iter(src, src_iter, bytes); + bio_advance_iter(dst, dst_iter, bytes); + } +} +EXPORT_SYMBOL(bio_copy_data_iter); + +/** + * bio_copy_data - copy contents of data buffers from one bio to another + * @src: source bio + * @dst: destination bio + * + * Stops when it reaches the end of either @src or @dst - that is, copies + * min(src->bi_size, dst->bi_size) bytes (or the equivalent for lists of bios). + */ +void bio_copy_data(struct bio *dst, struct bio *src) +{ + struct bvec_iter src_iter = src->bi_iter; + struct bvec_iter dst_iter = dst->bi_iter; + + bio_copy_data_iter(dst, &dst_iter, src, &src_iter); +} +EXPORT_SYMBOL(bio_copy_data); + +/** + * bio_list_copy_data - copy contents of data buffers from one chain of bios to + * another + * @src: source bio list + * @dst: destination bio list + * + * Stops when it reaches the end of either the @src list or @dst list - that is, + * copies min(src->bi_size, dst->bi_size) bytes (or the equivalent for lists of + * bios). + */ +void bio_list_copy_data(struct bio *dst, struct bio *src) +{ + struct bvec_iter src_iter = src->bi_iter; + struct bvec_iter dst_iter = dst->bi_iter; + while (1) { if (!src_iter.bi_size) { src = src->bi_next; @@ -995,45 +1049,10 @@ void bio_copy_data_iter(struct bio *dst, struct bvec_iter dst_iter, dst_iter = dst->bi_iter; } - src_bv = bio_iter_iovec(src, src_iter); - dst_bv = bio_iter_iovec(dst, dst_iter); - - bytes = min(src_bv.bv_len, dst_bv.bv_len); - - src_p = kmap_atomic(src_bv.bv_page); - dst_p = kmap_atomic(dst_bv.bv_page); - - memcpy(dst_p + dst_bv.bv_offset, - src_p + src_bv.bv_offset, - bytes); - - kunmap_atomic(dst_p); - kunmap_atomic(src_p); - - bio_advance_iter(src, &src_iter, bytes); - bio_advance_iter(dst, &dst_iter, bytes); + bio_copy_data_iter(dst, &dst_iter, src, &src_iter); } } -EXPORT_SYMBOL(bio_copy_data_iter); - -/** - * bio_copy_data - copy contents of data buffers from one chain of bios to - * another - * @src: source bio list - * @dst: destination bio list - * - * If @src and @dst are single bios, bi_next must be NULL - otherwise, treats - * @src and @dst as linked lists of bios. - * - * Stops when it reaches the end of either @src or @dst - that is, copies - * min(src->bi_size, dst->bi_size) bytes (or the equivalent for lists of bios). - */ -void bio_copy_data(struct bio *dst, struct bio *src) -{ - bio_copy_data_iter(dst, dst->bi_iter, - src, src->bi_iter); -} -EXPORT_SYMBOL(bio_copy_data); +EXPORT_SYMBOL(bio_list_copy_data); struct bio_map_data { int is_our_pages; diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c index ccfcf544830f..d8aff7f3256a 100644 --- a/drivers/block/pktcdvd.c +++ b/drivers/block/pktcdvd.c @@ -1285,7 +1285,7 @@ static void pkt_start_write(struct pktcdvd_device *pd, struct packet_data *pkt) * Fill-in bvec with data from orig_bios. */ spin_lock(&pkt->lock); - bio_copy_data(pkt->w_bio, pkt->orig_bios.head); + bio_list_copy_data(pkt->w_bio, pkt->orig_bios.head); pkt_set_state(pkt, PACKET_WRITE_WAIT_STATE); spin_unlock(&pkt->lock); diff --git a/include/linux/bio.h b/include/linux/bio.h index 5a6ee955a8ac..98b175cc00d5 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -505,9 +505,10 @@ static inline void bio_flush_dcache_pages(struct bio *bi) } #endif -extern void bio_copy_data_iter(struct bio *dst, struct bvec_iter dst_iter, - struct bio *src, struct bvec_iter src_iter); +extern void bio_copy_data_iter(struct bio *dst, struct bvec_iter *dst_iter, + struct bio *src, struct bvec_iter *src_iter); extern void bio_copy_data(struct bio *dst, struct bio *src); +extern void bio_list_copy_data(struct bio *dst, struct bio *src); extern void bio_free_pages(struct bio *bio); extern struct bio *bio_copy_user_iov(struct request_queue *,