Bug 616500

Summary: fix coverity Defect Type: Resource leaks issues CID 12094 - 12136
Product: [Retired] 389 Reporter: Endi Sukma Dewata <edewata>
Component: Directory ServerAssignee: Rich Megginson <rmeggins>
Status: CLOSED CURRENTRELEASE QA Contact: Viktor Ashirov <vashirov>
Severity: medium Docs Contact:
Priority: high    
Version: 1.2.7CC: amsharma, jgalipea, nhosoi
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: All   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-12-07 16:32:20 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 576869, 639035    
Attachments:
Description Flags
0001-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
0002-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
0003-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
0004-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review-
0005-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
0006-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review-
0007-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
0008-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
0009-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
0010-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
0011-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review-
0012-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
0013-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
0014-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review-
0015-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
0016-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
0017-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review-
coverity ID 12125
rmeggins: review+
coverity ID 12131
rmeggins: review+
coverity ID 12135
rmeggins: review+
0004a-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
0006a-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
0011a-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
0014a-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
0017a-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch
nhosoi: review+
coverity ID 12099 rmeggins: review+

Description Endi Sukma Dewata 2010-07-20 15:48:06 UTC
fix coverity Defect Type: Resource leaks issues CID 12094 - 12136

Comment 3 Endi Sukma Dewata 2010-07-21 23:23:52 UTC
Created attachment 433540 [details]
0001-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 4 Endi Sukma Dewata 2010-07-21 23:38:04 UTC
Created attachment 433542 [details]
0002-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 5 Endi Sukma Dewata 2010-07-21 23:38:23 UTC
Created attachment 433543 [details]
0003-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 6 Endi Sukma Dewata 2010-07-21 23:38:44 UTC
Created attachment 433544 [details]
0004-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 7 Endi Sukma Dewata 2010-07-21 23:39:02 UTC
Created attachment 433545 [details]
0005-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 8 Endi Sukma Dewata 2010-07-21 23:39:18 UTC
Created attachment 433546 [details]
0006-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 9 Endi Sukma Dewata 2010-07-21 23:39:38 UTC
Created attachment 433547 [details]
0007-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 10 Endi Sukma Dewata 2010-07-21 23:39:56 UTC
Created attachment 433548 [details]
0008-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 11 Endi Sukma Dewata 2010-07-21 23:40:13 UTC
Created attachment 433549 [details]
0009-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 12 Endi Sukma Dewata 2010-07-21 23:40:31 UTC
Created attachment 433550 [details]
0010-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 13 Endi Sukma Dewata 2010-07-21 23:40:50 UTC
Created attachment 433551 [details]
0011-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 14 Endi Sukma Dewata 2010-07-21 23:41:08 UTC
Created attachment 433552 [details]
0012-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 15 Endi Sukma Dewata 2010-07-21 23:41:37 UTC
Created attachment 433553 [details]
0013-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 16 Endi Sukma Dewata 2010-07-21 23:42:04 UTC
Created attachment 433554 [details]
0014-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 17 Endi Sukma Dewata 2010-07-21 23:42:31 UTC
Created attachment 433555 [details]
0015-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 18 Endi Sukma Dewata 2010-07-21 23:42:52 UTC
Created attachment 433556 [details]
0016-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 19 Endi Sukma Dewata 2010-07-21 23:43:12 UTC
Created attachment 433557 [details]
0017-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

Comment 26 Noriko Hosoi 2010-08-17 23:53:02 UTC
Comment on attachment 433544 [details]
0004-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

http://10.16.47.145:8080/sourcebrowser.htm?projectId=10030#mergedDefectId=12104&streamDefectId=12290&defectInstanceId=14177&fileInstanceId=49003

It's not a good idea to free memory which may not be allocated in this function.  (note: index could be one of the list items: plg->indexes.)  Rather, we could check the bail out case before allocating index.

diff --git a/ldap/servers/slapd/index_subsystem.c b/ldap/servers/slapd/index_subsystem.c
index a0b16b0..e9b4033 100644
--- a/ldap/servers/slapd/index_subsystem.c
+++ b/ldap/servers/slapd/index_subsystem.c
@@ -1056,6 +1056,15 @@ int slapi_index_register_index(char *plugin_id, indexed_item *registration_item,
        goto bail;
    }
 
