[PATCH] kvm tools: plug race between uip_init and virtio_net_rx_thread

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



When fa7226f (kvm tools: init network devices only when the virtio
driver is ready to go) was introduced, a tiny detail was overlooked:

- Initialization of the uip layer is now coming in very late (only
  when the guest driver says it is ready).
- In parallel, the rx thread is created quite early (as soon as the
  queues are allocated).

This cause the rx thread to call uip_rx, which calls uip_buf_get_used,
which starts to use buf_lock mutex/the buf_used_cond, which haven't
been initialized yet. Tears and devastation follow, not to mention a
certain lack of network connectivity for the unsuspecting guest.

The (not so pretty) fix is to split uip_init:
- uip_static_init: initialize the lists, mutexes and conditions,
  called from virtio_net__init_one.
- uip_init: perform the dynamic memory allocations, called from
  notify_status.

This allows the network to be safely initialized.

Cc: Sasha Levin <sasha.levin@xxxxxxxxxx>
Cc: Pekka Enberg <penberg@xxxxxxxxxx>
Cc: Will Deacon <will.deacon@xxxxxxx>
Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
---
 tools/kvm/include/kvm/uip.h |  1 +
 tools/kvm/net/uip/core.c    | 19 +++++++++++++------
 tools/kvm/virtio/net.c      |  1 +
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/tools/kvm/include/kvm/uip.h b/tools/kvm/include/kvm/uip.h
index 4e63808..a9a8d85 100644
--- a/tools/kvm/include/kvm/uip.h
+++ b/tools/kvm/include/kvm/uip.h
@@ -334,6 +334,7 @@ static inline u16 uip_eth_hdrlen(struct uip_eth *eth)
 
 int uip_tx(struct iovec *iov, u16 out, struct uip_info *info);
 int uip_rx(struct iovec *iov, u16 in, struct uip_info *info);
+void uip_static_init(struct uip_info *info);
 int uip_init(struct uip_info *info);
 
 int uip_tx_do_ipv4_udp_dhcp(struct uip_tx_arg *arg);
diff --git a/tools/kvm/net/uip/core.c b/tools/kvm/net/uip/core.c
index 789b814..b3cd8c2 100644
--- a/tools/kvm/net/uip/core.c
+++ b/tools/kvm/net/uip/core.c
@@ -94,19 +94,15 @@ int uip_rx(struct iovec *iov, u16 in, struct uip_info *info)
 	return len;
 }
 
-int uip_init(struct uip_info *info)
+void uip_static_init(struct uip_info *info)
 {
 	struct list_head *udp_socket_head;
 	struct list_head *tcp_socket_head;
 	struct list_head *buf_head;
-	struct uip_buf *buf;
-	int buf_nr;
-	int i;
 
 	udp_socket_head	= &info->udp_socket_head;
 	tcp_socket_head	= &info->tcp_socket_head;
 	buf_head	= &info->buf_head;
-	buf_nr		= info->buf_nr;
 
 	INIT_LIST_HEAD(udp_socket_head);
 	INIT_LIST_HEAD(tcp_socket_head);
@@ -119,6 +115,18 @@ int uip_init(struct uip_info *info)
 	pthread_cond_init(&info->buf_used_cond, NULL);
 	pthread_cond_init(&info->buf_free_cond, NULL);
 
+	info->buf_used_nr = 0;
+}
+
+int uip_init(struct uip_info *info)
+{
+	struct list_head *buf_head;
+	struct uip_buf *buf;
+	int buf_nr;
+	int i;
+
+	buf_head	= &info->buf_head;
+	buf_nr		= info->buf_nr;
 
 	for (i = 0; i < buf_nr; i++) {
 		buf = malloc(sizeof(*buf));
@@ -141,7 +149,6 @@ int uip_init(struct uip_info *info)
 	}
 
 	info->buf_free_nr = buf_nr;
-	info->buf_used_nr = 0;
 
 	uip_dhcp_get_dns(info);
 
diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
index 298903c..2c34996 100644
--- a/tools/kvm/virtio/net.c
+++ b/tools/kvm/virtio/net.c
@@ -712,6 +712,7 @@ static int virtio_net__init_one(struct virtio_net_params *params)
 		ndev->info.guest_netmask	= ntohl(inet_addr("255.255.255.0"));
 		ndev->info.buf_nr		= 20,
 		ndev->ops = &uip_ops;
+		uip_static_init(&ndev->info);
 	}
 
 	if (params->trans && strcmp(params->trans, "mmio") == 0)
-- 
1.8.2.3


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux