Description of problem: http://10.16.47.145:8080/defects/index.htm?projectId=10030 click Memory - illegal accesses
Created attachment 427794 [details] git patch file (9.0) 12300 USE_AFTER_FREE Triaged Unassigned Bug Minor Fix Required append_acl_to_file(char *, char *, char *…) ds/lib/libaccess/acltools.cpp Comment: this code is not used any more. The fix is to have open_file_buf set *buf to NULL after freeing
The patch in comment 1 fixes these defects as well. 12292 USE_AFTER_FREE Triaged Unassigned Bug Minor Fix Required delete_acl_from_file(char *, char *…) ds/lib/libaccess/acltools.cpp 12294 USE_AFTER_FREE Triaged Unassigned Bug Minor Fix Required get_acl_from_file(char *, char *, ACLListHandle **…) ds/lib/libaccess/acltools.cpp 12296 USE_AFTER_FREE Triaged Unassigned Bug Minor Fix Required ACL_FileGetNameList() ds/lib/libaccess/acltools.cpp 12298 USE_AFTER_FREE Triaged Unassigned Bug Minor Fix Required rename_acl_in_file(char *, char *, char *…) ds/lib/libaccess/acltools.cpp
Created attachment 427801 [details] git patch file (9.0) 12291 USE_AFTER_FREE Triaged Unassigned Bug Minor Fix Required dbconf_read_default_dbinfo_sub() ds/lib/ldaputil/dbconf.c Comment: should never happen - should always break out of the loop at 504 with a valid db_info - but we should set db_info = NULL after line 505 just to make sure
Created attachment 427824 [details] git patch file (9.0) 12210 UNINIT Triaged Unassigned Bug Minor Fix Required str2entry_fast() ds/ldap/servers/slapd/entry.c Comment: ptype is always NULL the first time through the loop, so a will always be initialized to NULL. But we should explicitly initialize it to NULL in the declaration.
Created attachment 427830 [details] git patch file (9.0) 12215 UNINIT Triaged Unassigned Bug Minor Fix Required _cl5LDIF2Operation() ds/ldap/servers/plugins/replication/cl5_api.c Comment: should init rawDN to NULL and check if it is NULL before using it Comment on the particular rawDN at the line 5218: /* * When it comes here, case T_DNSTR is already * passed and rawDN is supposed to set. * But it's a good idea to make sure it is * not NULL. */
Created attachment 427831 [details] git patch file (9.0) 12216 UNINIT Triaged Unassigned Bug Minor Fix Required private_protocol_factory() ds/ldap/servers/plugins/replication/repl5_protocol.c Comment: should be impossible for type to be anything but one of the valid values, but it wouldn't hurt to init prp to NULL anyway
Created attachment 427832 [details] git patch file (9.0) 12220 UNINIT Triaged Unassigned Bug Minor Fix Required create_NSDS50ReplicationExtopPayload() ds/ldap/servers/plugins/replication/repl_extop.c Comment: unlikely to cause a problem, but we should init sdn to NULL
Created attachment 427835 [details] git patch file (9.0) 12221 UNINIT Triaged Unassigned Bug Minor Fix Required create_NSDS50ReplicationExtopPayload() ds/ldap/servers/plugins/replication/repl_extop.c Comment: unlikely to cause a problem, but we should init repl_obj to NULL
Created attachment 427838 [details] git patch file (9.0) 12222 UNINIT Triaged Unassigned Bug Minor Fix Required replica_get_purl_for_op() ds/ldap/servers/plugins/replication/repl5_plugins.c Comment: In case of an error "cannot obtain consumer connection extension or supplier_ruv", uninitialized purl is returned to the caller. Init purl to NULL.
Created attachment 427839 [details] git patch file (9.0) 12223 UNINIT Triaged Unassigned Bug Minor Fix Required my_ber_scanf_attr() ds/ldap/servers/plugins/replication/repl5_total.c Comment: In case an error occurs between the line 594 and the line 648, uninitialized value is passed to slapi_value_free. Need to init value to NULL
Created attachment 428039 [details] git patch file (9.0) 12224 UNINIT Triaged Unassigned Bug Minor Fix Required windows_private_update_dirsync_control() ds/ldap/servers/plugins/replication/windows_private.c Comment: If DIRSYNC control is not found, uninitialized ber is passed to ber_free. should init ber to NULL.
Created attachment 428048 [details] git patch file (9.0) 12225 UNINIT Triaged Unassigned Bug Minor Fix Required windows_private_update_dirsync_control() ds/ldap/servers/plugins/replication/windows_private.c Comment: If DIRSYNC control is not found, uninitialized serverCookie is passed to ber_bvfree. We should init serverCookie to NULL.
Created attachment 428052 [details] git patch file (9.0) 12230 UNINIT Triaged Unassigned Bug Minor Fix Required preop_add() ds/ldap/servers/plugins/uiduniq/7bit.c Comment: Some cases such as NULL attrName is passed or it does not have a value, uninitialized "violated" is passed to slapi_ch_smprintf via issue_error. We should init violated to NULL. 12231 UNINIT Triaged Unassigned Bug Unspecified Fix Required preop_modify() ds/ldap/servers/plugins/uiduniq/7bit.c Comment: Some cases such as NULL attrName is passed or mods were empty, uninitialized "violated" is passed to slapi_ch_smprintf via issue_error. We should init violated to NULL. 12232 UNINIT Triaged Unassigned Bug Minor Fix Required preop_modrdn() ds/ldap/servers/plugins/uiduniq/7bit.c Comment: Some cases such as NULL attrName is passed or it does not have a value, uninitialized "violated" is passed to slapi_ch_smprintf via issue_error. We should init violated to NULL.
Created attachment 428053 [details] git patch file (9.0) 12233 UNINIT Triaged Unassigned Bug Minor Fix Required preop_modify() ds/ldap/servers/plugins/uiduniq/uid.c Comment: This is not an issue since attrName is an output variable for getArguments at the line 689. But to make coverity happy, we init attrName to NULL.
Created attachment 428057 [details] git patch file (9.0) 12236 UNINIT Triaged Unassigned Bug Minor Fix Required dblayer_get_aux_id2entry() ds/ldap/servers/slapd/back-ldbm/dblayer.c Comment: If a backend instance info (inst) or ldbminfo (li) or dblayer private info (opriv) is not available, uninitialized priv is passed to slapi_ch_free_string and slapi_ch_free. We need to init priv to NULL and handle done: case if priv is NULL.
Created attachment 428058 [details] git patch file (9.0) 12237 UNINIT Triaged Unassigned Bug Minor Fix Required vlv_trim_candidates_byvalue() ds/ldap/servers/slapd/back-ldbm/vlv.c Comment: There is almost no chance to pass uninitialized typedown_value to ber_bvecfree unless vlv_request_control value is NULL. Anyway, we init typedown_value to NULL.
Created attachment 428142 [details] git patch file (9.0) 12210 UNINIT Triaged Unassigned Bug Minor Fix Required str2entry_fast() ds/ldap/servers/slapd/entry.c Comment: ptype is always NULL the first time through the loop, so a will always be initialized to NULL. But we should explicitly initialize it to NULL in the declaration. Moved the declaration of the variable 'a' outside of the loop. This is needed to initialize the variable just once at the beginning.
Created attachment 428526 [details] git patch file (9.0) 12241 Triaged Unassigned Bug Minor Fix Required delete_acl_from_file(char *, char *…) ds/lib/libaccess/acltools.cpp 12242 UNINIT Triaged Unassigned Bug Minor Fix Required get_acl_from_file(char *, char *, ACLListHandle **…) ds/lib/libaccess/acltools.cpp 12243 UNINIT Triaged Unassigned Bug Minor Fix Required ACL_FileGetNameList() ds/lib/libaccess/acltools.cpp 12244 UNINIT Triaged Unassigned Bug Minor Fix Required ACL_FileGetNameList() ds/lib/libaccess/acltools.cpp 12245 UNINIT Triaged Unassigned Bug Minor Fix Required rename_acl_in_file(char *, char *, char *…) ds/lib/libaccess/acltools.cpp 12246 UNINIT Triaged Unassigned Bug Minor Fix Required append_acl_to_file(char *, char *, char *…) ds/lib/libaccess/acltools.cpp 12247 UNINIT Triaged Unassigned Bug Minor Fix Required append_acl_to_file(char *, char *, char *…) ds/lib/libaccess/acltools.cpp Comment: ACL_FileRenameAcl, ACL_FileDeleteAcl, ACL_FileGetAcl, ACL_FileSetAcl, ACL_FileMergeAcl, ACL_FileMergeFile and their helper functions are not used. These functions and their helper functions plus libaccess test programs under the directory ./utest are eliminated.
Note: the following coverity items are covered by the patch Created an attachment (id=428526) [details] in Comment 18 (https://bugzilla.redhat.com/show_bug.cgi?id=609255#c18). 12292 USE_AFTER_FREE Triaged Unassigned Bug Minor Fix Required delete_acl_from_file(char *, char *…) ds/lib/libaccess/acltools.cpp 12294 USE_AFTER_FREE Triaged Unassigned Bug Minor Fix Required get_acl_from_file(char *, char *, ACLListHandle **…) ds/lib/libaccess/acltools.cpp 12296 USE_AFTER_FREE Triaged Unassigned Bug Minor Fix Required ACL_FileGetNameList() ds/lib/libaccess/acltools.cpp 12298 USE_AFTER_FREE Triaged Unassigned Bug Minor Fix Required rename_acl_in_file(char *, char *, char *…) ds/lib/libaccess/acltools.cpp 12300 USE_AFTER_FREE Triaged Unassigned Bug Minor Fix Required append_acl_to_file(char *, char *, char *…) ds/lib/libaccess/acltools.cpp
Comment on attachment 427830 [details] git patch file (9.0) mods must be freed before returning.
Comment on attachment 428142 [details] git patch file (9.0) hmm - are you sure this is ok to do? this means 'a' can retain its previous value on the next loop iteration if there is a continue or some other condition that causes another loop iteration without freeing 'a'
Comment on attachment 428526 [details] git patch file (9.0) I love getting rid of useless code . . .
Created attachment 428563 [details] revised patch (replacing attachment 427830 [details]) (In reply to comment #20) > (From update of attachment 427830 [details]) > mods must be freed before returning. Thanks, Rich! Attached is a revised patch.
(In reply to comment #21) > (From update of attachment 428142 [details]) > hmm - are you sure this is ok to do? this means 'a' can retain its previous > value on the next loop iteration if there is a continue or some other condition > that causes another loop iteration without freeing 'a' Well, I tested just assigning NULL to 'a' at the original position in the loop, then I got this error many times: str2entry_fast: Error. Non-contiguous attribute values for objectclass Looking at the code around the error, NULL 'a' is used to determine the attribute values are non-contiguous. If 'a' is NULL, the type is treated at the first time in the loop. 382 if(a==NULL) 383 { 384 switch(attr_state) 385 { 386 case ATTRIBUTE_PRESENT: 387 if(attrlist_find_or_create_locking_optional(&e->e_attrs, type, &a, PR_FALSE)==0 /* Found */) 388 { 389 LDAPDebug (LDAP_DEBUG_ANY, "str2entry_fast: Error. Non-contiguous attribute values for %s\n", type, 0, 0); 390 PR_ASSERT(0); 391 if (freetype) slapi_ch_free_string(&type); 392 continue; 393 } 394 break; Please note that str2entry_fast sets NULL to 'a', at the first time or type has been changed since the previous loop. 264 if((ptype==NULL)||(strcasecmp(type,ptype) != 0)) 265 { 266 slapi_ch_free_string(&ptype); 267 ptype=slapi_ch_strdup(type); 268 nvals = 0; 269 maxvals = 0; 270 del_nvals = 0; 271 del_maxvals = 0; 272 a = NULL; 273 }
Reviewed by Rich (Thanks!!) Pushed to master. $ git merge coverity Updating 85eb921..e9a26dc Fast-forward include/libaccess/aclproto.h | 7 - ldap/servers/plugins/replication/cl5_api.c | 22 +- ldap/servers/plugins/replication/repl5_plugins.c | 2 +- ldap/servers/plugins/replication/repl5_protocol.c | 2 +- ldap/servers/plugins/replication/repl5_total.c | 2 +- ldap/servers/plugins/replication/repl_extop.c | 4 +- ldap/servers/plugins/replication/windows_private.c | 4 +- ldap/servers/plugins/uiduniq/7bit.c | 6 +- ldap/servers/plugins/uiduniq/uid.c | 6 +- ldap/servers/slapd/back-ldbm/dblayer.c | 12 +- ldap/servers/slapd/back-ldbm/vlv.c | 2 +- ldap/servers/slapd/entry.c | 2 +- lib/ldaputil/dbconf.c | 1 + lib/libaccess/acltools.cpp | 768 +------------------- lib/libaccess/oneeval.cpp | 8 +- lib/libaccess/utest/.purify | 19 - lib/libaccess/utest/Makefile | 147 ---- lib/libaccess/utest/acl.dat | 44 -- lib/libaccess/utest/aclfile0 | 87 --- lib/libaccess/utest/aclfile1 | 43 -- lib/libaccess/utest/aclfile10 | 45 -- lib/libaccess/utest/aclfile11 | 43 -- lib/libaccess/utest/aclfile12 | 43 -- lib/libaccess/utest/aclfile13 | 43 -- lib/libaccess/utest/aclfile14 | 43 -- lib/libaccess/utest/aclfile15 | 43 -- lib/libaccess/utest/aclfile16 | 43 -- lib/libaccess/utest/aclfile17 | 43 -- lib/libaccess/utest/aclfile18 | 51 -- lib/libaccess/utest/aclfile19 | 46 -- lib/libaccess/utest/aclfile2 | 43 -- lib/libaccess/utest/aclfile3 | 43 -- lib/libaccess/utest/aclfile4 | 43 -- lib/libaccess/utest/aclfile5 | 43 -- lib/libaccess/utest/aclfile6 | 55 -- lib/libaccess/utest/aclfile7 | 43 -- lib/libaccess/utest/aclfile8 | 43 -- lib/libaccess/utest/aclfile9 | 43 -- lib/libaccess/utest/aclgrp0 | 42 - lib/libaccess/utest/aclgrp1 | 42 - lib/libaccess/utest/aclgrp2 | 42 - lib/libaccess/utest/aclgrp3 | 42 - lib/libaccess/utest/aclgrp4 | 42 - lib/libaccess/utest/acltest.cpp | 794 -------------------- lib/libaccess/utest/onetest.cpp | 77 -- lib/libaccess/utest/shexp.cpp | 331 -------- lib/libaccess/utest/shexp.h | 168 ----- lib/libaccess/utest/test.ref | 217 ------ lib/libaccess/utest/testmain.cpp | 89 --- lib/libaccess/utest/twotest.cpp | 87 --- lib/libaccess/utest/ustubs.cpp | 331 -------- 51 files changed, 51 insertions(+), 4240 deletions(-) delete mode 100644 lib/libaccess/utest/.purify delete mode 100644 lib/libaccess/utest/Makefile delete mode 100644 lib/libaccess/utest/acl.dat delete mode 100644 lib/libaccess/utest/aclfile0 delete mode 100644 lib/libaccess/utest/aclfile1 delete mode 100644 lib/libaccess/utest/aclfile10 delete mode 100644 lib/libaccess/utest/aclfile11 delete mode 100644 lib/libaccess/utest/aclfile12 delete mode 100644 lib/libaccess/utest/aclfile13 delete mode 100644 lib/libaccess/utest/aclfile14 delete mode 100644 lib/libaccess/utest/aclfile15 delete mode 100644 lib/libaccess/utest/aclfile16 delete mode 100644 lib/libaccess/utest/aclfile17 delete mode 100644 lib/libaccess/utest/aclfile18 delete mode 100644 lib/libaccess/utest/aclfile19 delete mode 100644 lib/libaccess/utest/aclfile2 delete mode 100644 lib/libaccess/utest/aclfile3 delete mode 100644 lib/libaccess/utest/aclfile4 delete mode 100644 lib/libaccess/utest/aclfile5 delete mode 100644 lib/libaccess/utest/aclfile6 delete mode 100644 lib/libaccess/utest/aclfile7 delete mode 100644 lib/libaccess/utest/aclfile8 delete mode 100644 lib/libaccess/utest/aclfile9 delete mode 100644 lib/libaccess/utest/aclgrp0 delete mode 100644 lib/libaccess/utest/aclgrp1 delete mode 100644 lib/libaccess/utest/aclgrp2 delete mode 100644 lib/libaccess/utest/aclgrp3 delete mode 100644 lib/libaccess/utest/aclgrp4 delete mode 100644 lib/libaccess/utest/acltest.cpp delete mode 100644 lib/libaccess/utest/onetest.cpp delete mode 100644 lib/libaccess/utest/shexp.cpp delete mode 100644 lib/libaccess/utest/shexp.h delete mode 100644 lib/libaccess/utest/test.ref delete mode 100644 lib/libaccess/utest/testmain.cpp delete mode 100644 lib/libaccess/utest/twotest.cpp delete mode 100644 lib/libaccess/utest/ustubs.cpp $ git push Counting objects: 137, done. Delta compression using up to 4 threads. Compressing objects: 100% (109/109), done. Writing objects: 100% (109/109), 14.14 KiB, done. Total 109 (delta 80), reused 0 (delta 0) To ssh://git.fedorahosted.org/git/389/ds.git 85eb921..e9a26dc master -> master