oysteikt/heimdal
0
0
Fork 0
Code Issues 5 Pull Requests Releases Activity

Fix locking issues in DB1 HDB backend.

Browse Source 
Multiple concurrent writers would cause the HDB to become corrupted
as the locking was not sufficient to prevent these sorts of issues
from occurring.  We have changed the locking to obtain the appropriate
kind of lock on database open and to hold that lock until the
database closes.  We need to do this as Berkeley DB 1.85 will cache
information from the database in memory and if if this information
is updated without our knowledge then our later writes will corrupt
the database.  We speculate that there would be issues with a single
writer and reader but did not reproduce them.
This commit is contained in:
Roland C. Dowdeswell
2012-05-18 12:39:08 +01:00
parent 8cdcd53a5a
commit 1f63d6e4dc
1 changed files with 62 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

120
lib/hdb/db.c
View File

@@ -41,11 +41,27 @@
#include <db.h>
#endif
typedef struct {
HDB hdb; /* generic members */
int lock_fd; /* DB-specific */
} DB1_HDB;
static krb5_error_code
DB_close(krb5_context context, HDB *db)
{
DB1_HDB *db1 = (DB1_HDB *)db;
DB *d = (DB*)db->hdb_db;
heim_assert(d != 0, "Closing already closed HDB");
(*d->close)(d);
db->hdb_db = 0;
if (db1->lock_fd >= 0) {
close(db1->lock_fd);
db1->lock_fd = -1;
}
return 0;
}
@@ -63,47 +79,15 @@ DB_destroy(krb5_context context, HDB *db)
static krb5_error_code
DB_lock(krb5_context context, HDB *db, int operation)
{
DB *d = (DB*)db->hdb_db;
int fd = (*d->fd)(d);
krb5_error_code ret;
if (db->lock_count > 0) {
db->lock_count++;
if (db->lock_type == HDB_WLOCK || db->lock_type == operation)
return 0;
}
if(fd < 0) {
krb5_set_error_message(context, HDB_ERR_CANT_LOCK_DB,
"Can't lock database: %s", db->hdb_name);
return HDB_ERR_CANT_LOCK_DB;
}
ret = hdb_lock(fd, operation);
if (ret)
return ret;
db->lock_count++;
return 0;
}
static krb5_error_code
DB_unlock(krb5_context context, HDB *db)
{
DB *d = (DB*)db->hdb_db;
int fd = (*d->fd)(d);
if (db->lock_count > 1) {
db->lock_count--;
return 0;
}
heim_assert(db->lock_count == 1, "HDB lock/unlock sequence does not match");
db->lock_count--;
if(fd < 0) {
krb5_set_error_message(context, HDB_ERR_CANT_LOCK_DB,
"Can't unlock database: %s", db->hdb_name);
return HDB_ERR_CANT_LOCK_DB;
}
return hdb_unlock(fd);
return 0;
}
@@ -116,13 +100,7 @@ DB_seq(krb5_context context, HDB *db,
krb5_data key_data, data;
int code;
code = db->hdb_lock(context, db, HDB_RLOCK);
if(code == -1) {
krb5_set_error_message(context, HDB_ERR_DB_INUSE, "Database %s in use", db->hdb_name);
return HDB_ERR_DB_INUSE;
}
code = (*d->seq)(d, &key, &value, flag);
db->hdb_unlock(context, db); /* XXX check value */
if(code == -1) {
code = errno;
krb5_set_error_message(context, code, "Database %s seq error: %s",
@@ -201,11 +179,7 @@ DB__get(krb5_context context, HDB *db, krb5_data key, krb5_data *reply)
k.data = key.data;
k.size = key.length;
code = db->hdb_lock(context, db, HDB_RLOCK);
if(code)
return code;
code = (*d->get)(d, &k, &v, 0);
db->hdb_unlock(context, db);
if(code < 0) {
code = errno;
krb5_set_error_message(context, code, "Database %s get error: %s",
@@ -233,11 +207,7 @@ DB__put(krb5_context context, HDB *db, int replace,
k.size = key.length;
v.data = value.data;
v.size = value.length;
code = db->hdb_lock(context, db, HDB_WLOCK);
if(code)
return code;
code = (*d->put)(d, &k, &v, replace ? 0 : R_NOOVERWRITE);
db->hdb_unlock(context, db);
if(code < 0) {
code = errno;
krb5_set_error_message(context, code, "Database %s put error: %s",
@@ -259,11 +229,7 @@ DB__del(krb5_context context, HDB *db, krb5_data key)
krb5_error_code code;
k.data = key.data;
k.size = key.length;
code = db->hdb_lock(context, db, HDB_WLOCK);
if(code)
return code;
code = (*d->del)(d, &k, 0);
db->hdb_unlock(context, db);
if(code == 1) {
code = errno;
krb5_set_error_message(context, code, "Database %s put error: %s",
@@ -275,9 +241,45 @@ DB__del(krb5_context context, HDB *db, krb5_data key)
return 0;
}
static DB *
_open_db(char *fn, int flags, int mode, int *fd)
{
#ifndef O_EXLOCK
int op;
int ret;
*fd = open(fn, flags, mode);
if (*fd == -1)
return NULL;
if ((flags & O_ACCMODE) == O_RDONLY)
op = LOCK_SH;
else
op = LOCK_EX;
ret = flock(*fd, op);
if (ret == -1) {
int saved_errno;
saved_errno = errno;
close(*fd);
errno = saved_errno;
return NULL;
}
#else
if ((flags & O_ACCMODE) == O_RDONLY)
flags |= O_SHLOCK;
else
flags |= O_EXLOCK;
#endif
return dbopen(fn, flags, mode, DB_BTREE, NULL);
}
static krb5_error_code
DB_open(krb5_context context, HDB *db, int flags, mode_t mode)
{
DB1_HDB *db1 = (DB1_HDB *)db;
char *fn;
krb5_error_code ret;
@@ -286,16 +288,15 @@ DB_open(krb5_context context, HDB *db, int flags, mode_t mode)
krb5_set_error_message(context, ENOMEM, "malloc: out of memory");
return ENOMEM;
}
db->hdb_db = dbopen(fn, flags, mode, DB_BTREE, NULL);
db->hdb_db = _open_db(fn, flags, mode, &db1->lock_fd);
free(fn);
/* try to open without .db extension */
if(db->hdb_db == NULL && errno == ENOENT)
db->hdb_db = dbopen(db->hdb_name, flags, mode, DB_BTREE, NULL);
db->hdb_db = _open_db(db->hdb_name, flags, mode, &db1->lock_fd);
if(db->hdb_db == NULL) {
ret = errno;
krb5_set_error_message(context, ret, "dbopen (%s): %s",
db->hdb_name, strerror(ret));
return ret;
krb5_set_error_message(context, errno, "dbopen (%s): %s",
db->hdb_name, strerror(errno));
return errno;
}
if((flags & O_ACCMODE) == O_RDONLY)
ret = hdb_check_db_format(context, db);
@@ -319,7 +320,8 @@ krb5_error_code
hdb_db_create(krb5_context context, HDB **db,
const char *filename)
{
*db = calloc(1, sizeof(**db));
DB1_HDB **db1 = (DB1_HDB **)db;
*db = calloc(1, sizeof(**db1)); /* Allocate space for the larger db3 */
if (*db == NULL) {
krb5_set_error_message(context, ENOMEM, "malloc: out of memory");
return ENOMEM;
@@ -350,6 +352,8 @@ hdb_db_create(krb5_context context, HDB **db,
(*db)->hdb__put = DB__put;
(*db)->hdb__del = DB__del;
(*db)->hdb_destroy = DB_destroy;
(*db1)->lock_fd = -1;
return 0;
}