* [dpdk-dev] [PATCH v2] vhost: fix vring message handling broken in some case @ 2019-11-13 11:03 Zhike Wang 2019-11-14 8:52 ` Maxime Coquelin 0 siblings, 1 reply; 7+ messages in thread From: Zhike Wang @ 2019-11-13 11:03 UTC (permalink / raw) To: dev, stable, security; +Cc: wangzhike, Zhike Wang 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(-) 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; -- 1.8.3.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: fix vring message handling broken in some case 2019-11-13 11:03 [dpdk-dev] [PATCH v2] vhost: fix vring message handling broken in some case Zhike Wang @ 2019-11-14 8:52 ` Maxime Coquelin 2019-11-14 10:16 ` 王志克 2019-11-15 4:15 ` [dpdk-dev] [dpdk-stable] " Tiwei Bie 0 siblings, 2 replies; 7+ messages in thread From: Maxime Coquelin @ 2019-11-14 8:52 UTC (permalink / raw) To: Zhike Wang, dev, stable, security; +Cc: wangzhike 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: fix vring message handling broken in some case 2019-11-14 8:52 ` Maxime Coquelin @ 2019-11-14 10:16 ` 王志克 2019-11-14 10:23 ` Maxime Coquelin 2019-11-15 4:15 ` [dpdk-dev] [dpdk-stable] " Tiwei Bie 1 sibling, 1 reply; 7+ messages in thread From: 王志克 @ 2019-11-14 10:16 UTC (permalink / raw) To: Maxime Coquelin, Zhike Wang, dev, stable, security 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: fix vring message handling broken in some case 2019-11-14 10:16 ` 王志克 @ 2019-11-14 10:23 ` Maxime Coquelin 2019-11-14 10:58 ` 王志克 0 siblings, 1 reply; 7+ messages in thread From: Maxime Coquelin @ 2019-11-14 10:23 UTC (permalink / raw) To: 王志克, Zhike Wang, dev, stable, security 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: fix vring message handling broken in some case 2019-11-14 10:23 ` Maxime Coquelin @ 2019-11-14 10:58 ` 王志克 0 siblings, 0 replies; 7+ messages in thread From: 王志克 @ 2019-11-14 10:58 UTC (permalink / raw) To: Maxime Coquelin, Zhike Wang, dev, stable, security 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH v2] vhost: fix vring message handling broken in some case 2019-11-14 8:52 ` Maxime Coquelin 2019-11-14 10:16 ` 王志克 @ 2019-11-15 4:15 ` Tiwei Bie 2019-11-15 13:31 ` David Marchand 1 sibling, 1 reply; 7+ messages in thread From: Tiwei Bie @ 2019-11-15 4:15 UTC (permalink / raw) To: Maxime Coquelin; +Cc: Zhike Wang, dev, stable, security, wangzhike 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH v2] vhost: fix vring message handling broken in some case 2019-11-15 4:15 ` [dpdk-dev] [dpdk-stable] " Tiwei Bie @ 2019-11-15 13:31 ` David Marchand 0 siblings, 0 replies; 7+ messages in thread From: David Marchand @ 2019-11-15 13:31 UTC (permalink / raw) To: Zhike Wang Cc: Maxime Coquelin, dev, dpdk stable, security, wangzhike, Tiwei Bie 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. -- David Marchand ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-11-15 13:31 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-13 11:03 [dpdk-dev] [PATCH v2] vhost: fix vring message handling broken in some case Zhike Wang 2019-11-14 8:52 ` Maxime Coquelin 2019-11-14 10:16 ` 王志克 2019-11-14 10:23 ` Maxime Coquelin 2019-11-14 10:58 ` 王志克 2019-11-15 4:15 ` [dpdk-dev] [dpdk-stable] " Tiwei Bie 2019-11-15 13:31 ` David Marchand
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).