| Message ID | 1592650156-25845-1-git-send-email-raphael.norwitz@nutanix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] Stop vhost-user sending uninitialized mmap_offsets | expand |
On Tue, 23 Jun 2020 at 00:50, Raphael Norwitz <raphael.norwitz@nutanix.com> wrote: > > Prior to this change, the vhost_user_fill_msg_region function filled out > all elements of the VhostUserMemoryRegion struct except the mmap_offset. > > This function is often called on uninitialized structs, which are then > copied into VHOST_USER_SET_MEM_TABLE and VHOST_USER_ADD/REM_MEM_REG > messages. In some cases, where the mmap_offset was not needed, it was > left uninitialized, causing QEMU to send the backend uninitialized data, > which Coverity flagged as a series of issues. > > This change augments the vhost_user_fill_msg_region API, adding a > mmap_offset paramenter, forcing the caller to initialize mmap_offset. > > Fixes: ece99091c2d0aeb23734289a50ef2ff4e0a08929 > Fixes: f1aeb14b0809e313c74244d838645ed25e85ea63 > Reported-by: Coverity (CIDs 1429802, 1429803 and 1429804) > Suggested-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com> > --- Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On Tue, Jun 23, 2020 at 09:58:23AM +0100, Peter Maydell wrote: > On Tue, 23 Jun 2020 at 00:50, Raphael Norwitz > <raphael.norwitz@nutanix.com> wrote: > > > > Prior to this change, the vhost_user_fill_msg_region function filled out > > all elements of the VhostUserMemoryRegion struct except the mmap_offset. > > > > This function is often called on uninitialized structs, which are then > > copied into VHOST_USER_SET_MEM_TABLE and VHOST_USER_ADD/REM_MEM_REG > > messages. In some cases, where the mmap_offset was not needed, it was > > left uninitialized, causing QEMU to send the backend uninitialized data, > > which Coverity flagged as a series of issues. > > > > This change augments the vhost_user_fill_msg_region API, adding a > > mmap_offset paramenter, forcing the caller to initialize mmap_offset. > > > > Fixes: ece99091c2d0aeb23734289a50ef2ff4e0a08929 > > Fixes: f1aeb14b0809e313c74244d838645ed25e85ea63 > > Reported-by: Coverity (CIDs 1429802, 1429803 and 1429804) > > Suggested-by: Peter Maydell <peter.maydell@linaro.org> > > Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com> > > --- > > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > thanks > -- PMM Queued, thanks!
On Mon, Jun 22, 2020 at 11:50:44PM +0000, Raphael Norwitz wrote: > Prior to this change, the vhost_user_fill_msg_region function filled out > all elements of the VhostUserMemoryRegion struct except the mmap_offset. > > This function is often called on uninitialized structs, which are then > copied into VHOST_USER_SET_MEM_TABLE and VHOST_USER_ADD/REM_MEM_REG > messages. In some cases, where the mmap_offset was not needed, it was > left uninitialized, causing QEMU to send the backend uninitialized data, > which Coverity flagged as a series of issues. > > This change augments the vhost_user_fill_msg_region API, adding a > mmap_offset paramenter, forcing the caller to initialize mmap_offset. > > Fixes: ece99091c2d0aeb23734289a50ef2ff4e0a08929 > Fixes: f1aeb14b0809e313c74244d838645ed25e85ea63 > Reported-by: Coverity (CIDs 1429802, 1429803 and 1429804) > Suggested-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com> > --- > hw/virtio/vhost-user.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 4d6cd4e..3123121 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -460,12 +460,14 @@ static MemoryRegion *vhost_user_get_mr_data(uint64_t addr, ram_addr_t *offset, } static void vhost_user_fill_msg_region(VhostUserMemoryRegion *dst, - struct vhost_memory_region *src) + struct vhost_memory_region *src, + uint64_t mmap_offset) { assert(src != NULL && dst != NULL); dst->userspace_addr = src->userspace_addr; dst->memory_size = src->memory_size; dst->guest_phys_addr = src->guest_phys_addr; + dst->mmap_offset = mmap_offset; } static int vhost_user_fill_set_mem_table_msg(struct vhost_user *u, @@ -500,9 +502,8 @@ static int vhost_user_fill_set_mem_table_msg(struct vhost_user *u, error_report("Failed preparing vhost-user memory table msg"); return -1; } - vhost_user_fill_msg_region(®ion_buffer, reg); + vhost_user_fill_msg_region(®ion_buffer, reg, offset); msg->payload.memory.regions[*fd_num] = region_buffer; - msg->payload.memory.regions[*fd_num].mmap_offset = offset; fds[(*fd_num)++] = fd; } else if (track_ramblocks) { u->region_rb_offset[i] = 0; @@ -649,7 +650,7 @@ static int send_remove_regions(struct vhost_dev *dev, if (fd > 0) { msg->hdr.request = VHOST_USER_REM_MEM_REG; - vhost_user_fill_msg_region(®ion_buffer, shadow_reg); + vhost_user_fill_msg_region(®ion_buffer, shadow_reg, 0); msg->payload.mem_reg.region = region_buffer; if (vhost_user_write(dev, msg, &fd, 1) < 0) { @@ -709,9 +710,8 @@ static int send_add_regions(struct vhost_dev *dev, u->region_rb[reg_idx] = mr->ram_block; } msg->hdr.request = VHOST_USER_ADD_MEM_REG; - vhost_user_fill_msg_region(®ion_buffer, reg); + vhost_user_fill_msg_region(®ion_buffer, reg, offset); msg->payload.mem_reg.region = region_buffer; - msg->payload.mem_reg.region.mmap_offset = offset; if (vhost_user_write(dev, msg, &fd, 1) < 0) { return -1;
Prior to this change, the vhost_user_fill_msg_region function filled out all elements of the VhostUserMemoryRegion struct except the mmap_offset. This function is often called on uninitialized structs, which are then copied into VHOST_USER_SET_MEM_TABLE and VHOST_USER_ADD/REM_MEM_REG messages. In some cases, where the mmap_offset was not needed, it was left uninitialized, causing QEMU to send the backend uninitialized data, which Coverity flagged as a series of issues. This change augments the vhost_user_fill_msg_region API, adding a mmap_offset paramenter, forcing the caller to initialize mmap_offset. Fixes: ece99091c2d0aeb23734289a50ef2ff4e0a08929 Fixes: f1aeb14b0809e313c74244d838645ed25e85ea63 Reported-by: Coverity (CIDs 1429802, 1429803 and 1429804) Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com> --- hw/virtio/vhost-user.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)