Re: [PATCH v9 02/13] iov_iter: add iterator type for dmabuf maps

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] lib: iov_iter_dmabuf_map 64-bit offset truncation and revert underflow
--

commit 51412f5d857b723775fa3b8143fbc7dfc3a8a633
Author: Pavel Begunkov <asml.silence@gmail.com>

iov_iter: add iterator type for dmabuf maps
    
This commit introduces a new iterator type, ITER_DMABUF_MAP, for dmabuf maps.
The map is an opaque object specific to the subsystem or driver, and the
iterator is used by the middle layers to pass the map further down for IO
operations.

> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> index 2072c04e99d05..6831a5d9396e1 100644
> --- a/lib/iov_iter.c
> +++ b/lib/iov_iter.c

[ ... ]

> @@ -631,7 +632,8 @@ void iov_iter_revert(struct iov_iter *i, size_t unroll)
>  		return;
>  	}
>  	unroll -= i->iov_offset;
> -	if (iov_iter_is_xarray(i) || iter_is_ubuf(i)) {
> +	if (iov_iter_is_xarray(i) || iter_is_ubuf(i) ||
> +	    iov_iter_is_dmabuf_map(i)) {
>  		BUG(); /* We should never go beyond the start of the specified

[Severity: High]
Because ITER_DMABUF_MAP sets iov_offset to the absolute file offset in
iov_iter_dmabuf_map(), does this break the safety invariant here?

For iterators validated by this BUG() check (such as ITER_XARRAY and
ITER_UBUF), iov_offset is normally strictly initialized to 0 to track the
relative number of bytes advanced. Since iov_offset starts at the absolute
offset for ITER_DMABUF_MAP, the condition if (unroll <= i->iov_offset) just
above this hunk could evaluate to true even if the caller attempts to unroll
more bytes than were actually advanced, potentially allowing silent
out-of-bounds iteration.

[ ... ]

> @@ -775,6 +777,20 @@ void iov_iter_xarray(struct iov_iter *i, unsigned int direction,
>  }
>  EXPORT_SYMBOL(iov_iter_xarray);
>  
> +void iov_iter_dmabuf_map(struct iov_iter *i, unsigned int direction,
> +			 struct dma_buf_io_map *map,
> +			 loff_t off, size_t count)
> +{
> +	WARN_ON(direction & ~(READ | WRITE));
> +	*i = (struct iov_iter){
> +		.iter_type = ITER_DMABUF_MAP,
> +		.data_source = direction,
> +		.dmabuf_map = map,
> +		.count = count,
> +		.iov_offset = off,

[Severity: High]
Does this silently truncate the 64-bit offset?

The off argument is a 64-bit loff_t, but iov_offset is a size_t (which is 32
bits on 32-bit platforms). Any offset >= 4GB will be silently truncated here.
Unlike ITER_XARRAY which uses a separate 64-bit xarray_start union field,
ITER_DMABUF_MAP doesn't store this absolute offset anywhere else.

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