[dpdk-stable] patch 'vhost-user: drop connection on message handling failures' has been queued to stable release 18.08.1

[Kevin Traynor]

Hi,

FYI, your patch has been queued to stable release 18.08.1

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 11/23/18. So please
shout if anyone has objections.

Also note that after the patch there's a diff of the upstream commit vs the patch applied
to the branch. If the code is different (ie: not only metadata diffs), due for example to
a change in context or macro names, please double check it.

Thanks.

Kevin Traynor

---
>From f9894ad3409367bd479a787c5f21d34009caa59a Mon Sep 17 00:00:00 2001
From: Ilya Maximets <i.maximets at samsung.com>
Date: Mon, 3 Sep 2018 13:12:24 +0300
Subject: [PATCH] vhost-user: drop connection on message handling failures

[ upstream commit 0d7853a4da3bd681005ecb64ef1183c59356eeea ]

There are a lot of cases where vhost-user massage handling
could fail and end up in a fully not recoverable state. For
example, allocation failures of shadow used ring and batched
copy array are not recoverable and leads to the segmentation
faults like this on the receiving/transmission path:

  Program received signal SIGSEGV, Segmentation fault.
  [Switching to Thread 0x7f913fecf0 (LWP 43625)]
  in copy_desc_to_mbuf () at /lib/librte_vhost/virtio_net.c:760
  760       batch_copy[vq->batch_copy_nb_elems].dst =

This could be easily reproduced in case of low memory or big
number of vhost-user ports.

Fix that by propagating error to the upper layer which will
end up with disconnection in case we can not report to
the message sender when the error happens.

Fixes: f689586bc060 ("vhost: shadow used ring update")

Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin at redhat.com>
---
 lib/librte_vhost/vhost_user.c | 51 +++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 20 deletions(-)

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
@@ -1015,5 +1015,5 @@ vhost_user_set_vring_call(struct virtio_net *dev, struct VhostUserMsg *pmsg)
 }
 
-static void
+static int
 vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *pmsg)
 {
@@ -1033,5 +1033,5 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *pmsg)
 	dev = translate_ring_addresses(dev, file.index);
 	if (!dev)
-		return;
+		return -1;
 
 	*pdev = dev;
@@ -1050,4 +1050,5 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *pmsg)
 		close(vq->kickfd);
 	vq->kickfd = file.fd;
+	return 0;
 }
 
@@ -1173,12 +1174,17 @@ vhost_user_get_protocol_features(struct virtio_net *dev,
 }
 
-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;
 }
 
@@ -1658,6 +1664,4 @@ vhost_user_msg_handler(int vid, int fd)
 	case VHOST_USER_SET_FEATURES:
 		ret = vhost_user_set_features(dev, msg.payload.u64);
-		if (ret)
-			return -1;
 		break;
 
@@ -1667,12 +1671,12 @@ vhost_user_msg_handler(int vid, int fd)
 		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;
 
@@ -1682,6 +1686,7 @@ vhost_user_msg_handler(int vid, int fd)
 
 	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);
@@ -1694,15 +1699,17 @@ vhost_user_msg_handler(int vid, int fd)
 
 	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);
@@ -1710,5 +1717,5 @@ vhost_user_msg_handler(int vid, int fd)
 
 	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:
@@ -1729,8 +1736,8 @@ vhost_user_msg_handler(int vid, int fd)
 
 	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;
 
@@ -1753,5 +1760,5 @@ 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;
 
@@ -1773,4 +1780,8 @@ skip_to_reply:
 		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;
 	}
 
-- 
2.19.0

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2018-11-20 17:53:08.187279182 +0000
+++ 0030-vhost-user-drop-connection-on-message-handling-failu.patch	2018-11-20 17:53:07.000000000 +0000
@@ -1,8 +1,10 @@
-From 0d7853a4da3bd681005ecb64ef1183c59356eeea Mon Sep 17 00:00:00 2001
+From f9894ad3409367bd479a787c5f21d34009caa59a Mon Sep 17 00:00:00 2001
 From: Ilya Maximets <i.maximets at samsung.com>
 Date: Mon, 3 Sep 2018 13:12:24 +0300
 Subject: [PATCH] vhost-user: drop connection on message handling failures
 
+[ upstream commit 0d7853a4da3bd681005ecb64ef1183c59356eeea ]
+
 There are a lot of cases where vhost-user massage handling
 could fail and end up in a fully not recoverable state. For
 example, allocation failures of shadow used ring and batched
@@ -22,7 +24,6 @@
 the message sender when the error happens.
 
 Fixes: f689586bc060 ("vhost: shadow used ring update")
-Cc: stable at dpdk.org
 
 Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
 Reviewed-by: Maxime Coquelin <maxime.coquelin at redhat.com>