Re: [PATCH v11 04/13] block: introduce dma map backed bio type

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] block: type confusion in blk_rq_cur_bytes() due to bio_has_data() including REQ_DMABUF
--

commit 338a7d60f029eb7fdd6fca422f1d54a04222c016
Author: Pavel Begunkov <asml.silence@gmail.com>

block: introduce dma map backed bio type

This patch introduces a dma map backed bio type to support pre-mapped
buffers without requiring a generic bio_vec. It repurposes the bi_io_vec
space in struct bio to store a dma_buf_io_map when the REQ_DMABUF flag
is set. It also adds specific splitting logic based on the minimum
segment size provided by the map.

[ ... ]

> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index 98e21b4cbf328..0cc09b975d8ff 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -233,7 +233,12 @@ struct bio {
>  	atomic_t		__bi_remaining;
>  
>  	/* The actual vec list, preserved by bio_reset() */
> -	struct bio_vec		*bi_io_vec;
> +	union {
> +		struct bio_vec		*bi_io_vec;
> +		/* Driver specific dma map, valid IFF REQ_DMABUF is set */
> +		struct dma_buf_io_map	*bi_dmabuf_map;
> +	};
> +

[Severity: High]
Does this union introduce type confusion when existing block layer helpers
check bio_has_data()?

Because REQ_DMABUF is a flag rather than an operation type, bio_has_data() in
include/linux/bio.h will still return true for read/write requests carrying
this flag.

When a driver partially completes a REQ_DMABUF request, blk_update_request()
calls blk_rq_cur_bytes() in include/linux/blk-mq.h to verify the size of the
remaining segment:

include/linux/blk-mq.h:blk_rq_cur_bytes() {
    ...
    if (!bio_has_data(rq->bio))
        return rq->bio->bi_iter.bi_size;
    return bio_iovec(rq->bio).bv_len;
}

Since bio_has_data() is true, blk_rq_cur_bytes() invokes
bio_iovec(rq->bio).bv_len. This macro treats the bio->bi_dmabuf_map pointer
as an array of struct bio_vec, reading bv_len from the overlapping bytes of
the dma_buf_io_map struct.

If this garbage value is larger than the remaining blk_rq_bytes(req), it
corrupts req->__data_len and can trigger out-of-bounds memory access or
panics when the request resumes. Should bio_has_data() be updated to
exclude REQ_DMABUF, or is there another way to prevent blk_rq_cur_bytes()
from blindly dereferencing the dma map as a bio_vec?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791975048.git.asml.silence@gmail.com?part=4