Make sure to free buffers before returning to prevent memory leaks
Signed-off-by: Dongwon Kim dongwon.kim@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);