gdc: fix gdc coverity issue [1/1]
authorCao Jian <jian.cao@amlogic.com>
Fri, 15 Nov 2019 02:59:19 +0000 (10:59 +0800)
committerJianxin Pan <jianxin.pan@amlogic.com>
Mon, 9 Dec 2019 12:09:32 +0000 (05:09 -0700)
PD#SWPL-13979

Problem:
coverity defect cleanup for gdc

Solution:
DEADCODE on aml_dmabuf_ops_attach()
RESOURCE_LEAK on aml_dma_alloc()
REVERSE_INULL on aml_dma_mmap()
USE_AFTER_FREE on aml_dma_put()
CHECKED_RETURN on gdc_platform_probe()
FORWARD_NULL on gdc_process_ex_info()/gdc_process_with_fw()
UNINIT on gdc_process_ex_info()/gdc_process_with_fw()
NO_EFFECT on gdc_log()

Verify:
coverity check

Change-Id: Iafd7eb5f74792300c68c50ffc5d0e36063cce906
Signed-off-by: Cao Jian <jian.cao@amlogic.com>
(cherry picked from commit b28a8ac621c428437199386895a7ac3012e52905)

drivers/amlogic/media/gdc/app/gdc_dmabuf.c
drivers/amlogic/media/gdc/app/gdc_main.c
drivers/amlogic/media/gdc/app/gdc_module.c
drivers/amlogic/media/gdc/app/gdc_wq.c
drivers/amlogic/media/gdc/inc/sys/system_log.h

index f9514cf930b0765776a526b23cb8bd2367ec5dd0..2c0426cf59e1e584454eebb3906ec3cec4eca6b8 100644 (file)
@@ -65,7 +65,7 @@ static void *aml_mm_vmap(phys_addr_t phys, unsigned long size)
                return NULL;
        }
        vfree(pages);
-       gdc_log(LOG_INFO, "[HIGH-MEM-MAP] pa(%lx) to va(%p), size: %d\n",
+       gdc_log(LOG_DEBUG, "[HIGH-MEM-MAP] pa(%lx) to va(%p), size: %d\n",
                (unsigned long)phys, vaddr, npages << PAGE_SHIFT);
        return vaddr;
 }
