vhost: break out of polling loop on error

When ring parsing fails, we currently handle this as ring empty condition. This means that we enable kicks and recheck ring empty: if this not empty, we re-start polling which of course will fail again. Instead, let's return a negative error code and stop polling. Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

vhost: break out of polling loop on error
When ring parsing fails, we currently handle this as ring empty condition. This means that we enable kicks and recheck ring empty: if this not empty, we re-start polling which of course will fail again. Instead, let's return a negative error code and stop polling. Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Michael S. Tsirkin
1 parent 38000a94a9
Showing 3 changed files with 31 additions and 22 deletions Side-by-side Diff
drivers/vhost/net.c
drivers/vhost/vhost.c
drivers/vhost/vhost.h
@@ -98,7 +98,8 @@
 static void handle_tx(struct vhost_net *net)
 {
 	struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX];
-	unsigned head, out, in, s;
+	unsigned out, in, s;
+	int head;
 	struct msghdr msg = {
 		.msg_name = NULL,
 		.msg_namelen = 0,
@@ -135,6 +136,9 @@
 					 ARRAY_SIZE(vq->iov),
 					 &out, &in,
 					 NULL, NULL);
+		/* On error, stop handling until the next kick. */
+		if (head < 0)
+			break;
 		/* Nothing new?  Wait for eventfd to tell us they refilled. */
 		if (head == vq->num) {
 			wmem = atomic_read(&sock->sk->sk_wmem_alloc);
@@ -192,7 +196,8 @@
 static void handle_rx(struct vhost_net *net)
 {
 	struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_RX];
-	unsigned head, out, in, log, s;
+	unsigned out, in, log, s;
+	int head;
 	struct vhost_log *vq_log;
 	struct msghdr msg = {
 		.msg_name = NULL,
@@ -228,6 +233,9 @@
 					 ARRAY_SIZE(vq->iov),
 					 &out, &in,
 					 vq_log, &log);
+		/* On error, stop handling until the next kick. */
+		if (head < 0)
+			break;
 		/* OK, now we need to know about added descriptors. */
 		if (head == vq->num) {
 			if (unlikely(vhost_enable_notify(vq))) {
@@ -873,12 +873,13 @@
  * number of output then some number of input descriptors, it's actually two
  * iovecs, but we pack them into one and note how many of each there were.
  *
- * This function returns the descriptor number found, or vq->num (which
- * is never a valid descriptor number) if none was found. */
-unsigned vhost_get_vq_desc(struct vhost_dev *dev, struct vhost_virtqueue *vq,
-			   struct iovec iov[], unsigned int iov_size,
-			   unsigned int *out_num, unsigned int *in_num,
-			   struct vhost_log *log, unsigned int *log_num)
+ * This function returns the descriptor number found, or vq->num (which is
+ * never a valid descriptor number) if none was found.  A negative code is
+ * returned on error. */
+int vhost_get_vq_desc(struct vhost_dev *dev, struct vhost_virtqueue *vq,
+		      struct iovec iov[], unsigned int iov_size,
+		      unsigned int *out_num, unsigned int *in_num,
+		      struct vhost_log *log, unsigned int *log_num)
 {
 	struct vring_desc desc;
 	unsigned int i, head, found = 0;
  
@@ -890,13 +891,13 @@
 	if (get_user(vq->avail_idx, &vq->avail->idx)) {
 		vq_err(vq, "Failed to access avail idx at %p\n",
 		       &vq->avail->idx);
-		return vq->num;
+		return -EFAULT;
 	}
  
 	if ((u16)(vq->avail_idx - last_avail_idx) > vq->num) {
 		vq_err(vq, "Guest moved used index from %u to %u",
 		       last_avail_idx, vq->avail_idx);
-		return vq->num;
+		return -EFAULT;
 	}
  
 	/* If there's nothing new since last we looked, return invalid. */
  
@@ -912,14 +913,14 @@
 		vq_err(vq, "Failed to read head: idx %d address %p\n",
 		       last_avail_idx,
 		       &vq->avail->ring[last_avail_idx % vq->num]);
-		return vq->num;
+		return -EFAULT;
 	}
  
 	/* If their number is silly, that's an error. */
 	if (head >= vq->num) {
 		vq_err(vq, "Guest says index %u > %u is available",
 		       head, vq->num);
-		return vq->num;
+		return -EINVAL;
 	}
  
 	/* When we start there are none of either input nor output. */
  
  
@@ -933,19 +934,19 @@
 		if (i >= vq->num) {
 			vq_err(vq, "Desc index is %u > %u, head = %u",
 			       i, vq->num, head);
-			return vq->num;
+			return -EINVAL;
 		}
 		if (++found > vq->num) {
 			vq_err(vq, "Loop detected: last one at %u "
 			       "vq size %u head %u\n",
 			       i, vq->num, head);
-			return vq->num;
+			return -EINVAL;
 		}
 		ret = copy_from_user(&desc, vq->desc + i, sizeof desc);
 		if (ret) {
 			vq_err(vq, "Failed to get descriptor: idx %d addr %p\n",
 			       i, vq->desc + i);
-			return vq->num;
+			return -EFAULT;
 		}
 		if (desc.flags & VRING_DESC_F_INDIRECT) {
 			ret = get_indirect(dev, vq, iov, iov_size,
@@ -954,7 +955,7 @@
 			if (ret < 0) {
 				vq_err(vq, "Failure detected "
 				       "in indirect descriptor at idx %d\n", i);
-				return vq->num;
+				return ret;
 			}
 			continue;
 		}
@@ -964,7 +965,7 @@
 		if (ret < 0) {
 			vq_err(vq, "Translation failure %d descriptor idx %d\n",
 			       ret, i);
-			return vq->num;
+			return ret;
 		}
 		if (desc.flags & VRING_DESC_F_WRITE) {
 			/* If this is an input descriptor,
@@ -981,7 +982,7 @@
 			if (*in_num) {
 				vq_err(vq, "Descriptor has out after in: "
 				       "idx %d\n", i);
-				return vq->num;
+				return -EINVAL;
 			}
 			*out_num += ret;
 		}
@@ -120,10 +120,10 @@
 int vhost_vq_access_ok(struct vhost_virtqueue *vq);
 int vhost_log_access_ok(struct vhost_dev *);
  
-unsigned vhost_get_vq_desc(struct vhost_dev *, struct vhost_virtqueue *,
-			   struct iovec iov[], unsigned int iov_count,
-			   unsigned int *out_num, unsigned int *in_num,
-			   struct vhost_log *log, unsigned int *log_num);
+int vhost_get_vq_desc(struct vhost_dev *, struct vhost_virtqueue *,
+		      struct iovec iov[], unsigned int iov_count,
+		      unsigned int *out_num, unsigned int *in_num,
+		      struct vhost_log *log, unsigned int *log_num);
 void vhost_discard_vq_desc(struct vhost_virtqueue *);
  
 int vhost_add_used(struct vhost_virtqueue *, unsigned int head, int len);
...	...	@@ -98,7 +98,8 @@
98	98	static void handle_tx(struct vhost_net *net)
99	99	{
100	100	struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX];
101		- unsigned head, out, in, s;
	101	+ unsigned out, in, s;
	102	+ int head;
102	103	struct msghdr msg = {
103	104	.msg_name = NULL,
104	105	.msg_namelen = 0,
...	...	@@ -135,6 +136,9 @@
135	136	ARRAY_SIZE(vq->iov),
136	137	&out, &in,
137	138	NULL, NULL);
	139	+ /* On error, stop handling until the next kick. */
	140	+ if (head < 0)
	141	+ break;
138	142	/* Nothing new? Wait for eventfd to tell us they refilled. */
139	143	if (head == vq->num) {
140	144	wmem = atomic_read(&sock->sk->sk_wmem_alloc);
...	...	@@ -192,7 +196,8 @@
192	196	static void handle_rx(struct vhost_net *net)
193	197	{
194	198	struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_RX];
195		- unsigned head, out, in, log, s;
	199	+ unsigned out, in, log, s;
	200	+ int head;
196	201	struct vhost_log *vq_log;
197	202	struct msghdr msg = {
198	203	.msg_name = NULL,
...	...	@@ -228,6 +233,9 @@
228	233	ARRAY_SIZE(vq->iov),
229	234	&out, &in,
230	235	vq_log, &log);
	236	+ /* On error, stop handling until the next kick. */
	237	+ if (head < 0)
	238	+ break;
231	239	/* OK, now we need to know about added descriptors. */
232	240	if (head == vq->num) {
233	241	if (unlikely(vhost_enable_notify(vq))) {
...	...	@@ -873,12 +873,13 @@
873	873	* number of output then some number of input descriptors, it's actually two
874	874	* iovecs, but we pack them into one and note how many of each there were.
875	875	*
876		- * This function returns the descriptor number found, or vq->num (which
877		- * is never a valid descriptor number) if none was found. */
878		-unsigned vhost_get_vq_desc(struct vhost_dev dev, struct vhost_virtqueue vq,
879		- struct iovec iov[], unsigned int iov_size,
880		- unsigned int out_num, unsigned int in_num,
881		- struct vhost_log log, unsigned int log_num)
	876	+ * This function returns the descriptor number found, or vq->num (which is
	877	+ * never a valid descriptor number) if none was found. A negative code is
	878	+ * returned on error. */
	879	+int vhost_get_vq_desc(struct vhost_dev dev, struct vhost_virtqueue vq,
	880	+ struct iovec iov[], unsigned int iov_size,
	881	+ unsigned int out_num, unsigned int in_num,
	882	+ struct vhost_log log, unsigned int log_num)
882	883	{
883	884	struct vring_desc desc;
884	885	unsigned int i, head, found = 0;
885	886
...	...	@@ -890,13 +891,13 @@
890	891	if (get_user(vq->avail_idx, &vq->avail->idx)) {
891	892	vq_err(vq, "Failed to access avail idx at %p\n",
892	893	&vq->avail->idx);
893		- return vq->num;
	894	+ return -EFAULT;
894	895	}
895	896
896	897	if ((u16)(vq->avail_idx - last_avail_idx) > vq->num) {
897	898	vq_err(vq, "Guest moved used index from %u to %u",
898	899	last_avail_idx, vq->avail_idx);
899		- return vq->num;
	900	+ return -EFAULT;
900	901	}
901	902
902	903	/* If there's nothing new since last we looked, return invalid. */
903	904
...	...	@@ -912,14 +913,14 @@
912	913	vq_err(vq, "Failed to read head: idx %d address %p\n",
913	914	last_avail_idx,
914	915	&vq->avail->ring[last_avail_idx % vq->num]);
915		- return vq->num;
	916	+ return -EFAULT;
916	917	}
917	918
918	919	/* If their number is silly, that's an error. */
919	920	if (head >= vq->num) {
920	921	vq_err(vq, "Guest says index %u > %u is available",
921	922	head, vq->num);
922		- return vq->num;
	923	+ return -EINVAL;
923	924	}
924	925
925	926	/* When we start there are none of either input nor output. */
926	927
927	928
...	...	@@ -933,19 +934,19 @@
933	934	if (i >= vq->num) {
934	935	vq_err(vq, "Desc index is %u > %u, head = %u",
935	936	i, vq->num, head);
936		- return vq->num;
	937	+ return -EINVAL;
937	938	}
938	939	if (++found > vq->num) {
939	940	vq_err(vq, "Loop detected: last one at %u "
940	941	"vq size %u head %u\n",
941	942	i, vq->num, head);
942		- return vq->num;
	943	+ return -EINVAL;
943	944	}
944	945	ret = copy_from_user(&desc, vq->desc + i, sizeof desc);
945	946	if (ret) {
946	947	vq_err(vq, "Failed to get descriptor: idx %d addr %p\n",
947	948	i, vq->desc + i);
948		- return vq->num;
	949	+ return -EFAULT;
949	950	}
950	951	if (desc.flags & VRING_DESC_F_INDIRECT) {
951	952	ret = get_indirect(dev, vq, iov, iov_size,
...	...	@@ -954,7 +955,7 @@
954	955	if (ret < 0) {
955	956	vq_err(vq, "Failure detected "
956	957	"in indirect descriptor at idx %d\n", i);
957		- return vq->num;
	958	+ return ret;
958	959	}
959	960	continue;
960	961	}
...	...	@@ -964,7 +965,7 @@
964	965	if (ret < 0) {
965	966	vq_err(vq, "Translation failure %d descriptor idx %d\n",
966	967	ret, i);
967		- return vq->num;
	968	+ return ret;
968	969	}
969	970	if (desc.flags & VRING_DESC_F_WRITE) {
970	971	/* If this is an input descriptor,
...	...	@@ -981,7 +982,7 @@
981	982	if (*in_num) {
982	983	vq_err(vq, "Descriptor has out after in: "
983	984	"idx %d\n", i);
984		- return vq->num;
	985	+ return -EINVAL;
985	986	}
986	987	*out_num += ret;
987	988	}
...	...	@@ -120,10 +120,10 @@
120	120	int vhost_vq_access_ok(struct vhost_virtqueue *vq);
121	121	int vhost_log_access_ok(struct vhost_dev *);
122	122
123		-unsigned vhost_get_vq_desc(struct vhost_dev , struct vhost_virtqueue ,
124		- struct iovec iov[], unsigned int iov_count,
125		- unsigned int out_num, unsigned int in_num,
126		- struct vhost_log log, unsigned int log_num);
	123	+int vhost_get_vq_desc(struct vhost_dev , struct vhost_virtqueue ,
	124	+ struct iovec iov[], unsigned int iov_count,
	125	+ unsigned int out_num, unsigned int in_num,
	126	+ struct vhost_log log, unsigned int log_num);
127	127	void vhost_discard_vq_desc(struct vhost_virtqueue *);
128	128
129	129	int vhost_add_used(struct vhost_virtqueue *, unsigned int head, int len);