| Message ID | 1573643008-8216-1-git-send-email-wangzk320@163.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Marchand |
| Headers | show |
| Series |
|
| Related | show |
| Context | Check | Description |
|---|---|---|
| ci/iol-mellanox-Performance | success | Performance Testing PASS |
| ci/Intel-compilation | success | Compilation OK |
| ci/iol-compilation | success | Compile Testing PASS |
| ci/iol-intel-Performance | success | Performance Testing PASS |
| ci/checkpatch | success | coding style OK |
Maybe change the title to something like: vhost: fix vring requests validation broken if no FD in payload On 11/13/19 12:03 PM, Zhike Wang wrote: > When VHOST_USER_VRING_NOFD_MASK is set, the fd_num is 0, > so validate_msg_fds() will return error. In this case, > the negotiation of vring message between vhost user front end and > back end would fail, and as a result, vhost user link could NOT be up. > > How to reproduce: > 1.Run dpdk testpmd insides VM, which locates at host with ovs+dpdk. > 2.Notice that inside ovs there are endless logs regarding failure to > handle VHOST_USER_SET_VRING_CALL, and link of vm could NOT be up. > > Fixes: bf47225 ("vhost: fix possible denial of service by leaking FDs") > Signed-off-by: Zhike Wang <wangzk320@163.com> > --- > lib/librte_vhost/vhost_user.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > otherwise: Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> The title can be changed while applying if you agree with it, no need to send v3. I'll take care of the different backports. Thanks, Maxime
Thanks Maxime. Go ahead, your proposal sounds good. Br, Zhike Wang JDCloud, Product Development, IaaS ------------------------------------------------------------------------------------------------ Mobile／+86 13466719566 E- mail／wangzhike@jd.com Address／5F Building A,North-Star Century Center,8 Beichen West Street,Chaoyang District Beijing Https://JDCloud.com ------------------------------------------------------------------------------------------------ -----Original Message----- From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] Sent: Thursday, November 14, 2019 4:53 PM To: Zhike Wang; dev@dpdk.org; stable@dpdk.org; security@dpdk.org Cc: 王志克 Subject: Re: [dpdk-dev] [PATCH v2] vhost: fix vring message handling broken in some case Maybe change the title to something like: vhost: fix vring requests validation broken if no FD in payload On 11/13/19 12:03 PM, Zhike Wang wrote: > When VHOST_USER_VRING_NOFD_MASK is set, the fd_num is 0, > so validate_msg_fds() will return error. In this case, > the negotiation of vring message between vhost user front end and > back end would fail, and as a result, vhost user link could NOT be up. > > How to reproduce: > 1.Run dpdk testpmd insides VM, which locates at host with ovs+dpdk. > 2.Notice that inside ovs there are endless logs regarding failure to > handle VHOST_USER_SET_VRING_CALL, and link of vm could NOT be up. > > Fixes: bf47225 ("vhost: fix possible denial of service by leaking FDs") > Signed-off-by: Zhike Wang <wangzk320@163.com> > --- > lib/librte_vhost/vhost_user.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > otherwise: Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> The title can be changed while applying if you agree with it, no need to send v3. I'll take care of the different backports. Thanks, Maxime
On 11/14/19 11:16 AM, 王志克 wrote: > Thanks Maxime. > Go ahead, your proposal sounds good. Thanks, could you provide the qemu command line you used to reproduce the issue, and also the kernel version? It will help to document the commit message. Regards, Maxime > Br, > Zhike Wang > JDCloud, Product Development, IaaS > ------------------------------------------------------------------------------------------------ > Mobile／+86 13466719566 > E- mail／wangzhike@jd.com > Address／5F Building A,North-Star Century Center,8 Beichen West Street,Chaoyang District Beijing > Https://JDCloud.com > ------------------------------------------------------------------------------------------------ > > > > -----Original Message----- > From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] > Sent: Thursday, November 14, 2019 4:53 PM > To: Zhike Wang; dev@dpdk.org; stable@dpdk.org; security@dpdk.org > Cc: 王志克 > Subject: Re: [dpdk-dev] [PATCH v2] vhost: fix vring message handling broken in some case > > Maybe change the title to something like: > > vhost: fix vring requests validation broken if no FD in payload > > On 11/13/19 12:03 PM, Zhike Wang wrote: >> When VHOST_USER_VRING_NOFD_MASK is set, the fd_num is 0, >> so validate_msg_fds() will return error. In this case, >> the negotiation of vring message between vhost user front end and >> back end would fail, and as a result, vhost user link could NOT be up. >> >> How to reproduce: >> 1.Run dpdk testpmd insides VM, which locates at host with ovs+dpdk. >> 2.Notice that inside ovs there are endless logs regarding failure to >> handle VHOST_USER_SET_VRING_CALL, and link of vm could NOT be up. >> >> Fixes: bf47225 ("vhost: fix possible denial of service by leaking FDs") >> Signed-off-by: Zhike Wang <wangzk320@163.com> >> --- >> lib/librte_vhost/vhost_user.c | 13 ++++++++++--- >> 1 file changed, 10 insertions(+), 3 deletions(-) >> > > otherwise: > > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > The title can be changed while applying if you agree with it, no need to > send v3. > > I'll take care of the different backports. > > Thanks, > Maxime >
Hi Maxime, Usually I define some xml file, and use virsh to start the VM. Below is got from ps command. /usr/libexec/qemu-kvm -name guest=test_virtio,debug-threads=on -S -object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-4-test_virtio/master-key.aes -machine pc-i440fx-rhel7.0.0,accel=kvm,usb=off,dump-guest-core=off -cpu host -m 8192 -realtime mlock=off -smp 9,sockets=1,cores=9,threads=1 -object memory-backend-file,id=ram-node0,prealloc=yes,mem-path=/mnt/huge_1GB/libvirt/qemu/4-test_virtio,share=yes,size=8589934592,host-nodes=0,policy=preferred -numa node,nodeid=0,cpus=0,memdev=ram-node0 -uuid 311a9da4-0f02-4e9a-b558-7b2123794639 -display none -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-4-test_virtio/monitor.sock,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc,driftfix=slew -global kvm-pit.lost_tick_policy=delay -no-hpet -no-reboot -global PIIX4_PM.disable_s3=1 -global PIIX4_PM.disable_s4=1 -boot strict=on -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 -device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x5 -drive file=/export/wangzhike/image/centos74.qcow2,format=qcow2,if=none,id=drive-virtio-disk0 -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x6,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=2 -netdev tap,fd=26,id=hostnet0,vhost=on,vhostfd=28 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:92:db:8c,bus=pci.0,addr=0x8 -chardev socket,id=charnet1,path=/var/lib/openvswitch/port-vhost0,server -netdev vhost-user,chardev=charnet1,queues=4,id=hostnet1 -device virtio-net-pci,mrg_rxbuf=on,mq=on,vectors=10,rx_queue_size=1024,netdev=hostnet1,id=net1,mac=e4:22:22:33:44:01,bus=pci.0,multifunction=on,addr=0x9 -chardev socket,id=charnet2,path=/var/lib/openvswitch/port-vhost1,server -netdev vhost-user,chardev=charnet2,queues=4,id=hostnet2 -device virtio-net-pci,mrg_rxbuf=on,mq=on,vectors=10,rx_queue_size=1024,netdev=hostnet2,id=net2,mac=e4:22:22:33:44:02,bus=pci.0,addr=0x9.0x1 -chardev socket,id=charnet3,path=/var/lib/openvswitch/port-vhost2,server -netdev vhost-user,chardev=charnet3,queues=4,id=hostnet3 -device virtio-net-pci,mrg_rxbuf=on,mq=on,vectors=10,rx_queue_size=1024,netdev=hostnet3,id=net3,mac=e4:22:22:33:44:03,bus=pci.0,addr=0x9.0x2 -chardev socket,id=charnet4,path=/var/lib/openvswitch/port-vhost3,server -netdev vhost-user,chardev=charnet4,queues=4,id=hostnet4 -device virtio-net-pci,mrg_rxbuf=on,mq=on,vectors=10,rx_queue_size=1024,netdev=hostnet4,id=net4,mac=e4:22:22:33:44:04,bus=pci.0,addr=0x9.0x3 -chardev socket,id=charnet5,path=/var/lib/openvswitch/port-vhost4,server -netdev vhost-user,chardev=charnet5,queues=4,id=hostnet5 -device virtio-net-pci,mrg_rxbuf=on,mq=on,vectors=10,rx_queue_size=1024,netdev=hostnet5,id=net5,mac=e4:22:22:33:44:05,bus=pci.0,addr=0x9.0x4 -chardev socket,id=charnet6,path=/var/lib/openvswitch/port-vhost5,server -netdev vhost-user,chardev=charnet6,queues=4,id=hostnet6 -device virtio-net-pci,mrg_rxbuf=on,mq=on,vectors=10,rx_queue_size=1024,netdev=hostnet6,id=net6,mac=e4:22:22:33:44:06,bus=pci.0,addr=0x9.0x5 -chardev socket,id=charnet7,path=/var/lib/openvswitch/port-vhost6,server -netdev vhost-user,chardev=charnet7,queues=4,id=hostnet7 -device virtio-net-pci,mrg_rxbuf=on,mq=on,vectors=10,rx_queue_size=1024,netdev=hostnet7,id=net7,mac=e4:22:22:33:44:07,bus=pci.0,addr=0x9.0x6 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -chardev socket,id=charserial1,host=127.0.0.1,port=2445,telnet,server,nowait -device isa-serial,chardev=charserial1,id=serial1 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x7 -msg timestamp=on Both host and VM use CentOS7.4 with kernel 3.10.0-693.el7.x86_64. Testpmd insides VM uses DPDK18.11. Br, Zhike Wang JDCloud, Product Development, IaaS ------------------------------------------------------------------------------------------------ Mobile／+86 13466719566 E- mail／wangzhike@jd.com Address／5F Building A,North-Star Century Center,8 Beichen West Street,Chaoyang District Beijing Https://JDCloud.com ------------------------------------------------------------------------------------------------ -----Original Message----- From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] Sent: Thursday, November 14, 2019 6:24 PM To: 王志克; Zhike Wang; dev@dpdk.org; stable@dpdk.org; security@dpdk.org Subject: Re: [dpdk-dev] [PATCH v2] vhost: fix vring message handling broken in some case On 11/14/19 11:16 AM, 王志克 wrote: > Thanks Maxime. > Go ahead, your proposal sounds good. Thanks, could you provide the qemu command line you used to reproduce the issue, and also the kernel version? It will help to document the commit message. Regards, Maxime > Br, > Zhike Wang > JDCloud, Product Development, IaaS > ------------------------------------------------------------------------------------------------ > Mobile／+86 13466719566 > E- mail／wangzhike@jd.com > Address／5F Building A,North-Star Century Center,8 Beichen West Street,Chaoyang District Beijing > Https://JDCloud.com > ------------------------------------------------------------------------------------------------ > > > > -----Original Message----- > From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] > Sent: Thursday, November 14, 2019 4:53 PM > To: Zhike Wang; dev@dpdk.org; stable@dpdk.org; security@dpdk.org > Cc: 王志克 > Subject: Re: [dpdk-dev] [PATCH v2] vhost: fix vring message handling broken in some case > > Maybe change the title to something like: > > vhost: fix vring requests validation broken if no FD in payload > > On 11/13/19 12:03 PM, Zhike Wang wrote: >> When VHOST_USER_VRING_NOFD_MASK is set, the fd_num is 0, >> so validate_msg_fds() will return error. In this case, >> the negotiation of vring message between vhost user front end and >> back end would fail, and as a result, vhost user link could NOT be up. >> >> How to reproduce: >> 1.Run dpdk testpmd insides VM, which locates at host with ovs+dpdk. >> 2.Notice that inside ovs there are endless logs regarding failure to >> handle VHOST_USER_SET_VRING_CALL, and link of vm could NOT be up. >> >> Fixes: bf47225 ("vhost: fix possible denial of service by leaking FDs") >> Signed-off-by: Zhike Wang <wangzk320@163.com> >> --- >> lib/librte_vhost/vhost_user.c | 13 ++++++++++--- >> 1 file changed, 10 insertions(+), 3 deletions(-) >> > > otherwise: > > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > The title can be changed while applying if you agree with it, no need to > send v3. > > I'll take care of the different backports. > > Thanks, > Maxime >
On Thu, Nov 14, 2019 at 09:52:30AM +0100, Maxime Coquelin wrote: > Maybe change the title to something like: > > vhost: fix vring requests validation broken if no FD in payload > > On 11/13/19 12:03 PM, Zhike Wang wrote: > > When VHOST_USER_VRING_NOFD_MASK is set, the fd_num is 0, > > so validate_msg_fds() will return error. In this case, > > the negotiation of vring message between vhost user front end and > > back end would fail, and as a result, vhost user link could NOT be up. > > > > How to reproduce: > > 1.Run dpdk testpmd insides VM, which locates at host with ovs+dpdk. > > 2.Notice that inside ovs there are endless logs regarding failure to > > handle VHOST_USER_SET_VRING_CALL, and link of vm could NOT be up. > > > > Fixes: bf47225 ("vhost: fix possible denial of service by leaking FDs") > > Signed-off-by: Zhike Wang <wangzk320@163.com> > > --- > > lib/librte_vhost/vhost_user.c | 13 ++++++++++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > otherwise: > > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > The title can be changed while applying if you agree with it, no need to > send v3. > > I'll take care of the different backports. > Reviewed-by: Tiwei Bie <tiwei.bie@intel.com> Thanks! Tiwei
On Fri, Nov 15, 2019 at 5:14 AM Tiwei Bie <tiwei.bie@intel.com> wrote: > > On Thu, Nov 14, 2019 at 09:52:30AM +0100, Maxime Coquelin wrote: > > Maybe change the title to something like: > > > > vhost: fix vring requests validation broken if no FD in payload For the title, aligning to backport patches: "vhost: fix vring requests validation broken if no FD" > > > > On 11/13/19 12:03 PM, Zhike Wang wrote: > > > When VHOST_USER_VRING_NOFD_MASK is set, the fd_num is 0, > > > so validate_msg_fds() will return error. In this case, > > > the negotiation of vring message between vhost user front end and > > > back end would fail, and as a result, vhost user link could NOT be up. > > > > > > How to reproduce: > > > 1.Run dpdk testpmd insides VM, which locates at host with ovs+dpdk. > > > 2.Notice that inside ovs there are endless logs regarding failure to > > > handle VHOST_USER_SET_VRING_CALL, and link of vm could NOT be up. > > > > > > Fixes: bf47225 ("vhost: fix possible denial of service by leaking FDs") Fixes: bf472259dde6 ("vhost: fix possible denial of service by leaking FDs") Cc: stable@dpdk.org > > > Signed-off-by: Zhike Wang <wangzk320@163.com> > > > --- > > > lib/librte_vhost/vhost_user.c | 13 ++++++++++--- > > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > > > > otherwise: > > > > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > > > The title can be changed while applying if you agree with it, no need to > > send v3. > > > > I'll take care of the different backports. > > > > Reviewed-by: Tiwei Bie <tiwei.bie@intel.com> Applied, thanks.
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index 90ecee1..0cfb8b7 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c @@ -1563,8 +1563,10 @@ struct virtio_net *dev = *pdev; struct vhost_vring_file file; struct vhost_virtqueue *vq; + int expected_fds; - if (validate_msg_fds(msg, 1) != 0) + expected_fds = (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) ? 0 : 1; + if (validate_msg_fds(msg, expected_fds) != 0) return RTE_VHOST_MSG_RESULT_ERR; file.index = msg->payload.u64 & VHOST_USER_VRING_IDX_MASK; @@ -1588,7 +1590,10 @@ static int vhost_user_set_vring_err(struct virtio_net **pdev __rte_unused, struct VhostUserMsg *msg, int main_fd __rte_unused) { - if (validate_msg_fds(msg, 1) != 0) + int expected_fds; + + expected_fds = (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) ? 0 : 1; + if (validate_msg_fds(msg, expected_fds) != 0) return RTE_VHOST_MSG_RESULT_ERR; if (!(msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK)) @@ -1790,8 +1795,10 @@ static int vhost_user_set_vring_err(struct virtio_net **pdev __rte_unused, struct virtio_net *dev = *pdev; struct vhost_vring_file file; struct vhost_virtqueue *vq; + int expected_fds; - if (validate_msg_fds(msg, 1) != 0) + expected_fds = (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) ? 0 : 1; + if (validate_msg_fds(msg, expected_fds) != 0) return RTE_VHOST_MSG_RESULT_ERR; file.index = msg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
When VHOST_USER_VRING_NOFD_MASK is set, the fd_num is 0, so validate_msg_fds() will return error. In this case, the negotiation of vring message between vhost user front end and back end would fail, and as a result, vhost user link could NOT be up. How to reproduce: 1.Run dpdk testpmd insides VM, which locates at host with ovs+dpdk. 2.Notice that inside ovs there are endless logs regarding failure to handle VHOST_USER_SET_VRING_CALL, and link of vm could NOT be up. Fixes: bf47225 ("vhost: fix possible denial of service by leaking FDs") Signed-off-by: Zhike Wang <wangzk320@163.com> --- lib/librte_vhost/vhost_user.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)