Re: [Qemu-devel] [PATCH v2 2/2] vhost: double check used memslots number

[Igor Mammedov]

On Sat, 23 Dec 2017 08:27:25 +0000
"Zhoujian (jay)" <address@hidden> wrote:

> 
> 
> > -----Original Message-----
> > From: Igor Mammedov [mailto:address@hidden
> > Sent: Saturday, December 23, 2017 2:49 AM
> > To: Zhoujian (jay) <address@hidden>
> > Cc: address@hidden; address@hidden; Huangweidong (C)
> > <address@hidden>; Gonglei (Arei) <address@hidden>;
> > wangxin (U) <address@hidden>; Liuzhe (Cloud Open Labs, NFV)
> > <address@hidden>; address@hidden
> > Subject: Re: [PATCH v2 2/2] vhost: double check used memslots number
> > 
> > On Fri, 15 Dec 2017 16:45:55 +0800
> > Jay Zhou <address@hidden> wrote:
> > 
> > > If the VM already has N(N>8) available memory slots for vhost user,
> > > the VM will be crashed in vhost_user_set_mem_table if we try to
> > > hotplug the first vhost user NIC.
> > > This patch checks if memslots number exceeded or not after updating
> > > vhost_user_used_memslots.
> > Can't understand commit message, pls rephrase (what is being fixed, and
> > how it's fixed) also include reproducing steps for crash and maybe
> > describe call flow/backtrace that triggers crash.
> 
> Sorry about my pool english
> 
> > 
> > PS:
> > I wasn't able to reproduce crash
> 
> Steps to reproduce:
> (1) start up a VM successfully without any vhost device
> (2) hotplug 8 DIMM memory successfully
> (3) hotplug a vhost-user NIC, the VM crashed, it asserted
>     at the line
>         assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
quick fix for this crash could be:

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 093675ed98..07a37537dd 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -321,7 +321,9 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
             msg.payload.memory.regions[fd_num].memory_size  = reg->memory_size;
             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);
+            if (fd_num == VHOST_MEMORY_MAX_NREGIONS) {
+                return -1;
+            }
             fds[fd_num++] = fd;
         }
     }

it should gracefully prevent device to start.

>     in vhost_user_set_mem_table()
> 
> Regards,
> Jay
[...]