On Tue, 27 Aug 2019 at 21:16, Stefan Hajnoczi <stefa...@redhat.com> wrote: > > From: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > > Introduce new initialization API, to create requests with padding. Will > be used in the following patch. New API uses qemu_iovec_init_buf if > resulting io vector has only one element, to avoid extra allocations. > So, we need to update qemu_iovec_destroy to support destroying such > QIOVs. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > Acked-by: Stefan Hajnoczi <stefa...@redhat.com> > Message-id: 20190604161514.262241-2-vsement...@virtuozzo.com > Message-Id: <20190604161514.262241-2-vsement...@virtuozzo.com> > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
Hi -- Coverity thinks this new function could have an out-of-bounds read (CID 1405302): > +/* > + * Compile new iovec, combining @head_buf buffer, sub-qiov of @mid_qiov, > + * and @tail_buf buffer into new qiov. > + */ > +void qemu_iovec_init_extended( > + QEMUIOVector *qiov, > + void *head_buf, size_t head_len, > + QEMUIOVector *mid_qiov, size_t mid_offset, size_t mid_len, > + void *tail_buf, size_t tail_len) > +{ > + size_t mid_head, mid_tail; > + int total_niov, mid_niov = 0; > + struct iovec *p, *mid_iov; > + > + if (mid_len) { > + mid_iov = qiov_slice(mid_qiov, mid_offset, mid_len, > + &mid_head, &mid_tail, &mid_niov); > + } > + > + total_niov = !!head_len + mid_niov + !!tail_len; > + if (total_niov == 1) { > + qemu_iovec_init_buf(qiov, NULL, 0); > + p = &qiov->local_iov; > + } else { > + qiov->niov = qiov->nalloc = total_niov; > + qiov->size = head_len + mid_len + tail_len; > + p = qiov->iov = g_new(struct iovec, qiov->niov); > + } > + > + if (head_len) { > + p->iov_base = head_buf; > + p->iov_len = head_len; > + p++; > + } > + > + if (mid_len) { > + memcpy(p, mid_iov, mid_niov * sizeof(*p)); > + p[0].iov_base = (uint8_t *)p[0].iov_base + mid_head; > + p[0].iov_len -= mid_head; > + p[mid_niov - 1].iov_len -= mid_tail; > + p += mid_niov; > + } > + > + if (tail_len) { > + p->iov_base = tail_buf; > + p->iov_len = tail_len; > + } > +} but I'm not familiar enough with the code to be able to tell if it's correct or if it's just getting confused. Could somebody have a look? (It's possible it's getting confused because the calculation of 'total_niov' uses 'mid_niov', but the condition guarding the code that fills in that part of the vector is 'mid_len', so it thinks it can take the "total_niov == 1" codepath and also the "head_len == true" and "mid_len != 0" paths; in which case using "if (mid_niov)" instead might make it happier.) thanks -- PMM