[389-commits] [389-ds-base] 01/01: Ticket 49273 - crash when DBVERSIOn is corrupt.

This is an automated email from the git hooks/post-receive script.

firstyear pushed a commit to branch master
in repository 389-ds-base.

commit 9e1a761b45dfc2d28199e39c514ea66764c3f127
Author: William Brown <firstyear@redhat.com>
Date: Wed May 31 11:17:47 2017 +1000

Ticket 49273 - crash when DBVERSIOn is corrupt.

Bug Description: During a disk full DBVERSIOn was corrupted.
AS a result, we would segfault.

Fix Description: Rather than segfaulting, we should handle the
error gracefully, with steps to resolve.

https://pagure.io/389-ds-base/issue/49273

Author: wibrown

Review by: nhosoi (Thank you very much!)
---
dirsrvtests/tests/tickets/ticket49273_test.py | 50 +++++++++++++++++++++++++++
ldap/servers/slapd/back-ldbm/dbversion.c | 17 +++++++--
ldap/servers/slapd/back-ldbm/ldif2ldbm.c | 5 +--
ldap/servers/slapd/back-ldbm/upgrade.c | 14 +++++---
4 files changed, 77 insertions(+), 9 deletions(-)

diff --git a/dirsrvtests/tests/tickets/ticket49273_test.py b/dirsrvtests/tests/tickets/ticket49273_test.py
new file mode 100644
index 0000000..c50aaee
--- /dev/null
+++ b/dirsrvtests/tests/tickets/ticket49273_test.py
@@ -0,0 +1,50 @@
+# --- BEGIN COPYRIGHT BLOCK ---
+# Copyright (C) 2017 Red Hat, Inc.
+# All rights reserved.
+#
+# License: GPL (version 3 or any later version).
+# See LICENSE for details.
+# --- END COPYRIGHT BLOCK ---
+#
+
+import pytest
+import ldap
+
+from lib389.topologies import topology_st
+# This pulls in logging I think
+from lib389.utils import *
+from lib389.sasl import PlainSASL
+from lib389.idm.services import ServiceAccounts
+
+log = logging.getLogger(__name__)
+
+def test_49273_corrupt_dbversion(topology_st):
+ """
+ ticket 49273 was caused by a disk space full, which corrupted
+ the users DBVERSION files. We can't prevent this, but we can handle
+ the error better than "crash".
+ """
+
+ standalone = topology_st.standalone
+
+ # Stop the instance
+ standalone.stop()
+ # Corrupt userRoot dbversion
+ dbvf = os.path.join(standalone.ds_paths.db_dir, 'userRoot/DBVERSION')
+ with open(dbvf, 'w') as f:
+ # This will trunc the file
+ f.write('')
+ # Start up
+ try:
+ # post_open false, means ds state is OFFLINE, which allows
+ # dspaths below to use defaults rather than ldap check.
+ standalone.start(timeout=20, post_open=False)
+ except:
+ pass
+ # Trigger an update of the running server state, to move it OFFLINE.
+ standalone.status()
+
+ # CHeck error log?
+ error_lines = standalone.ds_error_log.match('.*Could not parse file.*')
+ assert(len(error_lines) > 0)
+
diff --git a/ldap/servers/slapd/back-ldbm/dbversion.c b/ldap/servers/slapd/back-ldbm/dbversion.c
index a29fd4e..d4cc806 100644
--- a/ldap/servers/slapd/back-ldbm/dbversion.c
+++ b/ldap/servers/slapd/back-ldbm/dbversion.c
@@ -101,7 +101,7 @@ dbversion_write(struct ldbminfo *li, const char *directory,
len = strlen(buf);
if ( slapi_write_buffer( prfd, buf, len ) != (PRInt32)len )
{
- slapi_log_err(SLAPI_LOG_ERR, "dbversion_write - "
+ slapi_log_err(SLAPI_LOG_ERR, "dbversion_write",
"Could not write to file \"%s\"\n", filename, 0, 0 );
rc= -1;
}
@@ -111,7 +111,7 @@ dbversion_write(struct ldbminfo *li, const char *directory,
len = strlen( buf );
if ( slapi_write_buffer( prfd, buf, len ) != (PRInt32)len )
{
- slapi_log_err(SLAPI_LOG_ERR, "dbversion_write - "
+ slapi_log_err(SLAPI_LOG_ERR, "dbversion_write",
"Could not write to file \"%s\"\n", filename, 0, 0 );
rc= -1;
}
@@ -160,7 +160,7 @@ dbversion_read(struct ldbminfo *li, const char *directory,
/* File missing... we are probably creating a new database. */
return EACCES;
} else {
- char buf[LDBM_VERSION_MAXBUF];
+ char buf[LDBM_VERSION_MAXBUF] = {0};
PRInt32 nr = slapi_read_buffer(prfd, buf, (PRInt32)LDBM_VERSION_MAXBUF-1);
if ( nr > 0 && nr != (PRInt32)LDBM_VERSION_MAXBUF-1 )
{
@@ -178,6 +178,17 @@ dbversion_read(struct ldbminfo *li, const char *directory,
}
}
(void)PR_Close( prfd );
+
+ if (*ldbmversion == NULL || *dataversion == NULL) {
+ /* DBVERSIOn is corrupt, COMPLAIN! */
+ /* This is IDRM Identifier removed (POSIX.1)
+ * which seems appropriate for the error here :)
+ */
+ slapi_log_err(SLAPI_LOG_CRIT, "dbversion_read", "Could not parse file \"%s\". It may be corrupted.\n", filename);
+ slapi_log_err(SLAPI_LOG_CRIT, "dbversion_read", "It may be possible to recover by replicing with a valid DBVERSION file from another DB");
+ return EIDRM;
+ }
+
return 0;
}
}
diff --git a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c
index 3fd604e..6d897ed 100644
--- a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c
+++ b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c
@@ -3644,6 +3644,7 @@ int ldbm_back_upgradednformat(Slapi_PBlock *pb)
char *ldbmversion = NULL;
char *dataversion = NULL;
int ud_flags = 0;
+ int result = 0;

slapi_pblock_get(pb, SLAPI_TASK_FLAGS, &task_flags);
slapi_pblock_get(pb, SLAPI_BACKEND_TASK, &task);
@@ -3723,8 +3724,8 @@ int ldbm_back_upgradednformat(Slapi_PBlock *pb)

workdbdir = rel2abspath(rawworkdbdir);

- dbversion_read(li, workdbdir, &ldbmversion, &dataversion);
- if (ldbmversion) {
+ result = dbversion_read(li, workdbdir, &ldbmversion, &dataversion);
+ if (result == 0 && ldbmversion) {
char *ptr = PL_strstr(ldbmversion, BDB_DNFORMAT);
if (ptr) {
/* DN format is RFC 4514 compliant */
diff --git a/ldap/servers/slapd/back-ldbm/upgrade.c b/ldap/servers/slapd/back-ldbm/upgrade.c
index c7e283f..d90383b 100644
--- a/ldap/servers/slapd/back-ldbm/upgrade.c
+++ b/ldap/servers/slapd/back-ldbm/upgrade.c
@@ -133,12 +133,15 @@ int
check_db_version( struct ldbminfo *li, int *action )
{
int value = 0;
+ int result = 0;
char *ldbmversion = NULL;
char *dataversion = NULL;

*action = 0;
- dbversion_read(li, li->li_directory, &ldbmversion, &dataversion);
- if (NULL == ldbmversion || '\0' == *ldbmversion) {
+ result = dbversion_read(li, li->li_directory, &ldbmversion, &dataversion);
+ if (result != 0) {
+ return 0;
+ } else if (NULL == ldbmversion || '\0' == *ldbmversion) {
slapi_ch_free_string(&ldbmversion);
slapi_ch_free_string(&dataversion);
return 0;
@@ -215,14 +218,17 @@ check_db_inst_version( ldbm_instance *inst )
char *ldbmversion = NULL;
char *dataversion = NULL;
int rval = 0;
+ int result = 0;
char inst_dir[MAXPATHLEN*2];
char *inst_dirp = NULL;

inst_dirp =
dblayer_get_full_inst_dir(inst->inst_li, inst, inst_dir, MAXPATHLEN*2);

- dbversion_read(inst->inst_li, inst_dirp, &ldbmversion, &dataversion);
- if (NULL == ldbmversion || '\0' == *ldbmversion) {
+ result = dbversion_read(inst->inst_li, inst_dirp, &ldbmversion, &dataversion);
+ if (result != 0) {
+ return rval;
+ } else if (NULL == ldbmversion || '\0' == *ldbmversion) {
slapi_ch_free_string(&ldbmversion);
slapi_ch_free_string(&dataversion);
return rval;

--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
_______________________________________________
389-commits mailing list -- 389-commits@lists.fedoraproject.org
To unsubscribe send an email to 389-commits-leave@lists.fedoraproject.org