#50671 Ticket 50659 AddressSanitizer: SEGV ... in bdb_pre_close
Closed 3 years ago by spichugi. Opened 4 years ago by lkrispen.
lkrispen/389-ds-base t50659  into  master

@@ -2499,9 +2499,6 @@ 

              import_log_notice(job, SLAPI_LOG_WARNING, "import_main_offline", "Failed to close database");

          }

      }

-     if (!(job->flags & FLAG_ONLINE))

-         dblayer_close(job->inst->inst_li, DBLAYER_IMPORT_MODE);

- 

      end = slapi_current_utc_time();

      if (verbose && (0 == ret)) {

          int seconds_to_import = end - beginning;

@@ -1073,7 +1073,7 @@ 

                      }

                      /* dn is not dup'ed in slapi_sdn_new_dn_byref.

                       * It's set to bdn and put in the dn cache. */

-                     sdn = slapi_sdn_new_normdn_byref(normdn);

+                     sdn = slapi_sdn_new_normdn_byval((const char *)normdn);

                      bdn = backdn_init(sdn, temp_id, 0);

                      CACHE_ADD(&inst->inst_dncache, bdn, NULL);

                      CACHE_RETURN(&inst->inst_dncache, &bdn);
@@ -1085,6 +1085,7 @@ 

                  e = slapi_str2entry_ext(normdn, NULL, data.dptr,

                                          SLAPI_STR2ENTRY_NO_ENTRYDN);

                  slapi_ch_free_string(&rdn);

+                 slapi_ch_free_string(&normdn);

              }

          } else {

              e = slapi_str2entry(data.data, 0);

@@ -1929,7 +1929,7 @@ 

      }

      /* Don't free priv->bdb_data_directories since priv doesn't own the memory */

      slapi_ch_free((void **)&conf);

-     slapi_ch_free((void **)&mypEnv);

+     bdb_free_env((void **)&mypEnv);

      if (inst_dirp != inst_dir)

          slapi_ch_free_string(&inst_dirp);

      return rval;
@@ -1957,10 +1957,12 @@ 

      }

  

  done:

-     if (pDB)

+     if (pDB) {

          pDB->close(pDB, 0);

-     if (pEnv)

+     }

+     if (pEnv) {

          pEnv->close(pEnv, 0);

+     }

      if (envdir) {

          ldbm_delete_dirs(envdir);

          slapi_ch_free_string(&envdir);

@@ -20,6 +20,7 @@ 

  {

      struct ldbminfo *li = NULL;

      slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &li);

+     dblayer_setup(li);

      dblayer_private *priv = (dblayer_private *)li->li_dblayer_private;

  

      return priv->dblayer_verify_fn(pb);;

@@ -2634,6 +2634,7 @@ 

                        backend_plugin->plg_name);

          return_value = -1;

      }

+     charray_free(mcfg->cmd_line_instance_names);

      slapi_pblock_destroy(pb);

  

      return (return_value);

Bug: The crash reported is caused by calling dblayer_close twice in some
offline exec modes. Investigating the crash revealed another crash
in dbverify and memory leaks, one introduced by the backend
patch, two existing previously

Fix: - call dblayer_close only once
- initialize db env properly in dbverify execmode
- don't set sdn by reference when adding to entrydncache
- free collected instances from commandline in dbupgrade mode
- free bdb env in index mode

Reviewed by: ?

rebased onto 4adca4e7d4ac24611b1a209f9fb49b302c5f3176

4 years ago

Looks reasonable to me, my only question is about the removal of dblayer_close()? Is this closed elsewhere?

Looks reasonable to me, my only question is about the removal of dblayer_close()? Is this closed elsewhere?

yes. and that has caused the crash, trying to free it twice. In the reported scenario it is also called in bdb_upgradedb() and in general it is called above import_main_offline()

Ack from me then, this all makes sense.

rebased onto b77f04a

4 years ago

Pull-Request has been merged by lkrispen

4 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3726

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

3 years ago