+   /* map the namespace dn to a backend dn */
+   be = slapi_be_select( registration_item->namespace_dn );
+  
+   if(be == defbackend_get_backend())
+   {
+       ret = -1;
+       goto bail;
+   }
+
    /* now add the new index - we shall assume indexes
     * will not be registered twice by different plugins,
     * in that event, the last one added wins
@@ -1096,15 +1105,6 @@ int slapi_index_register_index(char *plugin_id, indexed_item *registration_item,
    index->lookup_func = registration_item->search_op;
    index->user_data = user_data;
 
-   /* map the namespace dn to a backend dn */
-   be = slapi_be_select( registration_item->namespace_dn );
-  
-   if(be == defbackend_get_backend())
-   {
-       ret = -1;
-       goto bail;
-   }
-
    index->namespace_dn = (Slapi_DN*)slapi_be_getsuffix(be, 0);

Comment 28 Noriko Hosoi 2010-08-18 00:03:58 UTC
Comment on attachment 433546 [details]
0006-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

http://10.16.47.145:8080/sourcebrowser.htm?projectId=10030#mergedDefectId=12106&streamDefectId=12292&defectInstanceId=14179&fileInstanceId=49060

We don't have to check the return value from slapi_ch_strdup, too.
@@ -570,6 +570,7 @@ internal_plugin_search_referral_callback(char *referral, void *callback_data)
     
 	if ((this_referral->data = slapi_ch_strdup(referral)) == NULL) 
     {
+        slapi_ch_free(&this_referral);
         return(0);
     }
==>

  563    /* add this to the list of referrals we are making */  
  564    this_referral = (Referral_Node *)slapi_ch_calloc(1,sizeof(Referral_Node));
  566    this_referral->data = slapi_ch_strdup(referral);

Comment 34 Noriko Hosoi 2010-08-18 00:40:40 UTC
Comment on attachment 433551 [details]
0011-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

http://10.16.47.145:8080/sourcebrowser.htm?projectId=10030#mergedDefectId=12116&streamDefectId=12302&defectInstanceId=14189&fileInstanceId=49638

