[RFC PATCH 43/60] hyper_dmabuf: fixes on memory leaks in various places
Dongwon Kim
dongwon.kim at intel.com
Tue Dec 19 19:29:59 UTC 2017
Make sure to free buffers before returning to prevent memory leaks
Signed-off-by: Dongwon Kim <dongwon.kim at intel.com>
---
drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c | 19 +++++++-
drivers/xen/hyper_dmabuf/hyper_dmabuf_msg.c | 9 +++-
drivers/xen/hyper_dmabuf/hyper_dmabuf_ops.c | 6 ++-
drivers/xen/hyper_dmabuf/hyper_dmabuf_sgl_proc.c | 4 +-
.../xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.c | 52 +++++++++++++++++++---
5 files changed, 78 insertions(+), 12 deletions(-)
diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c b/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c
index 283fe5a..3215003 100644
--- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c
+++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c
@@ -282,6 +282,7 @@ static int hyper_dmabuf_export_remote_ioctl(struct file *filp, void *data)
/* free msg */
kfree(req);
+
/* free page_info */
kfree(page_info->pages);
kfree(page_info);
@@ -298,6 +299,10 @@ static int hyper_dmabuf_export_remote_ioctl(struct file *filp, void *data)
fail_map_req:
hyper_dmabuf_remove_exported(sgt_info->hid);
+ /* free page_info */
+ kfree(page_info->pages);
+ kfree(page_info);
+
fail_export:
kfree(sgt_info->va_vmapped);
@@ -433,6 +438,13 @@ static int hyper_dmabuf_export_fd_ioctl(struct file *filp, void *data)
sgt_info->num_importers--;
req = kcalloc(1, sizeof(*req), GFP_KERNEL);
+
+ if (!req) {
+ dev_err(hyper_dmabuf_private.device,
+ "No more space left\n");
+ return -ENOMEM;
+ }
+
hyper_dmabuf_create_request(req, HYPER_DMABUF_EXPORT_FD_FAILED, &operands[0]);
ops->send_req(HYPER_DMABUF_DOM_ID(sgt_info->hid), req, false);
kfree(req);
@@ -681,16 +693,19 @@ long hyper_dmabuf_ioctl(struct file *filp,
if (copy_from_user(kdata, (void __user *)param, _IOC_SIZE(cmd)) != 0) {
dev_err(hyper_dmabuf_private.device, "failed to copy from user arguments\n");
- return -EFAULT;
+ ret = -EFAULT;
+ goto ioctl_error;
}
ret = func(filp, kdata);
if (copy_to_user((void __user *)param, kdata, _IOC_SIZE(cmd)) != 0) {
dev_err(hyper_dmabuf_private.device, "failed to copy to user arguments\n");
- return -EFAULT;
+ ret = -EFAULT;
+ goto ioctl_error;
}
+ioctl_error:
kfree(kdata);
return ret;
diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_msg.c b/drivers/xen/hyper_dmabuf/hyper_dmabuf_msg.c
index c516df8..46cf9a4 100644
--- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_msg.c
+++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_msg.c
@@ -191,8 +191,7 @@ int hyper_dmabuf_msg_parse(int domid, struct hyper_dmabuf_req *req)
struct hyper_dmabuf_req *temp_req;
struct hyper_dmabuf_imported_sgt_info *sgt_info;
struct hyper_dmabuf_sgt_info *exp_sgt_info;
- hyper_dmabuf_id_t hid = {req->operands[0], /* hid.id */
- {req->operands[1], req->operands[2], req->operands[3]}}; /* hid.rng_key */
+ hyper_dmabuf_id_t hid;
int ret;
if (!req) {
@@ -200,6 +199,11 @@ int hyper_dmabuf_msg_parse(int domid, struct hyper_dmabuf_req *req)
return -EINVAL;
}
+ hid.id = req->operands[0];
+ hid.rng_key[0] = req->operands[1];
+ hid.rng_key[1] = req->operands[2];
+ hid.rng_key[2] = req->operands[3];
+
if ((req->command < HYPER_DMABUF_EXPORT) ||
(req->command > HYPER_DMABUF_OPS_TO_SOURCE)) {
dev_err(hyper_dmabuf_private.device, "invalid command\n");
@@ -332,6 +336,7 @@ int hyper_dmabuf_msg_parse(int domid, struct hyper_dmabuf_req *req)
if (!proc) {
dev_err(hyper_dmabuf_private.device,
"No memory left to be allocated\n");
+ kfree(temp_req);
return -ENOMEM;
}
diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_ops.c b/drivers/xen/hyper_dmabuf/hyper_dmabuf_ops.c
index 81cb09f..9313c42 100644
--- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_ops.c
+++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_ops.c
@@ -148,9 +148,8 @@ static struct sg_table* hyper_dmabuf_ops_map(struct dma_buf_attachment *attachme
if (!st)
goto err_free_sg;
- if (!dma_map_sg(attachment->dev, st->sgl, st->nents, dir)) {
+ if (!dma_map_sg(attachment->dev, st->sgl, st->nents, dir))
goto err_free_sg;
- }
ret = hyper_dmabuf_sync_request(sgt_info->hid,
HYPER_DMABUF_OPS_MAP);
@@ -171,6 +170,9 @@ static struct sg_table* hyper_dmabuf_ops_map(struct dma_buf_attachment *attachme
kfree(st);
}
+ kfree(page_info->pages);
+ kfree(page_info);
+
return NULL;
}
diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_sgl_proc.c b/drivers/xen/hyper_dmabuf/hyper_dmabuf_sgl_proc.c
index c2d013a..dd17d26 100644
--- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_sgl_proc.c
+++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_sgl_proc.c
@@ -89,8 +89,10 @@ struct hyper_dmabuf_pages_info *hyper_dmabuf_ext_pgs(struct sg_table *sgt)
return NULL;
pinfo->pages = kmalloc(sizeof(struct page *)*hyper_dmabuf_get_num_pgs(sgt), GFP_KERNEL);
- if (!pinfo->pages)
+ if (!pinfo->pages) {
+ kfree(pinfo);
return NULL;
+ }
sgl = sgt->sgl;
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 43dd3b6..9689346 100644
--- a/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.c
+++ b/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.c
@@ -229,9 +229,16 @@ int hyper_dmabuf_xen_init_tx_rbuf(int domid)
ring_info = kmalloc(sizeof(*ring_info), GFP_KERNEL);
+ if (!ring_info) {
+ dev_err(hyper_dmabuf_private.device,
+ "No more spae left\n");
+ return -ENOMEM;
+ }
+
/* from exporter to importer */
shared_ring = (void *)__get_free_pages(GFP_KERNEL, 1);
if (shared_ring == 0) {
+ kfree(ring_info);
return -ENOMEM;
}
@@ -246,6 +253,7 @@ int hyper_dmabuf_xen_init_tx_rbuf(int domid)
0);
if (ring_info->gref_ring < 0) {
/* fail to get gref */
+ kfree(ring_info);
return -EFAULT;
}
@@ -256,6 +264,7 @@ int hyper_dmabuf_xen_init_tx_rbuf(int domid)
if (ret) {
dev_err(hyper_dmabuf_private.device,
"Cannot allocate event channel\n");
+ kfree(ring_info);
return -EIO;
}
@@ -271,6 +280,7 @@ int hyper_dmabuf_xen_init_tx_rbuf(int domid)
HYPERVISOR_event_channel_op(EVTCHNOP_close, &close);
gnttab_end_foreign_access(ring_info->gref_ring, 0,
virt_to_mfn(shared_ring));
+ kfree(ring_info);
return -EIO;
}
@@ -299,6 +309,14 @@ int hyper_dmabuf_xen_init_tx_rbuf(int domid)
*/
ring_info->watch.callback = remote_dom_exporter_watch_cb;
ring_info->watch.node = (const char*) kmalloc(sizeof(char) * 255, GFP_KERNEL);
+
+ if (!ring_info->watch.node) {
+ dev_err(hyper_dmabuf_private.device,
+ "No more space left\n");
+ kfree(ring_info);
+ return -ENOMEM;
+ }
+
sprintf((char*)ring_info->watch.node,
"/local/domain/%d/data/hyper_dmabuf/%d/port",
domid, hyper_dmabuf_xen_get_domid());
@@ -392,8 +410,16 @@ int hyper_dmabuf_xen_init_rx_rbuf(int domid)
map_ops = kmalloc(sizeof(*map_ops), GFP_KERNEL);
+ if (!map_ops) {
+ dev_err(hyper_dmabuf_private.device,
+ "No memory left to be allocated\n");
+ ret = -ENOMEM;
+ goto fail_no_map_ops;
+ }
+
if (gnttab_alloc_pages(1, &shared_ring)) {
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto fail_others;
}
gnttab_set_map_op(&map_ops[0], (unsigned long)pfn_to_kaddr(page_to_pfn(shared_ring)),
@@ -405,12 +431,14 @@ int hyper_dmabuf_xen_init_rx_rbuf(int domid)
ret = gnttab_map_refs(map_ops, NULL, &shared_ring, 1);
if (ret < 0) {
dev_err(hyper_dmabuf_private.device, "Cannot map ring\n");
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail_others;
}
if (map_ops[0].status) {
dev_err(hyper_dmabuf_private.device, "Ring mapping failed\n");
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail_others;
} else {
ring_info->unmap_op.handle = map_ops[0].handle;
}
@@ -424,7 +452,8 @@ int hyper_dmabuf_xen_init_rx_rbuf(int domid)
ret = bind_interdomain_evtchn_to_irq(domid, rx_port);
if (ret < 0) {
- return -EIO;
+ ret = -EIO;
+ goto fail_others;
}
ring_info->irq = ret;
@@ -445,6 +474,12 @@ int hyper_dmabuf_xen_init_rx_rbuf(int domid)
back_ring_isr, 0,
NULL, (void*)ring_info);
+fail_others:
+ kfree(map_ops);
+
+fail_no_map_ops:
+ kfree(ring_info);
+
return ret;
}
@@ -520,15 +555,22 @@ int hyper_dmabuf_xen_send_req(int domid, struct hyper_dmabuf_req *req, int wait)
return -ENOENT;
}
-
mutex_lock(&ring_info->lock);
ring = &ring_info->ring_front;
while (RING_FULL(ring)) {
+ if (timeout == 0) {
+ dev_err(hyper_dmabuf_private.device,
+ "Timeout while waiting for an entry in the ring\n");
+ return -EIO;
+ }
usleep_range(100, 120);
+ timeout--;
}
+ timeout = 1000;
+
new_req = RING_GET_REQUEST(ring, ring->req_prod_pvt);
if (!new_req) {
mutex_unlock(&ring_info->lock);
--
2.7.4
More information about the dri-devel
mailing list