lkml.org 
[lkml]   [2016]   [Aug]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Subject[PATCH 4/8] cris-cryptocop: Less function calls in cryptocop_ioctl_process() after error detection
    From
    Date
    From: Markus Elfring <elfring@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 <elfring@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
    \
     
     \ /
      Last update: 2016-09-17 09:57    [W:3.492 / U:0.948 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site