| Message ID | CAKXUXMy=M42hapfG1S4ZT1v5WEdH2KYiF8Cgukmf48=FKFCyJg@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Potentially broken error path in bio_map_user_iov() | expand |
Hi Lukas, we can just remove the queue_dma_alignment check entirely, the caller already ensures bio_copy_user_iov is called instead of this case.
On Mon, Jan 3, 2022 at 5:35 PM Christoph Hellwig <hch@lst.de> wrote: > > Hi Lukas, > > we can just remove the queue_dma_alignment check entirely, the caller > already ensures bio_copy_user_iov is called instead of this case. Okay, I have sent you a patch to do so: https://lore.kernel.org/all/20220104120158.20177-1-lukas.bulwahn@gmail.com/ Feel free to pick it, reword it, rework it or ignore it. Lukas
diff --git a/block/blk-map.c b/block/blk-map.c index 4526adde0156..4a3f6703f46f 100644 --- a/block/blk-map.c +++ b/block/blk-map.c @@ -235,7 +235,7 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter, { unsigned int max_sectors = queue_max_hw_sectors(rq->q); struct bio *bio; - int ret; + int ret = 0; int j;
Dear Christoph, in the function bio_map_user_iov() in ./block/blk-map.c, there is an error branch for unlikely(offs & queue_dma_alignment(rq->q)), where the supposingly expected error return value of the function is set, i.e., ret = -EINVAL;. However, the variable ret is unconditionally reset by the blk_rq_append_bio(...) call below, so that this 'ret = -EINVAL;' assignment has no effect. I am unsure which control flow you expect for this error case through the bio_map_user_iov() function. Maybe you just want a patch like this below to return the error code if it is at least once set? if (!iov_iter_count(iter)) @@ -296,6 +296,9 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter, break; } + if (ret) + goto out_unmap; + ret = blk_rq_append_bio(rq, bio); if (ret) goto out_unmap; I know too little about this function and its intent to create a proper patch, though. I also looked at the previous versions, but the error code of this error branch was really never effectively returned before either (as far as I grasped the earlier versions of this function). So, this error path probably never worked as intended. Best regards, Lukas