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 <[email protected]> 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/[email protected]?part=2