@@ -88,7 +88,7 @@ static void aml_dma_put(void *buf_priv)
        void *vaddr = (void *)(PAGE_MASK & (ulong)buf->vaddr);
 
        if (!atomic_dec_and_test(&buf->refcount)) {
-               gdc_log(LOG_INFO, "gdc aml_dma_put, refcont=%d\n",
+               gdc_log(LOG_DEBUG, "gdc aml_dma_put, refcont=%d\n",
                        atomic_read(&buf->refcount));
                return;
        }
@@ -102,9 +102,10 @@ static void aml_dma_put(void *buf_priv)
        buf->vaddr = NULL;
        clear_dma_buffer((struct aml_dma_buffer *)buf->priv, buf->index);
        put_device(buf->dev);
-       kfree(buf);
-       gdc_log(LOG_INFO, "gdc free:aml_dma_buf=0x%p,buf->index=%d\n",
+
+       gdc_log(LOG_DEBUG, "gdc free:aml_dma_buf=0x%p,buf->index=%d\n",
                buf, buf->index);
+       kfree(buf);
 }
 
 static void *aml_dma_alloc(struct device *dev, unsigned long attrs,
@@ -129,6 +130,7 @@ static void *aml_dma_alloc(struct device *dev, unsigned long attrs,
        if (cma_pages) {
                paddr = page_to_phys(cma_pages);
        } else {
+               kfree(buf);
                pr_err("failed to alloc cma pages.\n");
                return NULL;
        }
@@ -138,7 +140,7 @@ static void *aml_dma_alloc(struct device *dev, unsigned long attrs,
        buf->dma_dir = dma_dir;
        buf->dma_addr = paddr;
        atomic_inc(&buf->refcount);
-       gdc_log(LOG_INFO, "aml_dma_buf=0x%p, refcont=%d\n",
+       gdc_log(LOG_DEBUG, "aml_dma_buf=0x%p, refcont=%d\n",
                buf, atomic_read(&buf->refcount));
 
        return buf;
@@ -148,7 +150,7 @@ static int aml_dma_mmap(void *buf_priv, struct vm_area_struct *vma)
 {
        struct aml_dma_buf *buf = buf_priv;
        unsigned long pfn = 0;
-       unsigned long vsize = vma->vm_end - vma->vm_start;
+       unsigned long vsize;
        int ret = -1;
 
        if (!buf || !vma) {
@@ -156,6 +158,8 @@ static int aml_dma_mmap(void *buf_priv, struct vm_area_struct *vma)
                return -EINVAL;
        }
 
+       vsize = vma->vm_end - vma->vm_start;
+
        pfn = buf->dma_addr >> PAGE_SHIFT;
        ret = remap_pfn_range(vma, vma->vm_start, pfn,
                vsize, vma->vm_page_prot);
@@ -165,7 +169,7 @@ static int aml_dma_mmap(void *buf_priv, struct vm_area_struct *vma)
        }
        vma->vm_flags |= VM_DONTEXPAND;
 
-       gdc_log(LOG_INFO, "mapped dma addr 0x%08lx at 0x%08lx, size %d\n",
+       gdc_log(LOG_DEBUG, "mapped dma addr 0x%08lx at 0x%08lx, size %d\n",
                (unsigned long)buf->dma_addr, vma->vm_start,
                buf->size);
        return 0;
@@ -207,11 +211,6 @@ static int aml_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
        for_each_sg(sgt->sgl, sg, sgt->nents, i) {
                struct page *page = phys_to_page(phys);
 
-               if (!page) {
-                       sg_free_table(sgt);
-                       kfree(attach);
-                       return -ENOMEM;
-               }
                sg_set_page(sg, page, PAGE_SIZE, 0);
                phys += PAGE_SIZE;
        }
@@ -346,7 +345,7 @@ static struct dma_buf *get_dmabuf(void *buf_priv, unsigned long flags)
 
        /* dmabuf keeps reference to vb2 buffer */
        atomic_inc(&buf->refcount);
-       gdc_log(LOG_INFO, "get_dmabuf, refcount=%d\n",
+       gdc_log(LOG_DEBUG, "get_dmabuf, refcount=%d\n",
                atomic_read(&buf->refcount));
 
        return dbuf;
@@ -362,7 +361,7 @@ static int find_empty_dma_buffer(struct aml_dma_buffer *buffer)
                if (buffer->gd_buffer[i].alloc)
                        continue;
                else {
-                       gdc_log(LOG_INFO, "find_empty_dma_buffer i=%d\n", i);
+                       gdc_log(LOG_DEBUG, "find_empty_dma_buffer i=%d\n", i);
                        found = 1;
                        break;
                }
@@ -504,7 +503,7 @@ int gdc_dma_buffer_export(struct aml_dma_buffer *buffer,
                dma_buf_put(dbuf);
                return ret;
        }
-       gdc_log(LOG_INFO, "buffer %d,exported as %d descriptor\n",
+       gdc_log(LOG_DEBUG, "buffer %d,exported as %d descriptor\n",
                index, ret);
        buffer->gd_buffer[index].fd = ret;
        buffer->gd_buffer[index].dbuf = dbuf;
@@ -563,7 +562,7 @@ int gdc_dma_buffer_map(struct aml_dma_cfg *cfg)
        cfg->attach = d_att;
        cfg->vaddr = vaddr;
        cfg->sg = sg;
-       gdc_log(LOG_INFO, "gdc_dma_buffer_map, dbuf=0x%p\n", dbuf);
+       gdc_log(LOG_DEBUG, "gdc_dma_buffer_map, dbuf=0x%p\n", dbuf);
 
        return ret;
 
@@ -621,7 +620,7 @@ int gdc_dma_buffer_get_phys(struct aml_dma_buffer *buffer,
        }
        ret = gdc_dma_buffer_get_phys_internal(buffer, cfg, addr);
        if (ret < 0) {
-               gdc_log(LOG_INFO, "get_phys(fd=%d) failed.\n", cfg->fd);
+               gdc_log(LOG_DEBUG, "get_phys(fd=%d) failed.\n", cfg->fd);
                ret = gdc_dma_buffer_map(cfg);
                if (ret < 0) {
                        pr_err("gdc_dma_buffer_map failed\n");
@@ -692,7 +691,7 @@ void gdc_dma_buffer_unmap(struct aml_dma_cfg *cfg)
        dma_buf_detach(dbuf, d_att);
 
        dma_buf_put(dbuf);
-       gdc_log(LOG_INFO, "gdc_dma_buffer_unmap, dbuf=0x%p\n", dbuf);
+       gdc_log(LOG_DEBUG, "gdc_dma_buffer_unmap, dbuf=0x%p\n", dbuf);
 }
 
 void gdc_dma_buffer_dma_flush(struct device *dev, int fd)
@@ -700,7 +699,7 @@ void gdc_dma_buffer_dma_flush(struct device *dev, int fd)
        struct dma_buf *dmabuf;
        struct aml_dma_buf *buf;
 
-       gdc_log(LOG_INFO, "gdc_dma_buffer_dma_flush fd=%d\n", fd);
+       gdc_log(LOG_DEBUG, "gdc_dma_buffer_dma_flush fd=%d\n", fd);
        dmabuf = dma_buf_get(fd);
        if (IS_ERR(dmabuf)) {
                pr_err("dma_buf_get failed\n");
@@ -722,7 +721,7 @@ void gdc_dma_buffer_cache_flush(struct device *dev, int fd)
        struct dma_buf *dmabuf;
        struct aml_dma_buf *buf;
 
-       gdc_log(LOG_INFO, "gdc_dma_buffer_cache_flush fd=%d\n", fd);
+       gdc_log(LOG_DEBUG, "gdc_dma_buffer_cache_flush fd=%d\n", fd);
        dmabuf = dma_buf_get(fd);
        if (IS_ERR(dmabuf)) {
                pr_err("dma_buf_get failed\n");
index 8827123073d19dfd9f03ed43148b99ffeb79af6e..ed3c67238d59d5917259c3dae1cb603186cbf293 100644 (file)
@@ -27,7 +27,7 @@ int gdc_run(struct gdc_cmd_s *g)
 
        gdc_stop(g);
 
-       gdc_log(LOG_INFO, "Done gdc load..\n");
+       gdc_log(LOG_DEBUG, "Done gdc load..\n");
 
        //initialise the gdc by the first configuration
        if (gdc_init(g) != 0) {
@@ -35,7 +35,7 @@ int gdc_run(struct gdc_cmd_s *g)
                return -1;
        }
 
-       gdc_log(LOG_INFO, "Done gdc config..\n");
+       gdc_log(LOG_DEBUG, "Done gdc config..\n");
 
        switch (g->gdc_config.format) {
        case NV12:
index 71ec73e627c632ee474f5185b5c40bf05c1a7f24..ddbd172464a4aeb344bb33c2c2a030791f894fd0 100644 (file)
@@ -48,7 +48,7 @@
 #include "gdc_dmabuf.h"
 #include "gdc_wq.h"
 
-unsigned int gdc_log_level;
+int gdc_log_level;
 struct gdc_manager_s gdc_manager;
 unsigned int gdc_reg_store_mode;
 int trace_mode_enable;
@@ -526,11 +526,6 @@ static int gdc_buffer_get_phys(struct aml_dma_cfg *cfg, unsigned long *addr)
        return gdc_dma_buffer_get_phys(gdc_manager.buffer, cfg, addr);
 }
 
-static int gdc_buffer_unmap(struct aml_dma_cfg *cfg)
-{
-       return gdc_dma_buffer_unmap_info(gdc_manager.buffer, cfg);
-}
-
 static int gdc_get_buffer_fd(int plane_id, struct gdc_buffer_info *buf_info)
 {
        int fd;
@@ -718,29 +713,39 @@ static long gdc_process_input_dma_info(struct gdc_context_s *context,
                        ret = meson_gdc_set_input_addr(addr, gdc_cmd);
                        if (ret != 0) {
                                gdc_log(LOG_ERR, "set input addr err\n");
-                               return -EINVAL;
+                               goto dma_buf_unmap;
                        }
                } else {
                        size = gdc_set_input_addr(i, addr, gdc_cmd);
                        if (size < 0) {
                                gdc_log(LOG_ERR, "set input addr err\n");
-                               return -EINVAL;
+                               goto dma_buf_unmap;
                        }
 
                }
-               gdc_log(LOG_INFO, "plane[%d] get input addr=%lx\n",
+               gdc_log(LOG_DEBUG, "plane[%d] get input addr=%lx\n",
                        i, addr);
                if (plane_number == 1) {
                        size = gdc_get_input_size(gdc_cmd);
                        if (size < 0) {
                                gdc_log(LOG_ERR, "set input addr err\n");
-                               return -EINVAL;
+                               ret = -EINVAL;
+                               goto dma_buf_unmap;
                        }
                }
                meson_gdc_dma_flush(&gdc_manager.gdc_dev->pdev->dev,
                        addr, size);
        }
        return 0;
+
+dma_buf_unmap:
+       while (i >= 0) {
+               cfg = &context->dma_cfg.input_cfg[i].dma_cfg;
+               gdc_dma_buffer_unmap(cfg);
+               context->dma_cfg.input_cfg[i].dma_used = 0;
+               i--;
+       }
+       return ret;
 }
 
 static long gdc_process_input_ion_info(struct gdc_context_s *context,
@@ -787,7 +792,7 @@ static long gdc_process_input_ion_info(struct gdc_context_s *context,
                        }
 
                }
-               gdc_log(LOG_INFO, "plane[%d] get input addr=%lx\n",
+               gdc_log(LOG_DEBUG, "plane[%d] get input addr=%lx\n",
                        i, addr);
        }
        return 0;
@@ -834,13 +839,26 @@ static long gdc_process_output_dma_info(struct gdc_context_s *context,
                        ret = gdc_set_output_addr(i, addr, gdc_cmd);
                        if (ret < 0) {
                                gdc_log(LOG_ERR, "set input addr err\n");
-                               return -EINVAL;
+                               ret = -EINVAL;
+                               goto dma_buf_unmap;
                        }
                }
-               gdc_log(LOG_INFO, "plane[%d] get output addr=%lx\n",
+               gdc_log(LOG_DEBUG, "plane[%d] get output addr=%lx\n",
                        i, addr);
        }
+
        return 0;
+
+dma_buf_unmap:
+       while (i >= 0) {
+               cfg = &context->dma_cfg.output_cfg[i].dma_cfg;
+               gdc_dma_buffer_unmap(cfg);
+               context->dma_cfg.output_cfg[i].dma_used = 0;
+               i--;
+       }
+
+       return ret;
+
 }
 
 static long gdc_process_output_ion_info(struct gdc_context_s *context,
@@ -883,7 +901,7 @@ static long gdc_process_output_ion_info(struct gdc_context_s *context,
                                return -EINVAL;
                        }
                }
-               gdc_log(LOG_INFO, "plane[%d] get output addr=%lx\n",
+               gdc_log(LOG_DEBUG, "plane[%d] get output addr=%lx\n",
                        i, addr);
        }
        return 0;
@@ -897,7 +915,7 @@ static long gdc_process_ex_info(struct gdc_context_s *context,
        size_t len;
        struct aml_dma_cfg *cfg = NULL;
        struct gdc_cmd_s *gdc_cmd = &context->cmd;
-       struct gdc_queue_item_s *pitem;
+       struct gdc_queue_item_s *pitem = NULL;
        int i;
 
        if (context == NULL || gs_ex == NULL) {
@@ -916,14 +934,14 @@ static long gdc_process_ex_info(struct gdc_context_s *context,
        if (gs_ex->output_buffer.mem_alloc_type == AML_GDC_MEM_ION) {
                ret = gdc_process_output_ion_info(context, gs_ex);
                if (ret < 0) {
-                       mutex_unlock(&context->d_mutext);
-                       return -EINVAL;
+                       ret = -EINVAL;
+                       goto unlock_return;
                }
        } else if (gs_ex->output_buffer.mem_alloc_type == AML_GDC_MEM_DMABUF) {
                ret = gdc_process_output_dma_info(context, gs_ex);
                if (ret < 0) {
                        ret = -EINVAL;
-                       goto unmap;
+                       goto unlock_return;
                }
        }
        gdc_cmd->base_gdc = 0;
@@ -937,7 +955,7 @@ static long gdc_process_ex_info(struct gdc_context_s *context,
                        gdc_log(LOG_ERR, "ion import config fd %d failed\n",
                                gs_ex->config_buffer.shared_fd);
                        ret = -EINVAL;
-                       goto unmap;
+                       goto unlock_return;
                }
        } else if (gs_ex->config_buffer.mem_alloc_type == AML_GDC_MEM_DMABUF) {
                /* dma alloc */
@@ -951,25 +969,25 @@ static long gdc_process_ex_info(struct gdc_context_s *context,
                        gdc_log(LOG_ERR, "dma import config fd %d failed\n",
                                gs_ex->config_buffer.shared_fd);
                        ret = -EINVAL;
-                       goto unmap;
+                       goto unlock_return;
                }
        }
        gdc_cmd->gdc_config.config_addr = addr;
-       gdc_log(LOG_INFO, "%s, config addr=%lx\n", __func__, addr);
+       gdc_log(LOG_DEBUG, "%s, config addr=%lx\n", __func__, addr);
 
        if (gs_ex->input_buffer.mem_alloc_type == AML_GDC_MEM_ION) {
                /* ion alloc */
                ret = gdc_process_input_ion_info(context, gs_ex);
                if (ret < 0) {
                        ret = -EINVAL;
-                       goto unmap;
+                       goto unlock_return;
                }
        } else if (gs_ex->input_buffer.mem_alloc_type == AML_GDC_MEM_DMABUF) {
                /* dma alloc */
                ret = gdc_process_input_dma_info(context, gs_ex);
                if (ret < 0) {
                        ret = -EINVAL;
-                       goto unmap;
+                       goto unlock_return;
                }
        }
        gdc_cmd->outplane = gs_ex->output_buffer.plane_number;
@@ -985,7 +1003,7 @@ static long gdc_process_ex_info(struct gdc_context_s *context,
        if (pitem == NULL) {
                gdc_log(LOG_ERR, "get item error\n");
                ret = -ENOMEM;
-               goto unmap;
+               goto unlock_return;
        }
        mutex_unlock(&context->d_mutext);
        if (gs_ex->config_buffer.mem_alloc_type == AML_GDC_MEM_DMABUF)
@@ -993,25 +1011,8 @@ static long gdc_process_ex_info(struct gdc_context_s *context,
 
        gdc_wq_add_work(context, pitem);
        return 0;
-unmap:
-       /* if dma buf detach it */
-       for (i = 0; i < MAX_PLANE; i++) {
-               if (pitem->dma_cfg.input_cfg[i].dma_used) {
-                       gdc_buffer_unmap
-                               (&pitem->dma_cfg.input_cfg[i].dma_cfg);
-                       pitem->dma_cfg.input_cfg[i].dma_used = 0;
-               }
-               if (pitem->dma_cfg.output_cfg[i].dma_used) {
-                       gdc_buffer_unmap
-                               (&pitem->dma_cfg.output_cfg[i].dma_cfg);
-                       pitem->dma_cfg.output_cfg[i].dma_used = 0;
-               }
-       }
-       if (pitem->dma_cfg.config_cfg.dma_used) {
-               gdc_buffer_unmap
-                       (&pitem->dma_cfg.config_cfg.dma_cfg);
-               pitem->dma_cfg.config_cfg.dma_used = 0;
-       }
+
+unlock_return:
        mutex_unlock(&context->d_mutext);
        return ret;
 }
@@ -1118,8 +1119,7 @@ static long gdc_process_with_fw(struct gdc_context_s *context,
        long ret = -1;
        struct gdc_cmd_s *gdc_cmd = &context->cmd;
        char *fw_name = NULL;
-       struct gdc_queue_item_s *pitem;
-       int i;
+       struct gdc_queue_item_s *pitem = NULL;
 
        if (context == NULL || gs_with_fw == NULL) {
                gdc_log(LOG_ERR, "Error input param\n");
@@ -1163,13 +1163,19 @@ static long gdc_process_with_fw(struct gdc_context_s *context,
                                        (struct gdc_settings_ex *)gs_with_fw);
                if (ret < 0) {
                        ret = -EINVAL;
-                       goto unmap;
+                       goto release_fw_name;
                }
        } else if (gs_with_fw->input_buffer.mem_alloc_type ==
                                                AML_GDC_MEM_DMABUF) {
                /* dma alloc */
+               ret =
                gdc_process_input_dma_info(context,
-                                       (struct gdc_settings_ex *)gs_with_fw);
+                                          (struct gdc_settings_ex *)
+                                          gs_with_fw);
+               if (ret < 0) {
+                       ret = -EINVAL;
+                       goto release_fw_name;
+               }
        }
 
        gdc_cmd->outplane = gs_with_fw->output_buffer.plane_number;
@@ -1318,29 +1324,12 @@ static long gdc_process_with_fw(struct gdc_context_s *context,
        release_config_firmware(gs_with_fw);
        kfree(fw_name);
        return 0;
+
 release_fw:
        release_config_firmware(gs_with_fw);
-unmap:
-       /* if dma buf detach it */
-       for (i = 0; i < MAX_PLANE; i++) {
-               if (pitem->dma_cfg.input_cfg[i].dma_used) {
-                       gdc_dma_buffer_unmap
-                               (&pitem->dma_cfg.input_cfg[i].dma_cfg);
-                       pitem->dma_cfg.input_cfg[i].dma_used = 0;
-               }
-               if (pitem->dma_cfg.output_cfg[i].dma_used) {
-                       gdc_dma_buffer_unmap
-                               (&pitem->dma_cfg.output_cfg[i].dma_cfg);
-                       pitem->dma_cfg.output_cfg[i].dma_used = 0;
-               }
-       }
-       if (pitem->dma_cfg.config_cfg.dma_used) {
-               gdc_dma_buffer_unmap
-                       (&pitem->dma_cfg.config_cfg.dma_cfg);
-               pitem->dma_cfg.config_cfg.dma_used = 0;
-       }
-       mutex_unlock(&context->d_mutext);
+
 release_fw_name:
+       mutex_unlock(&context->d_mutext);
        kfree(fw_name);
 
        return ret;
@@ -1365,7 +1354,7 @@ static long meson_gdc_ioctl(struct file *file, unsigned int cmd,
        ion_phys_addr_t addr;
        int index, dma_fd;
        void __user *argp = (void __user *)arg;
-       struct gdc_queue_item_s *pitem;
+       struct gdc_queue_item_s *pitem = NULL;
        struct device *dev = NULL;
 
        context = (struct gdc_context_s *)file->private_data;
@@ -1840,7 +1829,9 @@ static int gdc_platform_probe(struct platform_device *pdev)
                return -ENOMEM;
        }
 
-       of_reserved_mem_device_init(&(pdev->dev));
+       rc = of_reserved_mem_device_init(&pdev->dev);
+       if (rc != 0)
+               gdc_log(LOG_INFO, "reserve_mem is not used\n");
 
        /* alloc mem to store config out path*/
        config_out_file = kzalloc(CONFIG_PATH_LENG, GFP_KERNEL);
index 0a789698d130a279eacec7cd5b9315764591d408..e5b53f3713a138444cdcf8063cb46ea9a7790cf6 100644 (file)
@@ -447,11 +447,11 @@ void *gdc_prepare_item(struct gdc_context_s *wq)
 int gdc_wq_add_work(struct gdc_context_s *wq,
        struct gdc_queue_item_s *pitem)
 {
-       gdc_log(LOG_INFO, "gdc add work\n");
+       gdc_log(LOG_DEBUG, "gdc add work\n");
        spin_lock(&wq->lock);
        list_move_tail(&pitem->list, &wq->work_queue);
        spin_unlock(&wq->lock);
-       gdc_log(LOG_INFO, "gdc add work ok\n");
+       gdc_log(LOG_DEBUG, "gdc add work ok\n");
        /* only read not need lock */
        if (gdc_manager.event.cmd_in_sem.count == 0)
                up(&gdc_manager.event.cmd_in_sem);/* new cmd come in */
index 53380bd784aa35ce302314edad85ee7bcabb709a..cee79b4c956a2df5baa84e8eb7a2ab64cf936e6d 100644 (file)
 
 //changeable logs
 #include <linux/kernel.h>
-extern unsigned int gdc_log_level;
+extern int gdc_log_level;
 
 enum log_level_e {
-       LOG_NO_THING,
        LOG_CRIT,
-       LOG_ERR = 0,
+       LOG_ERR,
+       LOG_INFO = 0,
        LOG_WARNING,
-       LOG_INFO,
        LOG_DEBUG,
        LOG_MAX
 };