We should check the return value from syscall malloc.
  411scalab01_addLogin (
  412        thread_context  *tttctx,
  413        char            *dn,
  414        int              duration)
  415{
  416  int            ret;   /* Return value */
  417  isp_user      *new;   /* New entry */
  418  isp_user      *cur;   /* Current entry */
  419
  420  /*
  421   * Create the new record.
  422   */
Assigning: "new" = storage returned from "malloc(1040UL)".
Calling allocation function "malloc".
  423  new = (isp_user *) malloc (sizeof (isp_user));

            if (NULL == new) {
                fprintf (stderr, "ldclt[%d]: %s: cannot malloc(isp_user), error=%d (%s)\n",
                    mctx.pid, tttctx->thrdId, errno, strerror (errno));
                fflush (stderr);
                return (-1);
            }

  424  strcpy (new->dn, dn);
  425  new->cost = new->counter = duration;
  426  new->next = NULL;
  427

Comment 39 Noriko Hosoi 2010-08-18 01:16:34 UTC
Comment on attachment 433554 [details]
0014-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

http://10.16.47.145:8080/sourcebrowser.htm?projectId=10030#mergedDefectId=12124&streamDefectId=12310&defectInstanceId=14197&fileInstanceId=48875

In the error case, certinfo is freed.
1689 error:
1690     if (free_certinfo) free(certinfo);
1691

How about these members?
1665   certinfo->issuerName = strdup(issuerName);
1666   certinfo->issuerDN = strdup(issuerDN);

Comment 40 Noriko Hosoi 2010-08-18 18:05:22 UTC
Created attachment 439457 [details]
coverity ID 12125

http://10.16.47.145:8080/sourcebrowser.htm?projectId=10030#mergedDefectId=12125&streamDefectId=12311&defectInstanceId=14198&fileInstanceId=48875

Files:
Modified:
 include/ldaputil/ldaputil.h
 lib/ldaputil/certmap.c
Deleted:
 lib/ldaputil/utest/Makefile
 lib/ldaputil/utest/auth.cpp
 lib/ldaputil/utest/authtest
 lib/ldaputil/utest/certmap.conf
 lib/ldaputil/utest/dblist.conf
 lib/ldaputil/utest/example.c
 lib/ldaputil/utest/plugin.c
 lib/ldaputil/utest/plugin.h
 lib/ldaputil/utest/stubs.c
 lib/ldaputil/utest/stubs.cpp
 lib/ldaputil/utest/test.ref

Comment:
This function (ldapu_certinfo_save) is not used - just get rid of it
Removing unused functions from lib/ldaputil/certmap.c
  ldapu_certinfo_save, ldapu_certinfo_modify, ldapu_certinfo_delete
Also, removing obsolete test codes: lib/ldaputil/utest.

Comment 44 Noriko Hosoi 2010-08-18 18:23:48 UTC
Comment on attachment 433557 [details]
0017-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

http://10.16.47.145:8080/sourcebrowser.htm?projectId=10030#mergedDefectId=12136&streamDefectId=12322&defectInstanceId=14209&fileInstanceId=49656
0017-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

This error case is not fixed.
  176        printf("Error creating properties file %s\n",DATABASE_NAME);
Variable "dbfile" going out of scope leaks the storage it points to.
  177        return 1;

Comment 45 Noriko Hosoi 2010-08-18 19:12:37 UTC
Created attachment 439472 [details]
coverity ID 12131

Comment: ACL_AuthInfoSetDbname -- free dbtype in the error cases.

File:  lib/libaccess/register.cpp

Comment 46 Noriko Hosoi 2010-08-18 19:13:14 UTC
(In reply to comment #45)
> Created attachment 439472 [details]
> coverity ID 12131
> 
> Comment: ACL_AuthInfoSetDbname -- free dbtype in the error cases.
> 
> File:  lib/libaccess/register.cpp

http://10.16.47.145:8080/sourcebrowser.htm?projectId=10030#mergedDefectId=12131&streamDefectId=12317&defectInstanceId=14204&fileInstanceId=49208

Comment 47 Noriko Hosoi 2010-08-18 19:21:50 UTC
Created attachment 439475 [details]
coverity ID 12135

http://10.16.47.145:8080/sourcebrowser.htm?projectId=10030#mergedDefectId=12135&streamDefectId=12321&defectInstanceId=14208&fileInstanceId=49218

Comment: Getting rid of unused functions fopen_l and fclose_l
from lib/libadmin/util.c.

Comment 48 Endi Sukma Dewata 2010-08-18 23:56:43 UTC
Created attachment 439545 [details]
0004a-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

The patch has been modified to check the bail out case before allocating index.

Comment 49 Endi Sukma Dewata 2010-08-18 23:58:31 UTC
Created attachment 439546 [details]
0006a-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

The patch has been modified to remove unnecessary NULL checking.

Comment 50 Endi Sukma Dewata 2010-08-19 00:01:34 UTC
Created attachment 439548 [details]
0011a-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

The patch has been modified to check the return value from malloc.

Comment 51 Endi Sukma Dewata 2010-08-19 00:06:02 UTC
Created attachment 439552 [details]
0014a-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

The patch has been modified to remove the changes for ldapu_certinfo_modify() because the function has been removed in coverity ID 12125.

Comment 52 Endi Sukma Dewata 2010-08-19 00:10:38 UTC
Created attachment 439553 [details]
0017a-Bug-616500-fix-coverify-Defect-Type-Resource-leaks-i.patch

The patch has been modified to check for NULL dbfile and hresfile and release resources before it returns.

Comment 58 Noriko Hosoi 2010-08-19 22:35:42 UTC
Created attachment 439813 [details]
coverity ID 12099

http://10.16.47.145:8080/sourcebrowser.htm?projectId=10030&mergedDefectIds=11991#mergedDefectId=12099&streamDefectId=12285&defectInstanceId=14172&fileInstanceId=49665

Comment:
need to find out where the pb is freed - but not serious here because the server is shutting down

Comment 59 Rich Megginson 2010-08-20 18:37:59 UTC
Comment on attachment 439457 [details]
coverity ID 12125

yay for getting rid of code :-)

Comment 60 Rich Megginson 2010-08-20 18:39:09 UTC
Comment on attachment 439475 [details]
coverity ID 12135

yay less code :-)

