From 04904b61138a3bb877d535f221cfc65a5a95f335 Mon Sep 17 00:00:00 2001 From: William Brown Date: Jun 02 2017 04:21:59 +0000 Subject: 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. Patch notes: roll up of 9e1a761b45dfc2d28199e39c514ea66764c3f127 d8cccbf70e2cc8b807190214a93d20b20d77db50 5d5b9c617e8c73f3771720c1f6906bddae2612db https://pagure.io/389-ds-base/issue/49273 Author: wibrown Review by: nhosoi, mreynolds (Thanks!) --- 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..33ca329 100644 --- a/ldap/servers/slapd/back-ldbm/dbversion.c +++ b/ldap/servers/slapd/back-ldbm/dbversion.c @@ -101,8 +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 - " - "Could not write to file \"%s\"\n", filename, 0, 0 ); + slapi_log_err(SLAPI_LOG_ERR, "dbversion_write", "Could not write to file \"%s\"\n", filename ); rc= -1; } if(rc==0 && dataversion!=NULL) @@ -111,8 +110,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 - " - "Could not write to file \"%s\"\n", filename, 0, 0 ); + slapi_log_err(SLAPI_LOG_ERR, "dbversion_write", "Could not write to file \"%s\"\n", filename ); rc= -1; } } @@ -160,7 +158,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 +176,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 replacing with a valid DBVERSION file from another DB instance\n"); + 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;