[SCM] Samba Shared Repository - branch master updated
Michael Adam
obnox at samba.org
Mon Mar 3 08:31:04 MST 2014
The branch, master has been updated
via bbd0bda smbd: Fix an uninitialized memory read
via 7e12bfc pthreadpool: Add test for fork crash
via ccc187f pthreadpool: Fix pthreadpools with fork
via 925625b dbwrap_ctdb: avoid smbd/ctdb deadlocks: check whether we can work locally in db_ctdb_parse_record()
from 7e53506 torture: Fix a torture crash with -O3
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit bbd0bda9c2213215efeb13f5fe527d2409dc79b2
Author: Volker Lendecke <vl at samba.org>
Date: Mon Mar 3 13:49:46 2014 +0100
smbd: Fix an uninitialized memory read
This might be the reason for a few flaky builds.
Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
Autobuild-User(master): Michael Adam <obnox at samba.org>
Autobuild-Date(master): Mon Mar 3 16:30:53 CET 2014 on sn-devel-104
commit 7e12bfc6a8fe97da907525ae90288e37c4fa7a7f
Author: Volker Lendecke <vl at samba.org>
Date: Mon Mar 3 11:57:18 2014 +0000
pthreadpool: Add test for fork crash
Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Kamen Mazdrashki <kamenim at samba.org>
Reviewed-by: Simo Sorce <simo at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
commit ccc187ff5e6cbc411476c5f5e68cc6bb1fe35818
Author: Volker Lendecke <vl at samba.org>
Date: Mon Mar 3 12:20:41 2014 +0100
pthreadpool: Fix pthreadpools with fork
The current could would crash if a pthreadpool was created, deleted and the
process then fork()s. "pthreadpools" is NULL in this case, but the
pthread_atfork handlers are in place. This fixes walking the pthreadpools list
in reverse.
Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Kamen Mazdrashki <kamenim at samba.org>
Reviewed-by: Simo Sorce <simo at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
commit 925625b52886d40b50fc631bad8bdc81970f7598
Author: Michael Adam <obnox at samba.org>
Date: Fri Feb 28 15:49:30 2014 +0100
dbwrap_ctdb: avoid smbd/ctdb deadlocks: check whether we can work locally in db_ctdb_parse_record()
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 <vl at samba.org>
Signed-off-by: Michael Adam <obnox at samba.org>
Signed-off-by: Volker Lendecke <vl at samba.org>
Tested-by: Björn Baumbach <bb at sernet.de>
-----------------------------------------------------------------------
Summary of changes:
source3/lib/dbwrap/dbwrap_ctdb.c | 11 +-------
source3/lib/pthreadpool/pthreadpool.c | 21 ++++----------
source3/lib/pthreadpool/tests.c | 48 +++++++++++++++++++++++++++++++++
source3/locking/brlock.c | 2 +
4 files changed, 57 insertions(+), 25 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index 87e644d..09a51ce 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),
diff --git a/source3/lib/pthreadpool/pthreadpool.c b/source3/lib/pthreadpool/pthreadpool.c
index bd58d62..654d420 100644
--- a/source3/lib/pthreadpool/pthreadpool.c
+++ b/source3/lib/pthreadpool/pthreadpool.c
@@ -188,16 +188,11 @@ static void pthreadpool_parent(void)
int ret;
struct pthreadpool *pool;
- pool = DLIST_TAIL(pthreadpools);
-
- while (1) {
+ for (pool = DLIST_TAIL(pthreadpools);
+ pool != NULL;
+ pool = DLIST_PREV(pool)) {
ret = pthread_mutex_unlock(&pool->mutex);
assert(ret == 0);
-
- if (pool == pthreadpools) {
- break;
- }
- pool = pool->prev;
}
ret = pthread_mutex_unlock(&pthreadpools_mutex);
@@ -209,9 +204,10 @@ static void pthreadpool_child(void)
int ret;
struct pthreadpool *pool;
- pool = DLIST_TAIL(pthreadpools);
+ for (pool = DLIST_TAIL(pthreadpools);
+ pool != NULL;
+ pool = DLIST_PREV(pool)) {
- while (1) {
close(pool->sig_pipe[0]);
close(pool->sig_pipe[1]);
@@ -236,11 +232,6 @@ static void pthreadpool_child(void)
ret = pthread_mutex_unlock(&pool->mutex);
assert(ret == 0);
-
- if (pool == pthreadpools) {
- break;
- }
- pool = pool->prev;
}
ret = pthread_mutex_unlock(&pthreadpools_mutex);
diff --git a/source3/lib/pthreadpool/tests.c b/source3/lib/pthreadpool/tests.c
index 95d37b6..170cedf 100644
--- a/source3/lib/pthreadpool/tests.c
+++ b/source3/lib/pthreadpool/tests.c
@@ -5,6 +5,8 @@
#include <stdlib.h>
#include <pthread.h>
#include <unistd.h>
+#include <sys/types.h>
+#include <sys/wait.h>
#include "pthreadpool.h"
static int test_init(void)
@@ -318,6 +320,46 @@ static int test_threaded_addjob(int num_pools, int num_threads, int poolsize,
return 0;
}
+static int test_fork(void)
+{
+ struct pthreadpool *p;
+ pid_t child, waited;
+ int status, ret;
+
+ ret = pthreadpool_init(1, &p);
+ if (ret != 0) {
+ fprintf(stderr, "pthreadpool_init failed: %s\n",
+ strerror(ret));
+ return -1;
+ }
+ ret = pthreadpool_destroy(p);
+ if (ret != 0) {
+ fprintf(stderr, "pthreadpool_destroy failed: %s\n",
+ strerror(ret));
+ return -1;
+ }
+
+ child = fork();
+ if (child < 0) {
+ perror("fork failed");
+ return -1;
+ }
+ if (child == 0) {
+ exit(0);
+ }
+ waited = wait(&status);
+ if (waited == -1) {
+ perror("wait failed");
+ return -1;
+ }
+ if (waited != child) {
+ fprintf(stderr, "expected child %d, got %d\n",
+ (int)child, (int)waited);
+ return -1;
+ }
+ return 0;
+}
+
int main(void)
{
int ret;
@@ -328,6 +370,12 @@ int main(void)
return 1;
}
+ ret = test_fork();
+ if (ret != 0) {
+ fprintf(stderr, "test_fork failed\n");
+ return 1;
+ }
+
ret = test_jobs(10, 10000);
if (ret != 0) {
fprintf(stderr, "test_jobs failed\n");
diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c
index e6c8949..ac22ba4 100644
--- a/source3/locking/brlock.c
+++ b/source3/locking/brlock.c
@@ -2085,6 +2085,8 @@ static void brl_get_locks_readonly_parser(TDB_DATA key, TDB_DATA data,
if ((data.dsize % sizeof(struct lock_struct)) == 1) {
br_lock->have_read_oplocks = (data.dptr[data.dsize-1] == 1);
+ } else {
+ br_lock->have_read_oplocks = false;
}
DEBUG(10, ("Got %d bytes, have_read_oplocks: %s\n", (int)data.dsize,
--
Samba Shared Repository
More information about the samba-cvs
mailing list