lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 19 Dec 2017 11:29:42 -0800
From:   Dongwon Kim <dongwon.kim@...el.com>
To:     linux-kernel@...r.kernel.org
Cc:     dri-devel@...ts.freedesktop.org, xen-devel@...ts.xenproject.org,
        mateuszx.potrola@...el.com, dongwon.kim@...el.com
Subject: [RFC PATCH 26/60] hyper_dmabuf: add mutexes to prevent several race conditions

From: Mateusz Polrola <mateuszx.potrola@...el.com>

Added mutex to export_fd ioctl to prevent double pages mapping of the
same buffer to prevent race condition when two consumers are trying to
map the same buffer on importer VM.

Also locked mutex before sending request via xen communication channel
to prevent req_pending override by another caller.

Signed-off-by: Dongwon Kim <dongwon.kim@...el.com>
---
 drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c          |  2 ++
 drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.h          |  1 +
 drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c        |  6 ++++++
 drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.c | 10 ++++++++++
 drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.h |  1 +
 5 files changed, 20 insertions(+)

diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c b/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c
index 569b95e..584d55d 100644
--- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c
+++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c
@@ -56,6 +56,8 @@ static int __init hyper_dmabuf_drv_init(void)
 
 	printk( KERN_NOTICE "hyper_dmabuf_starting: Initialization started" );
 
+	mutex_init(&hyper_dmabuf_private.lock);
+
 	ret = register_device();
 	if (ret < 0) {
 		return -EINVAL;
diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.h b/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.h
index 0b1441e..8445416 100644
--- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.h
+++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.h
@@ -76,6 +76,7 @@ struct hyper_dmabuf_private {
 
 	/* backend ops - hypervisor specific */
 	struct hyper_dmabuf_backend_ops *backend_ops;
+	struct mutex lock;
 };
 
 #endif /* __LINUX_PUBLIC_HYPER_DMABUF_DRV_H__ */
diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c b/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c
index 018de8c..8851a9c 100644
--- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c
+++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c
@@ -260,6 +260,8 @@ static int hyper_dmabuf_export_fd_ioctl(void *data)
 	if (sgt_info == NULL || !sgt_info->valid) /* can't find sgt from the table */
 		return -1;
 
+	mutex_lock(&hyper_dmabuf_private.lock);
+
 	sgt_info->num_importers++;
 
 	/* send notification for export_fd to exporter */
@@ -274,6 +276,7 @@ static int hyper_dmabuf_export_fd_ioctl(void *data)
 		kfree(req);
 		dev_err(hyper_dmabuf_private.device, "Failed to create sgt or notify exporter\n");
 		sgt_info->num_importers--;
+		mutex_unlock(&hyper_dmabuf_private.lock);
 		return -EINVAL;
 	}
 	kfree(req);
@@ -282,6 +285,7 @@ static int hyper_dmabuf_export_fd_ioctl(void *data)
 		dev_err(hyper_dmabuf_private.device,
 			"Buffer invalid\n");
 		sgt_info->num_importers--;
+		mutex_unlock(&hyper_dmabuf_private.lock);
 		return -1;
 	} else {
 		dev_dbg(hyper_dmabuf_private.device, "Can import buffer\n");
@@ -303,6 +307,7 @@ static int hyper_dmabuf_export_fd_ioctl(void *data)
 
 		if (!data_pages) {
 			sgt_info->num_importers--;
+			mutex_unlock(&hyper_dmabuf_private.lock);
 			return -EINVAL;
 		}
 
@@ -318,6 +323,7 @@ static int hyper_dmabuf_export_fd_ioctl(void *data)
 		ret = export_fd_attr->fd;
 	}
 
+	mutex_unlock(&hyper_dmabuf_private.lock);
 	dev_dbg(hyper_dmabuf_private.device, "%s exit\n", __func__);
 	return 0;
 }
diff --git a/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.c b/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.c
index a8cce26..9d67b47 100644
--- a/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.c
+++ b/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.c
@@ -278,6 +278,8 @@ int hyper_dmabuf_xen_init_tx_rbuf(int domid)
 	ring_info->irq = ret;
 	ring_info->port = alloc_unbound.port;
 
+	mutex_init(&ring_info->lock);
+
 	dev_dbg(hyper_dmabuf_private.device,
 		"%s: allocated eventchannel gref %d  port: %d  irq: %d\n",
 		__func__,
@@ -512,6 +514,9 @@ int hyper_dmabuf_xen_send_req(int domid, struct hyper_dmabuf_req *req, int wait)
 		return -EINVAL;
 	}
 
+
+	mutex_lock(&ring_info->lock);
+
 	ring = &ring_info->ring_front;
 
 	if (RING_FULL(ring))
@@ -519,6 +524,7 @@ int hyper_dmabuf_xen_send_req(int domid, struct hyper_dmabuf_req *req, int wait)
 
 	new_req = RING_GET_REQUEST(ring, ring->req_prod_pvt);
 	if (!new_req) {
+		mutex_unlock(&ring_info->lock);
 		dev_err(hyper_dmabuf_private.device,
 			"NULL REQUEST\n");
 		return -EIO;
@@ -548,13 +554,17 @@ int hyper_dmabuf_xen_send_req(int domid, struct hyper_dmabuf_req *req, int wait)
 		}
 
 		if (timeout < 0) {
+			mutex_unlock(&ring_info->lock);
 			dev_err(hyper_dmabuf_private.device, "request timed-out\n");
 			return -EBUSY;
 		}
 
+		mutex_unlock(&ring_info->lock);
 		return req_pending.status;
 	}
 
+	mutex_unlock(&ring_info->lock);
+
 	return 0;
 }
 
diff --git a/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.h b/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.h
index 9c93165..0533e4d 100644
--- a/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.h
+++ b/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.h
@@ -39,6 +39,7 @@ struct xen_comm_tx_ring_info {
         int gref_ring;
         int irq;
         int port;
+	struct mutex lock;
 	struct xenbus_watch watch;
 };
 
-- 
2.7.4

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