From 1edea26ce9bf71e8d0fe8ea028fc45817a98ea17 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 25 Jan 2013 13:15:51 +1100 Subject: [PATCH 1/7] bug9598: s4-process_single: Use pid,fd as cluster_id in process_single just like process_prefork This avoids two different process single servers (say LDAP and the RPC server) sharing the same server id. Fix-bug: https://bugzilla.samba.org/show_bug.cgi?id=9598 Reported-by: Matthieu Patou Reviewed-by: Matthieu Patou Signed-off-by: Andrew Bartlett Autobuild-User(master): Andrew Bartlett Autobuild-Date(master): Fri Jan 25 12:00:04 CET 2013 on sn-devel-104 (cherry picked from commit c5db4eb9104f1a95220273ee2b0290d157053922) --- source4/smbd/process_single.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/source4/smbd/process_single.c b/source4/smbd/process_single.c index 742eac1..ff67750 100644 --- a/source4/smbd/process_single.c +++ b/source4/smbd/process_single.c @@ -49,6 +49,7 @@ static void single_accept_connection(struct tevent_context *ev, { NTSTATUS status; struct socket_context *connected_socket; + pid_t pid = getpid(); /* accept an incoming connection. */ status = socket_accept(listen_socket, &connected_socket); @@ -71,10 +72,14 @@ static void single_accept_connection(struct tevent_context *ev, talloc_steal(private_data, connected_socket); - /* The cluster_id(0, fd) cannot collide with the incrementing - * task below, as the first component is 0, not 1 */ + /* + * We use the PID so we cannot collide in with cluster ids + * generated in other single mode tasks, and, and won't + * collide with PIDs from process model standard because a the + * combination of pid/fd should be unique system-wide + */ new_conn(ev, lp_ctx, connected_socket, - cluster_id(0, socket_get_fd(connected_socket)), private_data); + cluster_id(pid, socket_get_fd(connected_socket)), private_data); } /* -- 1.7.9.5 From 229c173025ecf0649c64ef1b239970680075e020 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Sat, 26 Jan 2013 08:58:46 +1100 Subject: [PATCH 2/7] pymessaging: Use correct unsigned types for server ID tuple elememnts This is needed if we start using the top bits of these values. Andrew Bartlett Reviewed-by: Matthieu Patou Reviewed-by: Stefan Metzmacher (cherry picked from commit a3054323d3fa1dadff1675e7f8ec672a991d8e56) --- source4/lib/messaging/pymessaging.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source4/lib/messaging/pymessaging.c b/source4/lib/messaging/pymessaging.c index cb79d72..f8703b2 100644 --- a/source4/lib/messaging/pymessaging.c +++ b/source4/lib/messaging/pymessaging.c @@ -51,10 +51,10 @@ static bool server_id_from_py(PyObject *object, struct server_id *server_id) return true; } if (PyTuple_Size(object) == 3) { - return PyArg_ParseTuple(object, "iii", &server_id->pid, &server_id->task_id, &server_id->vnn); + return PyArg_ParseTuple(object, "KII", &server_id->pid, &server_id->task_id, &server_id->vnn); } else { int pid, task_id; - if (!PyArg_ParseTuple(object, "ii", &pid, &task_id)) + if (!PyArg_ParseTuple(object, "KI", &pid, &task_id)) return false; *server_id = cluster_id(pid, task_id); return true; @@ -165,7 +165,7 @@ static void py_msg_callback_wrapper(struct imessaging_context *msg, void *privat { PyObject *callback = (PyObject *)private_data; - PyObject_CallFunction(callback, discard_const_p(char, "i(iii)s#"), msg_type, + PyObject_CallFunction(callback, discard_const_p(char, "i(KII)s#"), msg_type, server_id.pid, server_id.task_id, server_id.vnn, data->data, data->length); } -- 1.7.9.5 From 8354387a5ac7c03a7286bcbe001f3f544dbfca7a Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Sat, 26 Jan 2013 09:09:23 +1100 Subject: [PATCH 3/7] pymessaging: Pass around the server_id struct to python callbacks rather than the tuple This is not used currently, but may avoid going to and from the python types when we do not need to. Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit a321dd3aafa0f6ec8b39cd4fc64146dfbf24ce65) --- source4/lib/messaging/pymessaging.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/source4/lib/messaging/pymessaging.c b/source4/lib/messaging/pymessaging.c index f8703b2..3901d4d 100644 --- a/source4/lib/messaging/pymessaging.c +++ b/source4/lib/messaging/pymessaging.c @@ -163,10 +163,20 @@ static void py_msg_callback_wrapper(struct imessaging_context *msg, void *privat uint32_t msg_type, struct server_id server_id, DATA_BLOB *data) { - PyObject *callback = (PyObject *)private_data; + PyObject *py_server_id, *callback = (PyObject *)private_data; - PyObject_CallFunction(callback, discard_const_p(char, "i(KII)s#"), msg_type, - server_id.pid, server_id.task_id, server_id.vnn, + struct server_id *p_server_id = talloc(NULL, struct server_id); + if (!p_server_id) { + PyErr_NoMemory(); + return; + } + *p_server_id = server_id; + + py_server_id = py_return_ndr_struct("samba.dcerpc.server_id", "server_id", p_server_id, p_server_id); + talloc_unlink(NULL, p_server_id); + + PyObject_CallFunction(callback, discard_const_p(char, "i(O)s#"), msg_type, + py_server_id, data->data, data->length); } -- 1.7.9.5 From b2b91293b9206542aa967070970ca29e8856afb1 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 25 Jan 2013 23:00:12 +1100 Subject: [PATCH 4/7] s4-process_single: Use pid,task_id as cluster_id in process_single just like process_prefork This avoids two different process single task servers (eg the drepl server) sharing the same server id. The task id starts at 2^31 to avoid collision with the fd based scheme for connections. Fix-bug: https://bugzilla.samba.org/show_bug.cgi?id=9598 Reported-by: Matthieu Patou Reviewed-by: Stefan Metzmacher Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Sat Jan 26 16:13:05 CET 2013 on sn-devel-104 (cherry picked from commit b9f1c8887ed1c8c29259021d4f2b9a549caa4061) --- source4/smbd/process_single.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/source4/smbd/process_single.c b/source4/smbd/process_single.c index ff67750..a1b785e 100644 --- a/source4/smbd/process_single.c +++ b/source4/smbd/process_single.c @@ -91,15 +91,20 @@ static void single_new_task(struct tevent_context *ev, void (*new_task)(struct tevent_context *, struct loadparm_context *, struct server_id, void *), void *private_data) { - /* start our taskids at 1, zero is reserved for the top - level samba task */ - static uint32_t taskid = 1; + pid_t pid = getpid(); + /* start our taskids at MAX_INT32, the first 2^31 tasks are is reserved for fd numbers */ + static uint32_t taskid = INT32_MAX; - /* We use 1 so we cannot collide in with cluster ids generated - * in the accept connection above, and unlikly to collide with - * PIDs from process model standard (don't run samba as - * init) */ - new_task(ev, lp_ctx, cluster_id(1, taskid++), private_data); + /* + * We use the PID so we cannot collide in with cluster ids + * generated in other single mode tasks, and, and won't + * collide with PIDs from process model starndard because a the + * combination of pid/task_id should be unique system-wide + * + * Using the pid unaltered makes debugging of which process + * owns the messaging socket easier. + */ + new_task(ev, lp_ctx, cluster_id(pid, taskid++), private_data); } -- 1.7.9.5 From 7875eeec9439249d6d1605166be2950e4b1804a4 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Sun, 27 Jan 2013 12:15:50 +0100 Subject: [PATCH 5/7] selftest: rename 'promoted_vampire_dc' to 'promoted_dc' Unix domain socket are limited to 104 characters on Linux. Using something like this fails as it uses more than 104 characters: '/memdisk/autobuild/flakey/b232141/samba/bin/ab/promoted_vampire_dc/private/smbd.tmp/msg/msg.482379.2147483647' Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 7e7cd07c405f8b66f5979047cb1a50e1e7a55edd) --- selftest/target/Samba4.pm | 18 +++++++++--------- source4/selftest/tests.py | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm index 5988b83..5f1c907 100644 --- a/selftest/target/Samba4.pm +++ b/selftest/target/Samba4.pm @@ -983,7 +983,7 @@ sub provision_rpc_proxy($$$) return $ret; } -sub provision_promoted_vampire_dc($$$) +sub provision_promoted_dc($$$) { my ($self, $prefix, $dcvars) = @_; print "PROVISIONING VAMPIRE DC..."; @@ -1044,9 +1044,9 @@ sub provision_promoted_vampire_dc($$$) return undef; } - $ret->{PROMOTED_VAMPIRE_DC_SERVER} = $ret->{SERVER}; - $ret->{PROMOTED_VAMPIRE_DC_SERVER_IP} = $ret->{SERVER_IP}; - $ret->{PROMOTED_VAMPIRE_DC_NETBIOSNAME} = $ret->{NETBIOSNAME}; + $ret->{PROMOTED_DC_SERVER} = $ret->{SERVER}; + $ret->{PROMOTED_DC_SERVER_IP} = $ret->{SERVER_IP}; + $ret->{PROMOTED_DC_NETBIOSNAME} = $ret->{NETBIOSNAME}; $ret->{DC_SERVER} = $dcvars->{DC_SERVER}; $ret->{DC_SERVER_IP} = $dcvars->{DC_SERVER_IP}; @@ -1620,11 +1620,11 @@ sub setup_env($$$) $self->setup_dc("$path/dc"); } return $self->setup_vampire_dc("$path/vampire_dc", $self->{vars}->{dc}); - } elsif ($envname eq "promoted_vampire_dc") { + } elsif ($envname eq "promoted_dc") { if (not defined($self->{vars}->{dc})) { $self->setup_dc("$path/dc"); } - return $self->setup_promoted_vampire_dc("$path/promoted_vampire_dc", $self->{vars}->{dc}); + return $self->setup_promoted_dc("$path/promoted_dc", $self->{vars}->{dc}); } elsif ($envname eq "subdom_dc") { if (not defined($self->{vars}->{dc})) { $self->setup_dc("$path/dc"); @@ -1818,18 +1818,18 @@ sub setup_vampire_dc($$$) return $env; } -sub setup_promoted_vampire_dc($$$) +sub setup_promoted_dc($$$) { my ($self, $path, $dc_vars) = @_; - my $env = $self->provision_promoted_vampire_dc($path, $dc_vars); + my $env = $self->provision_promoted_dc($path, $dc_vars); if (defined $env) { $self->check_or_start($env, "single"); $self->wait_for_start($env); - $self->{vars}->{promoted_vampire_dc} = $env; + $self->{vars}->{promoted_dc} = $env; # force replicated DC to update repsTo/repsFrom # for vampired partitions diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 58936e8..977b539 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -497,10 +497,10 @@ plantestsuite("samba4.blackbox.provision-backend", "none", ["PYTHON=%s" % python plantestsuite("samba4.blackbox.renamedc.sh", "none", ["PYTHON=%s" % python, os.path.join(bbdir, "renamedc.sh"), '$PREFIX/provision']) # Demote the vampire DC, it must be the last test on the VAMPIRE DC -for env in ['vampire_dc', 'promoted_vampire_dc']: +for env in ['vampire_dc', 'promoted_dc']: plantestsuite("samba4.blackbox.samba_tool_demote(%s)" % env, env, [os.path.join(samba4srcdir, "utils/tests/test_demote.sh"), '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$DOMAIN', '$DC_SERVER', '$PREFIX/%s' % env, smbclient4]) # TODO: Verifying the databases really should be a part of the # environment teardown. # check the databases are all OK. PLEASE LEAVE THIS AS THE LAST TEST -for env in ["dc", "fl2000dc", "fl2003dc", "fl2008r2dc", 'vampire_dc', 'promoted_vampire_dc']: +for env in ["dc", "fl2000dc", "fl2003dc", "fl2008r2dc", 'vampire_dc', 'promoted_dc']: plantestsuite("samba4.blackbox.dbcheck(%s)" % env, env + ":local" , ["PYTHON=%s" % python, os.path.join(bbdir, "dbcheck.sh"), '$PREFIX/provision', configuration]) -- 1.7.9.5 From 22015eee562abfa4f431d1307aeb65f82af7ac0d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Sun, 27 Jan 2013 11:01:07 +0100 Subject: [PATCH 6/7] s4:service_task: prevent a segfault if task->msg_ctx is not initialized yet Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 431692df422c3cac71ca12b7e89296172dfcf684) --- source4/smbd/service_task.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/smbd/service_task.c b/source4/smbd/service_task.c index 4531c3a..9a30cd2 100644 --- a/source4/smbd/service_task.c +++ b/source4/smbd/service_task.c @@ -34,7 +34,7 @@ void task_server_terminate(struct task_server *task, const char *reason, bool fa const struct model_ops *model_ops = task->model_ops; DEBUG(0,("task_server_terminate: [%s]\n", reason)); - if (fatal) { + if (fatal && task->msg_ctx != NULL) { struct dcerpc_binding_handle *irpc_handle; struct samba_terminate r; -- 1.7.9.5 From bac6ff27c5dcea6ee9049359d7bb9cf8592d8fa3 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Sun, 27 Jan 2013 11:09:39 +0100 Subject: [PATCH 7/7] s4:service_task: add missing imessaging_cleanup() to task_server_terminate() Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Sun Jan 27 15:50:30 CET 2013 on sn-devel-104 (cherry picked from commit bb3238b46f0ffaf0bc8c0e16bdcc1cf5d2cad197) --- source4/smbd/service_task.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source4/smbd/service_task.c b/source4/smbd/service_task.c index 9a30cd2..5d14faf 100644 --- a/source4/smbd/service_task.c +++ b/source4/smbd/service_task.c @@ -46,6 +46,8 @@ void task_server_terminate(struct task_server *task, const char *reason, bool fa } } + imessaging_cleanup(task->msg_ctx); + model_ops->terminate(event_ctx, task->lp_ctx, reason); /* don't free this above, it might contain the 'reason' being printed */ -- 1.7.9.5