From: Markus Elfring <elfr...@users.sourceforge.net>
Date: Fri, 26 Aug 2016 13:31:40 +0200

1. The kfree() function was called in some cases by the
   cryptocop_ioctl_process() function during error handling
   even if the passed variable contained a null pointer.

2. Split a condition check for memory allocation failures.

3. Adjust jump targets according to the Linux coding style convention.

4. Omit an unneeded check for the local variable "cop" then at the end.

5. Delete assignments which became unnecessary with this refactoring
   for two local variables.

Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
---
 arch/cris/arch-v32/drivers/cryptocop.c | 80 ++++++++++++++++++----------------
 1 file changed, 42 insertions(+), 38 deletions(-)

diff --git a/arch/cris/arch-v32/drivers/cryptocop.c 
b/arch/cris/arch-v32/drivers/cryptocop.c
index 1165639..26347a2 100644
--- a/arch/cris/arch-v32/drivers/cryptocop.c
+++ b/arch/cris/arch-v32/drivers/cryptocop.c
@@ -2542,7 +2542,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
        if (!jc) {
                DEBUG_API(printk("cryptocop_ioctl_process: kmalloc\n"));
                err = -ENOMEM;
-               goto error_cleanup;
+               goto free_cop;
        }
        jc->processed = 0;
 
