| Message ID | 1579143426-18305-2-git-send-email-raphael.norwitz@nutanix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vhost-user: Lift Max Ram Slots Limitation | expand |
On Wed, Jan 15, 2020 at 09:57:04PM -0500, Raphael Norwitz wrote: > The current vhost_user_set_mem_table_postcopy() implementation > populates each region of the VHOST_USER_SET_MEM_TABLE message without > first checking if there are more than VHOST_MEMORY_MAX_NREGIONS already > populated. This can cause memory corruption if too many regions are > added to the message during the postcopy step. > > This change moves an existing assert up such that attempting to > construct a VHOST_USER_SET_MEM_TABLE message with too many memory > regions will gracefully bring down qemu instead of corrupting memory. > > Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com> > Signed-off-by: Peter Turschmid <peter.turschm@nutanix.com> Could you pls add Fixes: and stable tags? > --- > hw/virtio/vhost-user.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 2e81f55..cce851a 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -443,6 +443,7 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev, > &offset); > fd = memory_region_get_fd(mr); > if (fd > 0) { > + assert(fd_num < VHOST_MEMORY_MAX_NREGIONS); > trace_vhost_user_set_mem_table_withfd(fd_num, mr->name, > reg->memory_size, > reg->guest_phys_addr, > @@ -455,7 +456,6 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev, > msg.payload.memory.regions[fd_num].guest_phys_addr = > reg->guest_phys_addr; > msg.payload.memory.regions[fd_num].mmap_offset = offset; > - assert(fd_num < VHOST_MEMORY_MAX_NREGIONS); > fds[fd_num++] = fd; > } else { > u->region_rb_offset[i] = 0; > -- > 1.8.3.1
On Thu, Feb 06, 2020 at 03:17:04AM -0500, Michael S. Tsirkin wrote: > On Wed, Jan 15, 2020 at 09:57:04PM -0500, Raphael Norwitz wrote: > > The current vhost_user_set_mem_table_postcopy() implementation > > populates each region of the VHOST_USER_SET_MEM_TABLE message without > > first checking if there are more than VHOST_MEMORY_MAX_NREGIONS already > > populated. This can cause memory corruption if too many regions are > > added to the message during the postcopy step. > > > > This change moves an existing assert up such that attempting to > > construct a VHOST_USER_SET_MEM_TABLE message with too many memory > > regions will gracefully bring down qemu instead of corrupting memory. > > > > Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com> > > Signed-off-by: Peter Turschmid <peter.turschm@nutanix.com> > > > Could you pls add Fixes: and stable tags? oh wait no, this is just a theoretical thing, right? it doesn't actually trigger, it's just a cleanup. no fixes/stable needed then, sorry > > --- > > hw/virtio/vhost-user.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > index 2e81f55..cce851a 100644 > > --- a/hw/virtio/vhost-user.c > > +++ b/hw/virtio/vhost-user.c > > @@ -443,6 +443,7 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev, > > &offset); > > fd = memory_region_get_fd(mr); > > if (fd > 0) { > > + assert(fd_num < VHOST_MEMORY_MAX_NREGIONS); > > trace_vhost_user_set_mem_table_withfd(fd_num, mr->name, > > reg->memory_size, > > reg->guest_phys_addr, > > @@ -455,7 +456,6 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev, > > msg.payload.memory.regions[fd_num].guest_phys_addr = > > reg->guest_phys_addr; > > msg.payload.memory.regions[fd_num].mmap_offset = offset; > > - assert(fd_num < VHOST_MEMORY_MAX_NREGIONS); > > fds[fd_num++] = fd; > > } else { > > u->region_rb_offset[i] = 0; > > -- > > 1.8.3.1
Yes - it's just a cleanup. On Thu, Feb 06, 2020 at 03:20:01AM -0500, Michael S. Tsirkin wrote: > > On Thu, Feb 06, 2020 at 03:17:04AM -0500, Michael S. Tsirkin wrote: > > On Wed, Jan 15, 2020 at 09:57:04PM -0500, Raphael Norwitz wrote: > > > The current vhost_user_set_mem_table_postcopy() implementation > > > populates each region of the VHOST_USER_SET_MEM_TABLE message without > > > first checking if there are more than VHOST_MEMORY_MAX_NREGIONS already > > > populated. This can cause memory corruption if too many regions are > > > added to the message during the postcopy step. > > > > > > This change moves an existing assert up such that attempting to > > > construct a VHOST_USER_SET_MEM_TABLE message with too many memory > > > regions will gracefully bring down qemu instead of corrupting memory. > > > > > > Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com> > > > Signed-off-by: Peter Turschmid <peter.turschm@nutanix.com> > > > > > > Could you pls add Fixes: and stable tags? > > oh wait no, this is just a theoretical thing, right? > it doesn't actually trigger, it's just a cleanup. > > no fixes/stable needed then, sorry >
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 2e81f55..cce851a 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -443,6 +443,7 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev, &offset); fd = memory_region_get_fd(mr); if (fd > 0) { + assert(fd_num < VHOST_MEMORY_MAX_NREGIONS); trace_vhost_user_set_mem_table_withfd(fd_num, mr->name, reg->memory_size, reg->guest_phys_addr, @@ -455,7 +456,6 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev, msg.payload.memory.regions[fd_num].guest_phys_addr = reg->guest_phys_addr; msg.payload.memory.regions[fd_num].mmap_offset = offset; - assert(fd_num < VHOST_MEMORY_MAX_NREGIONS); fds[fd_num++] = fd; } else { u->region_rb_offset[i] = 0;