Comment 61 Noriko Hosoi 2010-08-20 22:01:23 UTC
On behalf of Endi (edewata), pushed to master.

$ git merge endi
Updating 10d4035..d5b1593
Fast-forward
 include/ldaputil/ldaputil.h                   |   10 -
 ldap/servers/plugins/collation/collate.c      |   22 +-
 ldap/servers/slapd/back-ldbm/import-threads.c |   12 +-
 ldap/servers/slapd/back-ldbm/vlv.c            |   30 +-
 ldap/servers/slapd/connection.c               |    9 +-
 ldap/servers/slapd/csngen.c                   |    4 +-
 ldap/servers/slapd/daemon.c                   |    2 +-
 ldap/servers/slapd/index_subsystem.c          |   18 +-
 ldap/servers/slapd/libglobs.c                 |    1 +
 ldap/servers/slapd/plugin_internal_op.c       |    7 +-
 ldap/servers/slapd/psearch.c                  |    1 +
 ldap/servers/slapd/tools/ldclt/data.c         |   50 ++-
 ldap/servers/slapd/tools/ldclt/ldapfct.c      |   28 +-
 ldap/servers/slapd/tools/ldclt/parser.c       |   19 +-
 ldap/servers/slapd/tools/ldclt/scalab01.c     |   21 +-
 ldap/servers/snmp/ldap-agent.c                |    3 +
 ldap/systools/idsktune.c                      |   12 +-
 lib/ldaputil/certmap.c                        |  209 ++-------
 lib/ldaputil/utest/Makefile                   |  149 ------
 lib/ldaputil/utest/auth.cpp                   |  611 -------------------------
 lib/ldaputil/utest/authtest                   |  138 ------
 lib/ldaputil/utest/certmap.conf               |   68 ---
 lib/ldaputil/utest/dblist.conf                |   47 --
 lib/ldaputil/utest/example.c                  |  153 ------
 lib/ldaputil/utest/plugin.c                   |  152 ------
 lib/ldaputil/utest/plugin.h                   |   57 ---
 lib/ldaputil/utest/stubs.c                    |  144 ------
 lib/ldaputil/utest/stubs.cpp                  |  139 ------
 lib/ldaputil/utest/test.ref                   |  480 -------------------
 lib/libaccess/register.cpp                    |   17 +-
 lib/libadmin/error.c                          |    2 +
 lib/libadmin/template.c                       |    2 +
 lib/libadmin/util.c                           |   48 --
 lib/libsi18n/makstrdb.c                       |   21 +-
 34 files changed, 236 insertions(+), 2450 deletions(-)
 delete mode 100644 lib/ldaputil/utest/Makefile
 delete mode 100644 lib/ldaputil/utest/auth.cpp
 delete mode 100755 lib/ldaputil/utest/authtest
 delete mode 100644 lib/ldaputil/utest/certmap.conf
 delete mode 100644 lib/ldaputil/utest/dblist.conf
 delete mode 100644 lib/ldaputil/utest/example.c
 delete mode 100644 lib/ldaputil/utest/plugin.c
 delete mode 100644 lib/ldaputil/utest/plugin.h
 delete mode 100644 lib/ldaputil/utest/stubs.c
 delete mode 100644 lib/ldaputil/utest/stubs.cpp
 delete mode 100644 lib/ldaputil/utest/test.ref

$ git push
Counting objects: 192, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (151/151), done.
Writing objects: 100% (151/151), 16.57 KiB, done.
Total 151 (delta 128), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
   10d4035..d5b1593  master -> master

Comment 62 Amita Sharma 2011-05-13 06:56:57 UTC
Coverity Related, Can I request dev to please test this.