@@ -2575,7 +2575,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
                if (!tc) {
                        DEBUG_API(printk("cryptocop_ioctl_process: no cipher 
transform in session.\n"));
                        err = -EINVAL;
-                       goto error_cleanup;
+                       goto free_jc;
                }
                ciph_tcfg.tid = CRYPTOCOP_IOCTL_CIPHER_TID;
                ciph_tcfg.inject_ix = 0;
@@ -2628,14 +2628,14 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
                if (!tc) {
                        DEBUG_API(printk("cryptocop_ioctl_process: no digest 
transform in session.\n"));
                        err = -EINVAL;
-                       goto error_cleanup;
+                       goto free_jc;
                }
                digest_length = tc->init.alg == cryptocop_alg_md5 ? 16 : 20;
                digest_result = kmalloc(digest_length, GFP_KERNEL);
                if (!digest_result) {
                        DEBUG_API(printk("cryptocop_ioctl_process: kmalloc 
digest_result\n"));
                        err = -EINVAL;
-                       goto error_cleanup;
+                       goto free_jc;
                }
                DEBUG(memset(digest_result, 0xff, digest_length));
 
@@ -2645,7 +2645,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
                if ((oper.digest_start < 0) || (oper.digest_len <= 0) || 
(oper.digest_start > oper.inlen) || ((oper.digest_start + oper.digest_len) > 
oper.inlen)){
                        DEBUG_API(printk("cryptocop_ioctl_process: bad digest 
length\n"));
                        err = -EINVAL;
-                       goto error_cleanup;
+                       goto free_digest;
                }
 
                digest_tcfg.next = cop->tfrm_op.tfrm_cfg;
@@ -2670,9 +2670,8 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
        prev_ix = first_cfg_change_ix(&oper);
        if (prev_ix > oper.inlen) {
                DEBUG_API(printk("cryptocop_ioctl_process: length mismatch\n"));
-               nooutpages = noinpages = 0;
                err = -EINVAL;
-               goto error_cleanup;
+               goto free_digest;
        }
        DEBUG(printk("cryptocop_ioctl_process: inlen=%d, cipher_outlen=%d\n", 
oper.inlen, oper.cipher_outlen));
 
@@ -2682,9 +2681,8 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
        inpages = kmalloc_array(noinpages, sizeof(*inpages), GFP_KERNEL);
        if (!inpages){
                DEBUG_API(printk("cryptocop_ioctl_process: kmalloc inpages\n"));
-               nooutpages = noinpages = 0;
                err = -ENOMEM;
-               goto error_cleanup;
+               goto free_digest;
        }
        if (oper.do_cipher){
                nooutpages = (((unsigned long int)oper.cipher_outdata & 
~PAGE_MASK) + oper.cipher_outlen - 1 + ~PAGE_MASK) >> PAGE_SHIFT;
@@ -2694,9 +2692,8 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
                                         GFP_KERNEL);
                if (!outpages){
                        DEBUG_API(printk("cryptocop_ioctl_process: kmalloc 
outpages\n"));
-                       nooutpages = noinpages = 0;
                        err = -ENOMEM;
-                       goto error_cleanup;
+                       goto free_inpages;
                }
        }
 
@@ -2712,9 +2709,8 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
 
        if (err < 0) {
                up_read(&current->mm->mmap_sem);
-               nooutpages = noinpages = 0;
                DEBUG_API(printk("cryptocop_ioctl_process: get_user_pages 
indata\n"));
-               goto error_cleanup;
+               goto free_outpages;
        }
        noinpages = err;
        if (oper.do_cipher){
@@ -2726,9 +2722,8 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
                                     NULL);
                up_read(&current->mm->mmap_sem);
                if (err < 0) {
-                       nooutpages = 0;
                        DEBUG_API(printk("cryptocop_ioctl_process: 
get_user_pages outdata\n"));
-                       goto error_cleanup;
+                       goto put_inpages;
                }
                nooutpages = err;
        } else {
@@ -2740,13 +2735,18 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
        cop->tfrm_op.indata = kmalloc_array(noinpages,
                                            sizeof(*cop->tfrm_op.indata),
                                            GFP_KERNEL);
+       if (!cop->tfrm_op.indata) {
+               DEBUG_API(printk("cryptocop_ioctl_process: kmalloc indata\n"));
+               err = -ENOMEM;
+               goto put_outpages;
+       }
        cop->tfrm_op.outdata = kmalloc_array(6 + nooutpages,
                                             sizeof(*cop->tfrm_op.outdata),
                                             GFP_KERNEL);
-       if (!cop->tfrm_op.indata || !cop->tfrm_op.outdata) {
-               DEBUG_API(printk("cryptocop_ioctl_process: kmalloc iovecs\n"));
+       if (!cop->tfrm_op.outdata) {
+               DEBUG_API(printk("cryptocop_ioctl_process: kmalloc outdata\n"));
                err = -ENOMEM;
-               goto error_cleanup;
+               goto free_indata;
        }
 
        cop->tfrm_op.inlen = oper.inlen - prev_ix;
@@ -2780,7 +2780,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
        if (prev_ix == next_ix){
                DEBUG_API(printk("cryptocop_ioctl_process: length configuration 
broken.\n"));
                err = -EINVAL;  /* This should be impossible barring bugs. */
-               goto error_cleanup;
+               goto free_outdata;
        }
        while (prev_ix != next_ix){
                end_digest = end_csum = cipher_active = digest_active = 
csum_active = 0;
@@ -2834,7 +2834,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
                if (!descs[desc_ix].cfg){
                        DEBUG_API(printk("cryptocop_ioctl_process: data segment 
%d (%d to %d) had no active transforms\n", desc_ix, prev_ix, next_ix));
                        err = -EINVAL;
-                       goto error_cleanup;
+                       goto mark_outpages_dirty;
                }
                descs[desc_ix].next = &(descs[desc_ix]) + 1;
                ++desc_ix;
@@ -2864,7 +2864,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
                if (!map_pages_to_iovec(cop->tfrm_op.outdata, iovlen, &iovix, 
outpages, nooutpages, &pageix, &pageoffset, oper.cipher_outlen)){
                        DEBUG_API(printk("cryptocop_ioctl_process: failed to 
map pages to iovec.\n"));
                        err = -ENOSYS; /* This should be impossible barring 
bugs. */
-                       goto error_cleanup;
+                       goto mark_outpages_dirty;
                }
        DEBUG(printk("cryptocop_ioctl_process: setting cop->tfrm_op.outcount 
%d\n", iovix));
        cop->tfrm_op.outcount = iovix;
@@ -2878,7 +2878,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
        if ((err = cryptocop_job_queue_insert_user_job(cop)) != 0) {
                DEBUG_API(printk("cryptocop_ioctl_process: insert job %d\n", 
err));
                err = -EINVAL;
-               goto error_cleanup;
+               goto mark_outpages_dirty;
        }
 
        DEBUG(printk("cryptocop_ioctl_process: begin wait for result\n"));
@@ -2888,7 +2888,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
         if (!jc->processed){
                printk(KERN_WARNING "cryptocop_ioctl_process: job not processed 
at completion\n");
                err = -EIO;
-               goto error_cleanup;
+               goto mark_outpages_dirty;
        }
 
        /* Job process done.  Cipher output should already be correct in job so 
no post processing of outdata. */
@@ -2900,7 +2900,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
                        if (0 != err){
                                DEBUG_API(printk("cryptocop_ioctl_process: 
copy_to_user, digest length %d, err %d\n", digest_length, err));
                                err = -EFAULT;
-                               goto error_cleanup;
+                               goto mark_outpages_dirty;
                        }
                }
                if (oper.do_csum){
@@ -2909,7 +2909,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
                        if (0 != err){
                                DEBUG_API(printk("cryptocop_ioctl_process: 
copy_to_user, csum, err %d\n", err));
                                err = -EFAULT;
-                               goto error_cleanup;
+                               goto mark_outpages_dirty;
                        }
                }
                err = 0;
@@ -2917,29 +2917,33 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
                DEBUG(printk("cryptocop_ioctl_process: returning err = 
operation_status = %d\n", cop->operation_status));
                err = cop->operation_status;
        }
-
- error_cleanup:
-       /* Release page caches. */
-       for (i = 0; i < noinpages; i++)
-               put_page(inpages[i]);
+ mark_outpages_dirty:
        for (i = 0; i < nooutpages; i++){
                int spdl_err;
                /* Mark output pages dirty. */
                spdl_err = set_page_dirty_lock(outpages[i]);
                DEBUG(if (spdl_err < 0)printk("cryptocop_ioctl_process: 
set_page_dirty_lock returned %d\n", spdl_err));
        }
+ free_outdata:
+       kfree(cop->tfrm_op.outdata);
+ free_indata:
+       kfree(cop->tfrm_op.indata);
+ put_outpages:
        for (i = 0; i < nooutpages; i++)
                put_page(outpages[i]);
-       kfree(digest_result);
-       kfree(inpages);
+ put_inpages:
+       for (i = 0; i < noinpages; i++)
+               put_page(inpages[i]);
+ free_outpages:
        kfree(outpages);
-       if (cop){
-               kfree(cop->tfrm_op.indata);
-               kfree(cop->tfrm_op.outdata);
-               kfree(cop);
-       }
+ free_inpages:
+       kfree(inpages);
+ free_digest:
+       kfree(digest_result);
+ free_jc:
        kfree(jc);
-
+ free_cop:
+       kfree(cop);
        DEBUG(print_lock_status());
 
        return err;
-- 
2.9.3

Reply via email to