[PATCH] ublk: don't mutate struct bio_vec in iteration

[PATCH] ublk: don't mutate struct bio_vec in iteration

[Caleb Sander Mateos]

__bio_for_each_segment() uses the returned struct bio_vec's bv_len field
to advance the struct bvec_iter at the end of each loop iteration. So
it's incorrect to modify it during the loop. Don't assign to bv_len (or
bv_offset, for that matter) in ublk_copy_user_pages().

Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Fixes: e87d66ab27ac ("ublk: use rq_for_each_segment() for user copy")
---
 drivers/block/ublk_drv.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 2c715df63f23..1e1a167d776d 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -924,30 +924,30 @@ static size_t ublk_copy_user_pages(const struct request *req,
 	struct req_iterator iter;
 	struct bio_vec bv;
 	size_t done = 0;
 
 	rq_for_each_segment(bv, req, iter) {
+		unsigned len;
 		void *bv_buf;
 		size_t copied;
 
 		if (offset >= bv.bv_len) {
 			offset -= bv.bv_len;
 			continue;
 		}
 
-		bv.bv_offset += offset;
-		bv.bv_len -= offset;
-		bv_buf = bvec_kmap_local(&bv);
+		len = bv.bv_len - offset;
+		bv_buf = kmap_local_page(bv.bv_page) + bv.bv_offset + offset;
 		if (dir == ITER_DEST)
-			copied = copy_to_iter(bv_buf, bv.bv_len, uiter);
+			copied = copy_to_iter(bv_buf, len, uiter);
 		else
-			copied = copy_from_iter(bv_buf, bv.bv_len, uiter);
+			copied = copy_from_iter(bv_buf, len, uiter);
 
 		kunmap_local(bv_buf);
 
 		done += copied;
-		if (copied < bv.bv_len)
+		if (copied < len)
 			break;
 
 		offset = 0;
 	}
 	return done;
-- 
2.45.2

Re: [PATCH] ublk: don't mutate struct bio_vec in iteration

[Ming Lei]

On Mon, Dec 08, 2025 at 08:14:23PM -0700, Caleb Sander Mateos wrote:
> __bio_for_each_segment() uses the returned struct bio_vec's bv_len field
> to advance the struct bvec_iter at the end of each loop iteration. So
> it's incorrect to modify it during the loop. Don't assign to bv_len (or
> bv_offset, for that matter) in ublk_copy_user_pages().
> 
> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> Fixes: e87d66ab27ac ("ublk: use rq_for_each_segment() for user copy")

Reviewed-by: Ming Lei <ming.lei@redhat.com>


Thanks,
Ming

Re: [PATCH] ublk: don't mutate struct bio_vec in iteration

[Jens Axboe]

On Mon, 08 Dec 2025 20:14:23 -0700, Caleb Sander Mateos wrote:
> __bio_for_each_segment() uses the returned struct bio_vec's bv_len field
> to advance the struct bvec_iter at the end of each loop iteration. So
> it's incorrect to modify it during the loop. Don't assign to bv_len (or
> bv_offset, for that matter) in ublk_copy_user_pages().
> 
> 

Applied, thanks!

[1/1] ublk: don't mutate struct bio_vec in iteration
      commit: db339b4067eccb7fa3d9787d5d3ab5d466fd9efa

Best regards,
-- 
Jens Axboe