From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id C48FC4C88 for ; Thu, 30 Aug 2018 08:53:33 +0200 (CEST) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20180830065332euoutp020c498beac31d53d1b72237582fdccde0~Plv1CCRRV1023110231euoutp02b for ; Thu, 30 Aug 2018 06:53:32 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20180830065332euoutp020c498beac31d53d1b72237582fdccde0~Plv1CCRRV1023110231euoutp02b DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1535612012; bh=peWC1L0N4/OtuAyGDIscch4LAhsrXh4v9KzFsOycIgE=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=j9kFv4w796hK0vdoK7t3IdTJPqWzxq4ucc/anpS/YbGYtTHvh8MdZFy1X/lVZrdxi XClOpaJVI6txzK/D10mu/Jt52fsSaU0h1T6ApdMKpV6nbSmUKAUr9YPv8wBmtRCZqb 9t6m5hCRKzazWuQx/Gbbj1hrUsf5Co91FbXU7JHw= Received: from eusmges2new.samsung.com (unknown [203.254.199.244]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20180830065332eucas1p1539e30b4808b1d0876665e873f3b400d~Plv0o5U-I2569325693eucas1p1_; Thu, 30 Aug 2018 06:53:32 +0000 (GMT) Received: from eucas1p2.samsung.com ( [182.198.249.207]) by eusmges2new.samsung.com (EUCPMTA) with SMTP id 6D.91.04294.B64978B5; Thu, 30 Aug 2018 07:53:31 +0100 (BST) Received: from eusmtrp1.samsung.com (unknown [182.198.249.138]) by eucas1p2.samsung.com (KnoxPortal) with ESMTPA id 20180830065331eucas1p23467f23f8363577a842bc891c1b5f3e2~Plvz4erV-1928619286eucas1p2F; Thu, 30 Aug 2018 06:53:31 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp1.samsung.com (KnoxPortal) with ESMTP id 20180830065330eusmtrp19030be6ea00c772138edec0ad9198303~PlvzmHLB60557605576eusmtrp1F; Thu, 30 Aug 2018 06:53:30 +0000 (GMT) X-AuditID: cbfec7f4-835ff700000010c6-f2-5b87946b91dc Received: from eusmtip1.samsung.com ( [203.254.199.221]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id 58.B9.04128.A64978B5; Thu, 30 Aug 2018 07:53:30 +0100 (BST) Received: from [106.109.129.180] (unknown [106.109.129.180]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20180830065330eusmtip14f61bb3f5a9cea5b8e657ba1f68e0ae1~PlvzMVjQa1906019060eusmtip1S; Thu, 30 Aug 2018 06:53:30 +0000 (GMT) To: Tiwei Bie Cc: dev@dpdk.org, Maxime Coquelin , Zhihong Wang , stable@dpdk.org From: Ilya Maximets Date: Thu, 30 Aug 2018 09:54:50 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20180830060642.GA7890@fbsd.sh.intel.com> Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprPKsWRmVeSWpSXmKPExsWy7djP87rZU9qjDe7P1rZ492k7k8WV9p/s Fsc697BY/Ov4w26xteE/k8Xmi5OYHNg8fi1YyuqxeM9LJo/3+66yefRtWcUYwBLFZZOSmpNZ llqkb5fAlbHz7D72gkU6FfOmP2RtYFyv0sXIySEhYCLxduIEpi5GLg4hgRWMEs19u1lBEkIC Xxgl3n3WhEh8ZpQ4t2ciexcjB1jHkrv+EPHljBI7771ih3A+Mkrs+3mCGaRbWMBNYv21Z2wg toiAssS3MzNZQZqZBSokLuwxBQmzCehInFp9hBHEZhFQlfi1ppMFxBYViJA48mAhWJxXQFDi 5MwnYHFOATOJi5dfMoHYzALiEk1fVrJC2PIS29/OYQa5QUJgGbvE5Z1noZrLJC62HmSDeNNF Ysfj3ywQtrDEq+Nb2CFsGYnTk3ug4vUS91teMkIM6mCUmH7oHxNEwl5iy+tz7BAPaEqs36UP EXaU+N+1mQUSKHwSN94KQtzDJzFp23RmiDCvREebEES1isTvg8uZIWwpiZvvPrNPYFSaheTL WUg+m4Xks1kIexcwsqxiFE8tLc5NTy02ykst1ytOzC0uzUvXS87P3cQITDan/x3/soNx15+k Q4wCHIxKPLwXZrRFC7EmlhVX5h5ilOBgVhLh5ToHFOJNSaysSi3Kjy8qzUktPsQozcGiJM7L p5UWLSSQnliSmp2aWpBaBJNl4uCUamD0tTD5q+PydJGMnttPwRP3gi/+ctZ/NePlOyWuwJnq Qm5R19NP9q+wYjze1vxL5tTCm6wyvuZTG+2EdvLoXL3F/bTpQ92p0hdLJ518OPvRzrlqef3C Fa/nOUxvjzok+jM25b5M7TxlvZ2xW+zcQ+8YzbkbMo1jRnKMymfBEo/3MhvnBKbrZDEpsRRn JBpqMRcVJwIAeFFm+DIDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrMIsWRmVeSWpSXmKPExsVy+t/xu7pZU9qjDb5ONbF492k7k8WV9p/s Fsc697BY/Ov4w26xteE/k8Xmi5OYHNg8fi1YyuqxeM9LJo/3+66yefRtWcUYwBKlZ1OUX1qS qpCRX1xiqxRtaGGkZ2hpoWdkYqlnaGwea2VkqqRvZ5OSmpNZllqkb5egl7Hz7D72gkU6FfOm P2RtYFyv0sXIwSEhYCKx5K5/FyMXh5DAUkaJmy+vsHcxcgLFpSR+/LrACmELS/y51sUGYgsJ vGeUOLlVA8QWFnCTWH/tGVhcREBZ4tuZmWD1zAIVEkt3vWGDGLqCSWLDkruMIAk2AR2JU6uP gNm8AnYS63YsYgKxWQRUJX6t6WQBsUUFIiRWL3/BClEjKHFy5hOwOKeAmcTFyy+ZIBaoS/yZ d4kZwhaXaPqyEmqxvMT2t3OYJzAKzULSPgtJyywkLbOQtCxgZFnFKJJaWpybnltspFecmFtc mpeul5yfu4kRGGHbjv3csoOx613wIUYBDkYlHl6DRW3RQqyJZcWVuYcYJTiYlUR4uc4BhXhT EiurUovy44tKc1KLDzGaAj03kVlKNDkfGP15JfGGpobmFpaG5sbmxmYWSuK85w0qo4QE0hNL UrNTUwtSi2D6mDg4pRoYVTKuL+myvlqvfZb7xD2v17JuV/6K91zomFD/jSP2Y1m1LyfLxTCe g6nHWHa9L9xtfvmhGMOeeaytp1J+36o0+r22tvmiPqvijqWrf00zsem+dCVa/8W8Axz6i3L4 uux6Th6zaJ6i/jjTJlNl4WXPPqnFVnv+V1g8FrLgDF7et6/+UIPX7JQ3SizFGYmGWsxFxYkA xQ8CzsYCAAA= Message-Id: <20180830065331eucas1p23467f23f8363577a842bc891c1b5f3e2~Plvz4erV-1928619286eucas1p2F@eucas1p2.samsung.com> X-CMS-MailID: 20180830065331eucas1p23467f23f8363577a842bc891c1b5f3e2 X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20180817113255eucas1p2042d22241265dafa241c2a6a2c949244 X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20180817113255eucas1p2042d22241265dafa241c2a6a2c949244 References: <20180817113354.16140-1-i.maximets@samsung.com> <20180829135924eucas1p284d5711018eeba8b0ec3aaa6c84cef10~PX6Xgs7eA2716827168eucas1p22@eucas1p2.samsung.com> <20180830060642.GA7890@fbsd.sh.intel.com> Subject: Re: [dpdk-dev] [PATCH] vhost: fix crash if set vring num handling failed X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Aug 2018 06:53:33 -0000 On 30.08.2018 09:06, Tiwei Bie wrote: > On Wed, Aug 29, 2018 at 05:00:47PM +0300, Ilya Maximets wrote: >> Any thoughts on this? > > Hi Ilya, > > Currently, the errors happened during messages handling > aren't handled properly. E.g. in vhost_user_set_mem_table(), > similar issues [1] may happen too. We might want to find > a way to fix this once and for all. > > [1] > https://github.com/DPDK/dpdk/blob/3605968c2fa7/lib/librte_vhost/vhost_user.c#L826 > https://github.com/DPDK/dpdk/blob/3605968c2fa7/lib/librte_vhost/vhost_user.c#L837 > https://github.com/DPDK/dpdk/blob/3605968c2fa7/lib/librte_vhost/vhost_user.c#L887 > > Thanks Hi. I understand your position position and, actually, I looked at other functions while preparing this fix. There is one difference between vhost_user_set_mem_table() and vhost_user_set_vring_num(). The first one is able to reply bad status. But yes, you're right that we should handle all the errors. What about following patch (compile tested only): ---- diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index 9aa1ce118..63d145b2d 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c @@ -1014,7 +1014,7 @@ vhost_user_set_vring_call(struct virtio_net *dev, struct VhostUserMsg *pmsg) vq->callfd = file.fd; } -static void +static int vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *pmsg) { struct vhost_vring_file file; @@ -1032,7 +1032,7 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *pmsg) /* Interpret ring addresses only when ring is started. */ dev = translate_ring_addresses(dev, file.index); if (!dev) - return; + return -1; *pdev = dev; @@ -1049,6 +1049,7 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *pmsg) if (vq->kickfd >= 0) close(vq->kickfd); vq->kickfd = file.fd; + return 0; } static void @@ -1172,14 +1173,19 @@ vhost_user_get_protocol_features(struct virtio_net *dev, msg->size = sizeof(msg->payload.u64); } -static void +static int vhost_user_set_protocol_features(struct virtio_net *dev, uint64_t protocol_features) { - if (protocol_features & ~VHOST_USER_PROTOCOL_FEATURES) - return; + if (protocol_features & ~VHOST_USER_PROTOCOL_FEATURES) { + RTE_LOG(ERR, VHOST_CONFIG, + "(%d) received invalid protocol features.\n", + dev->vid); + return -1; + } dev->protocol_features = protocol_features; + return 0; } static int @@ -1657,8 +1663,6 @@ vhost_user_msg_handler(int vid, int fd) break; case VHOST_USER_SET_FEATURES: ret = vhost_user_set_features(dev, msg.payload.u64); - if (ret) - return -1; break; case VHOST_USER_GET_PROTOCOL_FEATURES: @@ -1666,14 +1670,14 @@ vhost_user_msg_handler(int vid, int fd) send_vhost_reply(fd, &msg); break; case VHOST_USER_SET_PROTOCOL_FEATURES: - vhost_user_set_protocol_features(dev, msg.payload.u64); + ret = vhost_user_set_protocol_features(dev, msg.payload.u64); break; case VHOST_USER_SET_OWNER: - vhost_user_set_owner(); + ret = vhost_user_set_owner(); break; case VHOST_USER_RESET_OWNER: - vhost_user_reset_owner(dev); + ret = vhost_user_reset_owner(dev); break; case VHOST_USER_SET_MEM_TABLE: @@ -1681,8 +1685,9 @@ vhost_user_msg_handler(int vid, int fd) break; case VHOST_USER_SET_LOG_BASE: - vhost_user_set_log_base(dev, &msg); - + ret = vhost_user_set_log_base(dev, &msg); + if (ret) + goto skip_to_reply; /* it needs a reply */ msg.size = sizeof(msg.payload.u64); send_vhost_reply(fd, &msg); @@ -1693,23 +1698,25 @@ vhost_user_msg_handler(int vid, int fd) break; case VHOST_USER_SET_VRING_NUM: - vhost_user_set_vring_num(dev, &msg); + ret = vhost_user_set_vring_num(dev, &msg); break; case VHOST_USER_SET_VRING_ADDR: - vhost_user_set_vring_addr(&dev, &msg); + ret = vhost_user_set_vring_addr(&dev, &msg); break; case VHOST_USER_SET_VRING_BASE: - vhost_user_set_vring_base(dev, &msg); + ret = vhost_user_set_vring_base(dev, &msg); break; case VHOST_USER_GET_VRING_BASE: - vhost_user_get_vring_base(dev, &msg); + ret = vhost_user_get_vring_base(dev, &msg); + if (ret) + goto skip_to_reply; msg.size = sizeof(msg.payload.state); send_vhost_reply(fd, &msg); break; case VHOST_USER_SET_VRING_KICK: - vhost_user_set_vring_kick(&dev, &msg); + ret = vhost_user_set_vring_kick(&dev, &msg); break; case VHOST_USER_SET_VRING_CALL: vhost_user_set_vring_call(dev, &msg); @@ -1728,10 +1735,10 @@ vhost_user_msg_handler(int vid, int fd) break; case VHOST_USER_SET_VRING_ENABLE: - vhost_user_set_vring_enable(dev, &msg); + ret = vhost_user_set_vring_enable(dev, &msg); break; case VHOST_USER_SEND_RARP: - vhost_user_send_rarp(dev, &msg); + ret = vhost_user_send_rarp(dev, &msg); break; case VHOST_USER_NET_SET_MTU: @@ -1752,7 +1759,7 @@ vhost_user_msg_handler(int vid, int fd) } skip_to_post_handle: - if (dev->extern_ops.post_msg_handle) { + if (!ret && dev->extern_ops.post_msg_handle) { uint32_t need_reply; ret = (*dev->extern_ops.post_msg_handle)( @@ -1772,6 +1779,10 @@ vhost_user_msg_handler(int vid, int fd) msg.payload.u64 = !!ret; msg.size = sizeof(msg.payload.u64); send_vhost_reply(fd, &msg); + } else if (ret) { + RTE_LOG(ERR, VHOST_CONFIG, + "vhost message handling failed.\n"); + return -1; } if (!(dev->flags & VIRTIO_DEV_RUNNING) && virtio_is_ready(dev)) { ---- Best regards, Ilya Maximets.