From 925625b52886d40b50fc631bad8bdc81970f7598 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Fri, 28 Feb 2014 15:49:30 +0100 Subject: [PATCH] dbwrap_ctdb: avoid smbd/ctdb deadlocks: check whether we can work locally in db_ctdb_parse_record() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit If the same process tries to re-lock the same record it has already locked, don't go to the ctdbd again. There are situations where we already have a lock on a record and then do a dbwrap_parse_record() on that record, for instance in locking code: do_lock() -> grabs lock on brl record with brl_get_locks() -> calls brl_lock() -> calls brl_lock_posix or _windows_default() -> calls contend_level2_oplocks_begin() -> calls brl_locks_get_read_only() -> calls dbwrap_parse_record on the same brl record as above In the local (tdb) case, this is not a problem, because identical fcntl locks in the same process don't contend each other, but calling out to ctdb for this lets smbd and ctdb deadlock. db_ctdb_fetch_lock() already correclty checks first whether we can simply try to lock locally. But db_ctdb_parse_record() failed to do so for empty records, i.e. records that only consist of the ctdb record header. (These can be deleted records but can also be freshly created and still empty records.) This patch lets db_ctdb_parse_record() not skip local access for empty records, hence fixing the deadlock. PLAN: In the long run, it would be better to solve this generically on the dbwrap_layer, i.e. root the notion of an already locked record there, and skip any call to the db (tdb or ctdb backend) if we have it. This would also solve the problem for all calls like fetch_locked, parse_record and possibly others. But this is the urgent fix for now. Pair-Programmed-With: Volker Lendecke Signed-off-by: Michael Adam Signed-off-by: Volker Lendecke Tested-by: Björn Baumbach --- source3/lib/dbwrap/dbwrap_ctdb.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c index 87e644dbe5f..09a51ceff88 100644 --- a/source3/lib/dbwrap/dbwrap_ctdb.c +++ b/source3/lib/dbwrap/dbwrap_ctdb.c @@ -109,16 +109,7 @@ static int db_ctdb_ltdb_parser(TDB_DATA key, TDB_DATA data, if (data.dsize < sizeof(struct ctdb_ltdb_header)) { return -1; } - if (data.dsize == sizeof(struct ctdb_ltdb_header)) { - /* - * Making this a separate case that needs fixing - * separately. This is an empty record. ctdbd does not - * distinguish between empty and deleted records. Samba right - * now can live without empty records, so lets treat zero-size - * (i.e. deleted) records as non-existing. - */ - return -1; - } + state->parser( key, (struct ctdb_ltdb_header *)data.dptr, make_tdb_data(data.dptr + sizeof(struct ctdb_ltdb_header), -- 2.11.4.GIT