On Sat, Jan 11, 2025 at 1:11 PM Akihiko Odaki <akihiko.od...@daynix.com> wrote: > > Jason, can you pull this series?
Queued. Thanks > > Regards, > Akihiko Odaki > > On 2024/05/08 23:51, Philippe Mathieu-Daudé wrote: > > ping? > > > > On 28/4/24 13:11, Akihiko Odaki wrote: > >> iov_from_buf(), iov_to_buf(), iov_memset(), and iov_copy() asserts > >> that the given offset fits in the iov while tolerating the specified > >> number of bytes to operate with to be greater than the size of iov. > >> This is inconsistent so remove the assertions. > >> > >> Asserting the offset fits in the iov makes sense if it is expected that > >> there are other operations that process the content before the offset > >> and the content is processed in order. Under this expectation, the > >> offset should point to the end of bytes that are previously processed > >> and fit in the iov. However, this expectation depends on the details of > >> the caller, and did not hold true at least one case and required code to > >> check iov_size(), which is added with commit 83ddb3dbba2e > >> ("hw/net/net_tx_pkt: Fix overrun in update_sctp_checksum()"). > >> > >> Adding such a check is inefficient and error-prone. These functions > >> already tolerate the specified number of bytes to operate with to be > >> greater than the size of iov to avoid such checks so remove the > >> assertions to tolerate invalid offset as well. They return the number of > >> bytes they operated with so their callers can still check the returned > >> value to ensure there are sufficient space at the given offset. > >> > >> Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com> > >> --- > >> include/qemu/iov.h | 5 +++-- > >> util/iov.c | 5 ----- > >> 2 files changed, 3 insertions(+), 7 deletions(-) > >> > >> diff --git a/include/qemu/iov.h b/include/qemu/iov.h > >> index 63a1c01965d1..33548058d2ee 100644 > >> --- a/include/qemu/iov.h > >> +++ b/include/qemu/iov.h > >> @@ -30,7 +30,7 @@ size_t iov_size(const struct iovec *iov, const > >> unsigned int iov_cnt); > >> * only part of data will be copied, up to the end of the iovec. > >> * Number of bytes actually copied will be returned, which is > >> * min(bytes, iov_size(iov)-offset) > >> - * `Offset' must point to the inside of iovec. > >> + * Returns 0 when `offset' points to the outside of iovec. > >> */ > >> size_t iov_from_buf_full(const struct iovec *iov, unsigned int iov_cnt, > >> size_t offset, const void *buf, size_t bytes); > >> @@ -66,11 +66,12 @@ iov_to_buf(const struct iovec *iov, const unsigned > >> int iov_cnt, > >> /** > >> * Set data bytes pointed out by iovec `iov' of size `iov_cnt' > >> elements, > >> * starting at byte offset `start', to value `fillc', repeating it > >> - * `bytes' number of times. `Offset' must point to the inside of iovec. > >> + * `bytes' number of times. > >> * If `bytes' is large enough, only last bytes portion of iovec, > >> * up to the end of it, will be filled with the specified value. > >> * Function return actual number of bytes processed, which is > >> * min(size, iov_size(iov) - offset). > >> + * Returns 0 when `offset' points to the outside of iovec. > >> */ > >> size_t iov_memset(const struct iovec *iov, const unsigned int iov_cnt, > >> size_t offset, int fillc, size_t bytes); > >> diff --git a/util/iov.c b/util/iov.c > >> index 7e73948f5e3d..a523b406b7f8 100644 > >> --- a/util/iov.c > >> +++ b/util/iov.c > >> @@ -36,7 +36,6 @@ size_t iov_from_buf_full(const struct iovec *iov, > >> unsigned int iov_cnt, > >> offset -= iov[i].iov_len; > >> } > >> } > >> - assert(offset == 0); > >> return done; > >> } > >> @@ -55,7 +54,6 @@ size_t iov_to_buf_full(const struct iovec *iov, > >> const unsigned int iov_cnt, > >> offset -= iov[i].iov_len; > >> } > >> } > >> - assert(offset == 0); > >> return done; > >> } > >> @@ -74,7 +72,6 @@ size_t iov_memset(const struct iovec *iov, const > >> unsigned int iov_cnt, > >> offset -= iov[i].iov_len; > >> } > >> } > >> - assert(offset == 0); > >> return done; > >> } > >> @@ -266,7 +263,6 @@ unsigned iov_copy(struct iovec *dst_iov, unsigned > >> int dst_iov_cnt, > >> bytes -= len; > >> offset = 0; > >> } > >> - assert(offset == 0); > >> return j; > >> } > >> @@ -337,7 +333,6 @@ size_t qemu_iovec_concat_iov(QEMUIOVector *dst, > >> soffset -= src_iov[i].iov_len; > >> } > >> } > >> - assert(soffset == 0); /* offset beyond end of src */ > >> return done; > >> } > >> >