[PATCH] Remove use of talloc_autofree_context() from source4/smbd/server.c
Volker Lendecke
vl at samba.org
Tue Apr 11 06:02:15 UTC 2017
On Mon, Apr 10, 2017 at 08:11:44PM -0700, Jeremy Allison via samba-technical wrote:
> Whilst fixing the previous bug in reinitializing
> s4 imessaging after fork(), I thought there should
> be a way to move the /bin/samba event context off
> the talloc_autofree context, which was causing all
> the trouble.
Cool stuff :-)
Attached find a patchset with a few to-squash patches and a question.
No formal review yet, but some minor cleanups (IMHO at least).
Volker
-------------- next part --------------
From 2e2c6937f56d08bbf2348b126b1d55e8aba7689a Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 31 Mar 2017 11:07:35 -0700
Subject: [PATCH 01/15] s4: messaging. Minor cleanup. Check for error returns
on imessaging_register calls.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source4/lib/messaging/messaging.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/source4/lib/messaging/messaging.c b/source4/lib/messaging/messaging.c
index 844466d..a1c790f 100644
--- a/source4/lib/messaging/messaging.c
+++ b/source4/lib/messaging/messaging.c
@@ -322,6 +322,7 @@ struct imessaging_context *imessaging_init(TALLOC_CTX *mem_ctx,
struct server_id server_id,
struct tevent_context *ev)
{
+ NTSTATUS status;
struct imessaging_context *msg;
bool ok;
int ret;
@@ -393,11 +394,26 @@ struct imessaging_context *imessaging_init(TALLOC_CTX *mem_ctx,
goto fail;
}
- imessaging_register(msg, NULL, MSG_PING, ping_message);
- imessaging_register(msg, NULL, MSG_REQ_POOL_USAGE, pool_message);
- imessaging_register(msg, NULL, MSG_IRPC, irpc_handler);
- imessaging_register(msg, NULL, MSG_REQ_RINGBUF_LOG, ringbuf_log_msg);
- IRPC_REGISTER(msg, irpc, IRPC_UPTIME, irpc_uptime, msg);
+ status = imessaging_register(msg, NULL, MSG_PING, ping_message);
+ if (!NT_STATUS_IS_OK(status)) {
+ goto fail;
+ }
+ status = imessaging_register(msg, NULL, MSG_REQ_POOL_USAGE, pool_message);
+ if (!NT_STATUS_IS_OK(status)) {
+ goto fail;
+ }
+ status = imessaging_register(msg, NULL, MSG_IRPC, irpc_handler);
+ if (!NT_STATUS_IS_OK(status)) {
+ goto fail;
+ }
+ status = imessaging_register(msg, NULL, MSG_REQ_RINGBUF_LOG, ringbuf_log_msg);
+ if (!NT_STATUS_IS_OK(status)) {
+ goto fail;
+ }
+ status = IRPC_REGISTER(msg, irpc, IRPC_UPTIME, irpc_uptime, msg);
+ if (!NT_STATUS_IS_OK(status)) {
+ goto fail;
+ }
DLIST_ADD(msg_ctxs, msg);
--
2.1.4
From 4572803efc39729e846e35db86009cc6b4eff5ae Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 11 Apr 2017 07:29:47 +0200
Subject: [PATCH 02/15] SQ: 80 cols
---
source4/lib/messaging/messaging.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/source4/lib/messaging/messaging.c b/source4/lib/messaging/messaging.c
index a1c790f..19873c5 100644
--- a/source4/lib/messaging/messaging.c
+++ b/source4/lib/messaging/messaging.c
@@ -398,7 +398,8 @@ struct imessaging_context *imessaging_init(TALLOC_CTX *mem_ctx,
if (!NT_STATUS_IS_OK(status)) {
goto fail;
}
- status = imessaging_register(msg, NULL, MSG_REQ_POOL_USAGE, pool_message);
+ status = imessaging_register(msg, NULL, MSG_REQ_POOL_USAGE,
+ pool_message);
if (!NT_STATUS_IS_OK(status)) {
goto fail;
}
@@ -406,7 +407,8 @@ struct imessaging_context *imessaging_init(TALLOC_CTX *mem_ctx,
if (!NT_STATUS_IS_OK(status)) {
goto fail;
}
- status = imessaging_register(msg, NULL, MSG_REQ_RINGBUF_LOG, ringbuf_log_msg);
+ status = imessaging_register(msg, NULL, MSG_REQ_RINGBUF_LOG,
+ ringbuf_log_msg);
if (!NT_STATUS_IS_OK(status)) {
goto fail;
}
--
2.1.4
From 7c0205f3e70c936649d538457e1ed4fdeab03141 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 31 Mar 2017 11:43:17 -0700
Subject: [PATCH 03/15] s4: server. Whitespace and 80+ column cleanup.
No logic changes.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source4/smbd/server.c | 149 ++++++++++++++++++++++++++++++++------------------
1 file changed, 97 insertions(+), 52 deletions(-)
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index b8545b8..4242fe6 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -1,4 +1,4 @@
-/*
+/*
Unix SMB/CIFS implementation.
Main SMB server routines
@@ -7,17 +7,17 @@
Copyright (C) Martin Pool 2002
Copyright (C) Jelmer Vernooij 2002
Copyright (C) James J Myers 2003 <myersjj at samba.org>
-
+
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 3 of the License, or
(at your option) any later version.
-
+
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
-
+
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
@@ -75,7 +75,7 @@ static void recursive_delete(const char *path)
continue;
}
if (unlink(fname) != 0) {
- DEBUG(0,("Unabled to delete '%s' - %s\n",
+ DEBUG(0,("Unabled to delete '%s' - %s\n",
fname, strerror(errno)));
smb_panic("unable to cleanup tmp files");
}
@@ -89,7 +89,7 @@ static void recursive_delete(const char *path)
TDB_CLEAR_IF_FIRST. Unfortunately TDB_CLEAR_IF_FIRST is not
efficient on unix systems due to the lack of scaling of the byte
range locking system. So instead of putting the burden on tdb to
- cleanup tmp files, this function deletes them.
+ cleanup tmp files, this function deletes them.
*/
static void cleanup_tmp_files(struct loadparm_context *lp_ctx)
{
@@ -143,7 +143,8 @@ static void setup_signals(void)
#endif
/* POSIX demands that signals are inherited. If the invoking process has
- * these signals masked, we will have problems, as we won't receive them. */
+ * these signals masked, we will have problems,
+ * as we won't receive them. */
BlockSignals(false, SIGHUP);
BlockSignals(false, SIGTERM);
@@ -154,16 +155,20 @@ static void setup_signals(void)
/*
handle io on stdin
*/
-static void server_stdin_handler(struct tevent_context *event_ctx, struct tevent_fd *fde,
- uint16_t flags, void *private_data)
+static void server_stdin_handler(struct tevent_context *event_ctx,
+ struct tevent_fd *fde,
+ uint16_t flags,
+ void *private_data)
{
const char *binary_name = (const char *)private_data;
uint8_t c;
if (read(0, &c, 1) == 0) {
- DEBUG(0,("%s: EOF on stdin - PID %d terminating\n", binary_name, (int)getpid()));
+ DEBUG(0,("%s: EOF on stdin - PID %d terminating\n",
+ binary_name, (int)getpid()));
#if HAVE_GETPGRP
if (getpgrp() == getpid()) {
- DEBUG(0,("Sending SIGTERM from pid %d\n", (int)getpid()));
+ DEBUG(0,("Sending SIGTERM from pid %d\n",
+ (int)getpid()));
kill(-getpgrp(), SIGTERM);
}
#endif
@@ -174,13 +179,17 @@ static void server_stdin_handler(struct tevent_context *event_ctx, struct tevent
/*
die if the user selected maximum runtime is exceeded
*/
-_NORETURN_ static void max_runtime_handler(struct tevent_context *ev,
- struct tevent_timer *te,
+_NORETURN_ static void max_runtime_handler(struct tevent_context *ev,
+ struct tevent_timer *te,
struct timeval t, void *private_data)
{
const char *binary_name = (const char *)private_data;
- DEBUG(0,("%s: maximum runtime exceeded - terminating PID %d at %llu, current ts: %llu\n",
- binary_name, (int)getpid(), (unsigned long long)t.tv_sec, (unsigned long long) time(NULL)));
+ DEBUG(0,("%s: maximum runtime exceeded - "
+ "terminating PID %d at %llu, current ts: %llu\n",
+ binary_name,
+ (int)getpid(),
+ (unsigned long long)t.tv_sec,
+ (unsigned long long)time(NULL)));
exit(0);
}
@@ -193,7 +202,11 @@ static void prime_ldb_databases(struct tevent_context *event_ctx)
TALLOC_CTX *db_context;
db_context = talloc_new(event_ctx);
- samdb_connect(db_context, event_ctx, cmdline_lp_ctx, system_session(cmdline_lp_ctx), 0);
+ samdb_connect(db_context,
+ event_ctx,
+ cmdline_lp_ctx,
+ system_session(cmdline_lp_ctx),
+ 0);
privilege_connect(db_context, cmdline_lp_ctx);
/* we deliberately leave these open, which allows them to be
@@ -204,7 +217,7 @@ static void prime_ldb_databases(struct tevent_context *event_ctx)
/*
called when a fatal condition occurs in a child task
*/
-static NTSTATUS samba_terminate(struct irpc_message *msg,
+static NTSTATUS samba_terminate(struct irpc_message *msg,
struct samba_terminate *r)
{
DEBUG(0,("samba_terminate of %d: %s\n",
@@ -215,7 +228,7 @@ static NTSTATUS samba_terminate(struct irpc_message *msg,
/*
setup messaging for the top level samba (parent) task
*/
-static NTSTATUS setup_parent_messaging(struct tevent_context *event_ctx,
+static NTSTATUS setup_parent_messaging(struct tevent_context *event_ctx,
struct loadparm_context *lp_ctx)
{
struct imessaging_context *msg;
@@ -277,7 +290,9 @@ static void show_build(void)
printf("Paths:\n");
for (i=0; config_options[i].name; i++) {
- printf(" %s: %s\n", config_options[i].name, config_options[i].value);
+ printf(" %s: %s\n",
+ config_options[i].name,
+ config_options[i].value);
}
exit(0);
@@ -292,7 +307,9 @@ static int event_ctx_destructor(struct tevent_context *event_ctx)
/*
main server.
*/
-static int binary_smbd_main(const char *binary_name, int argc, const char *argv[])
+static int binary_smbd_main(const char *binary_name,
+ int argc,
+ const char *argv[])
{
bool opt_daemon = false;
bool opt_interactive = false;
@@ -320,11 +337,13 @@ static int binary_smbd_main(const char *binary_name, int argc, const char *argv[
"Become a daemon (default)", NULL },
{"interactive", 'i', POPT_ARG_NONE, NULL, OPT_INTERACTIVE,
"Run interactive (not a daemon)", NULL},
- {"model", 'M', POPT_ARG_STRING, NULL, OPT_PROCESS_MODEL,
+ {"model", 'M', POPT_ARG_STRING, NULL, OPT_PROCESS_MODEL,
"Select process model", "MODEL"},
- {"maximum-runtime",0, POPT_ARG_INT, &max_runtime, 0,
- "set maximum runtime of the server process, till autotermination", "seconds"},
- {"show-build", 'b', POPT_ARG_NONE, NULL, OPT_SHOW_BUILD, "show build info", NULL },
+ {"maximum-runtime",0, POPT_ARG_INT, &max_runtime, 0,
+ "set maximum runtime of the server process, "
+ "till autotermination", "seconds"},
+ {"show-build", 'b', POPT_ARG_NONE, NULL, OPT_SHOW_BUILD,
+ "show build info", NULL },
POPT_COMMON_SAMBA
POPT_COMMON_VERSION
{ NULL }
@@ -355,7 +374,8 @@ static int binary_smbd_main(const char *binary_name, int argc, const char *argv[
if (opt_daemon && opt_interactive) {
fprintf(stderr,"\nERROR: "
- "Option -i|--interactive is not allowed together with -D|--daemon\n\n");
+ "Option -i|--interactive is "
+ "not allowed together with -D|--daemon\n\n");
poptPrintUsage(pc, stderr, 0);
return 1;
} else if (!opt_interactive) {
@@ -374,13 +394,22 @@ static int binary_smbd_main(const char *binary_name, int argc, const char *argv[
so set our umask to 0 */
umask(0);
- DEBUG(0,("%s version %s started.\n", binary_name, SAMBA_VERSION_STRING));
- DEBUGADD(0,("Copyright Andrew Tridgell and the Samba Team 1992-2017\n"));
-
- if (sizeof(uint16_t) < 2 || sizeof(uint32_t) < 4 || sizeof(uint64_t) < 8) {
- DEBUG(0,("ERROR: Samba is not configured correctly for the word size on your machine\n"));
- DEBUGADD(0,("sizeof(uint16_t) = %u, sizeof(uint32_t) %u, sizeof(uint64_t) = %u\n",
- (unsigned int)sizeof(uint16_t), (unsigned int)sizeof(uint32_t), (unsigned int)sizeof(uint64_t)));
+ DEBUG(0,("%s version %s started.\n",
+ binary_name,
+ SAMBA_VERSION_STRING));
+ DEBUGADD(0,("Copyright Andrew Tridgell and the Samba Team"
+ " 1992-2017\n"));
+
+ if (sizeof(uint16_t) < 2 ||
+ sizeof(uint32_t) < 4 ||
+ sizeof(uint64_t) < 8) {
+ DEBUG(0,("ERROR: Samba is not configured correctly "
+ "for the word size on your machine\n"));
+ DEBUGADD(0,("sizeof(uint16_t) = %u, sizeof(uint32_t) %u, "
+ "sizeof(uint64_t) = %u\n",
+ (unsigned int)sizeof(uint16_t),
+ (unsigned int)sizeof(uint32_t),
+ (unsigned int)sizeof(uint64_t)));
return 1;
}
@@ -398,19 +427,22 @@ static int binary_smbd_main(const char *binary_name, int argc, const char *argv[
pidfile_create(lpcfg_pid_directory(cmdline_lp_ctx), binary_name);
if (lpcfg_server_role(cmdline_lp_ctx) == ROLE_ACTIVE_DIRECTORY_DC) {
- if (!open_schannel_session_store(talloc_autofree_context(), cmdline_lp_ctx)) {
- exit_daemon("Samba cannot open schannel store for secured NETLOGON operations.", EACCES);
+ if (!open_schannel_session_store(talloc_autofree_context(),
+ cmdline_lp_ctx)) {
+ exit_daemon("Samba cannot open schannel store "
+ "for secured NETLOGON operations.", EACCES);
}
}
/* make sure we won't go through nss_winbind */
if (!winbind_off()) {
- exit_daemon("Samba failed to disable recusive winbindd calls.", EACCES);
+ exit_daemon("Samba failed to disable recusive "
+ "winbindd calls.", EACCES);
}
gensec_init(); /* FIXME: */
- process_model_init(cmdline_lp_ctx);
+ process_model_init(cmdline_lp_ctx);
shared_init = load_samba_modules(NULL, "service");
@@ -418,7 +450,7 @@ static int binary_smbd_main(const char *binary_name, int argc, const char *argv[
run_init_functions(shared_init);
talloc_free(shared_init);
-
+
/* the event context is the top level structure in smbd. Everything else
should hang off that */
event_ctx = s4_event_context_init(talloc_autofree_context());
@@ -443,7 +475,8 @@ static int binary_smbd_main(const char *binary_name, int argc, const char *argv[
#endif
if (fstat(0, &st) != 0) {
- exit_daemon("Samba failed to set standard input handler", ENOTTY);
+ exit_daemon("Samba failed to set standard input handler",
+ ENOTTY);
}
if (S_ISFIFO(st.st_mode) || S_ISSOCK(st.st_mode)) {
@@ -456,9 +489,10 @@ static int binary_smbd_main(const char *binary_name, int argc, const char *argv[
}
if (max_runtime) {
- DEBUG(0,("%s PID %d was called with maxruntime %d - current ts %llu\n",
- binary_name, (int)getpid(),
- max_runtime, (unsigned long long) time(NULL)));
+ DEBUG(0,("%s PID %d was called with maxruntime %d - "
+ "current ts %llu\n",
+ binary_name, (int)getpid(),
+ max_runtime, (unsigned long long) time(NULL)));
tevent_add_timer(event_ctx, event_ctx,
timeval_current_ofs(max_runtime, 0),
max_runtime_handler,
@@ -466,29 +500,40 @@ static int binary_smbd_main(const char *binary_name, int argc, const char *argv[
}
if (lpcfg_server_role(cmdline_lp_ctx) != ROLE_ACTIVE_DIRECTORY_DC
- && !lpcfg_parm_bool(cmdline_lp_ctx, NULL, "server role check", "inhibit", false)
- && !str_list_check_ci(lpcfg_server_services(cmdline_lp_ctx), "smb")
- && !str_list_check_ci(lpcfg_dcerpc_endpoint_servers(cmdline_lp_ctx), "remote")
- && !str_list_check_ci(lpcfg_dcerpc_endpoint_servers(cmdline_lp_ctx), "mapiproxy")) {
- DEBUG(0, ("At this time the 'samba' binary should only be used for either:\n"));
- DEBUGADD(0, ("'server role = active directory domain controller' or to access the ntvfs file server with 'server services = +smb' or the rpc proxy with 'dcerpc endpoint servers = remote'\n"));
- DEBUGADD(0, ("You should start smbd/nmbd/winbindd instead for domain member and standalone file server tasks\n"));
- exit_daemon("Samba detected misconfigured 'server role' and exited. Check logs for details", EINVAL);
+ && !lpcfg_parm_bool(cmdline_lp_ctx, NULL,
+ "server role check", "inhibit", false)
+ && !str_list_check_ci(lpcfg_server_services(cmdline_lp_ctx), "smb")
+ && !str_list_check_ci(lpcfg_dcerpc_endpoint_servers(cmdline_lp_ctx),
+ "remote")
+ && !str_list_check_ci(lpcfg_dcerpc_endpoint_servers(cmdline_lp_ctx),
+ "mapiproxy")) {
+ DEBUG(0, ("At this time the 'samba' binary should only be used "
+ "for either:\n"));
+ DEBUGADD(0, ("'server role = active directory domain "
+ "controller' or to access the ntvfs file server "
+ "with 'server services = +smb' or the rpc proxy "
+ "with 'dcerpc endpoint servers = remote'\n"));
+ DEBUGADD(0, ("You should start smbd/nmbd/winbindd instead for "
+ "domain member and standalone file server tasks\n"));
+ exit_daemon("Samba detected misconfigured 'server role' "
+ "and exited. Check logs for details", EINVAL);
};
prime_ldb_databases(event_ctx);
status = setup_parent_messaging(event_ctx, cmdline_lp_ctx);
if (!NT_STATUS_IS_OK(status)) {
- exit_daemon("Samba failed to setup parent messaging", NT_STATUS_V(status));
+ exit_daemon("Samba failed to setup parent messaging",
+ NT_STATUS_V(status));
}
DEBUG(0,("%s: using '%s' process model\n", binary_name, model));
- status = server_service_startup(event_ctx, cmdline_lp_ctx, model,
+ status = server_service_startup(event_ctx, cmdline_lp_ctx, model,
lpcfg_server_services(cmdline_lp_ctx));
if (!NT_STATUS_IS_OK(status)) {
- exit_daemon("Samba failed to start services", NT_STATUS_V(status));
+ exit_daemon("Samba failed to start services",
+ NT_STATUS_V(status));
}
if (opt_daemon) {
--
2.1.4
From f5b072c5cb1c55490def0af66fa584dc23ea7e0a Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 31 Mar 2017 11:54:45 -0700
Subject: [PATCH 04/15] s4: server: Create a server 'state' struct.
No logic changes, will be used to move allocated
pointers off the talloc autofree context in a later commit.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source4/smbd/server.c | 40 ++++++++++++++++++++++++++--------------
1 file changed, 26 insertions(+), 14 deletions(-)
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index 4242fe6..8e807ca 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -44,6 +44,11 @@
#include "nsswitch/winbind_client.h"
#include "libds/common/roles.h"
+struct server_state {
+ struct tevent_context *event_ctx;
+ const char *binary_name;
+};
+
/*
recursively delete a directory tree
*/
@@ -319,7 +324,6 @@ static int binary_smbd_main(const char *binary_name,
STATIC_service_MODULES_PROTO;
init_module_fn static_init[] = { STATIC_service_MODULES };
init_module_fn *shared_init;
- struct tevent_context *event_ctx;
uint16_t stdin_event_flags;
NTSTATUS status;
const char *model = "standard";
@@ -348,6 +352,7 @@ static int binary_smbd_main(const char *binary_name,
POPT_COMMON_VERSION
{ NULL }
};
+ struct server_state *state = NULL;
pc = poptGetContext(binary_name, argc, argv, long_options, 0);
while((opt = poptGetNextOpt(pc)) != -1) {
@@ -418,6 +423,13 @@ static int binary_smbd_main(const char *binary_name,
become_daemon(true, false, false);
}
+ /* Create the memory context to hang everything off. */
+ state = talloc_zero(NULL, struct server_state);
+ if (state == NULL) {
+ exit_daemon("Samba cannot create server state", ENOMEM);
+ };
+ state->binary_name = binary_name;
+
cleanup_tmp_files(cmdline_lp_ctx);
if (!directory_exist(lpcfg_lock_directory(cmdline_lp_ctx))) {
@@ -453,13 +465,13 @@ static int binary_smbd_main(const char *binary_name,
/* the event context is the top level structure in smbd. Everything else
should hang off that */
- event_ctx = s4_event_context_init(talloc_autofree_context());
+ state->event_ctx = s4_event_context_init(talloc_autofree_context());
- if (event_ctx == NULL) {
+ if (state->event_ctx == NULL) {
exit_daemon("Initializing event context failed", EACCES);
}
- talloc_set_destructor(event_ctx, event_ctx_destructor);
+ talloc_set_destructor(state->event_ctx, event_ctx_destructor);
if (opt_interactive) {
/* terminate when stdin goes away */
@@ -480,8 +492,8 @@ static int binary_smbd_main(const char *binary_name,
}
if (S_ISFIFO(st.st_mode) || S_ISSOCK(st.st_mode)) {
- tevent_add_fd(event_ctx,
- event_ctx,
+ tevent_add_fd(state->event_ctx,
+ state->event_ctx,
0,
stdin_event_flags,
server_stdin_handler,
@@ -493,7 +505,7 @@ static int binary_smbd_main(const char *binary_name,
"current ts %llu\n",
binary_name, (int)getpid(),
max_runtime, (unsigned long long) time(NULL)));
- tevent_add_timer(event_ctx, event_ctx,
+ tevent_add_timer(state->event_ctx, state->event_ctx,
timeval_current_ofs(max_runtime, 0),
max_runtime_handler,
discard_const(binary_name));
@@ -519,9 +531,9 @@ static int binary_smbd_main(const char *binary_name,
"and exited. Check logs for details", EINVAL);
};
- prime_ldb_databases(event_ctx);
+ prime_ldb_databases(state->event_ctx);
- status = setup_parent_messaging(event_ctx, cmdline_lp_ctx);
+ status = setup_parent_messaging(state->event_ctx, cmdline_lp_ctx);
if (!NT_STATUS_IS_OK(status)) {
exit_daemon("Samba failed to setup parent messaging",
NT_STATUS_V(status));
@@ -529,7 +541,7 @@ static int binary_smbd_main(const char *binary_name,
DEBUG(0,("%s: using '%s' process model\n", binary_name, model));
- status = server_service_startup(event_ctx, cmdline_lp_ctx, model,
+ status = server_service_startup(state->event_ctx, cmdline_lp_ctx, model,
lpcfg_server_services(cmdline_lp_ctx));
if (!NT_STATUS_IS_OK(status)) {
exit_daemon("Samba failed to start services",
@@ -542,11 +554,11 @@ static int binary_smbd_main(const char *binary_name,
/* wait for events - this is where smbd sits for most of its
life */
- tevent_loop_wait(event_ctx);
+ tevent_loop_wait(state->event_ctx);
- /* as everything hangs off this event context, freeing it
- should initiate a clean shutdown of all services */
- talloc_free(event_ctx);
+ /* as everything hangs off this state->event context, freeing state
+ will initiate a clean shutdown of all services */
+ TALLOC_FREE(state);
return 0;
}
--
2.1.4
From 51a97c25eeedc30c9765def7c05b955b76ae3b1c Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 31 Mar 2017 11:59:13 -0700
Subject: [PATCH 05/15] s4: server: Use server_state as a parameter to stdin
handler, not just name.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source4/smbd/server.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index 8e807ca..0353bfa 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -165,11 +165,11 @@ static void server_stdin_handler(struct tevent_context *event_ctx,
uint16_t flags,
void *private_data)
{
- const char *binary_name = (const char *)private_data;
+ struct server_state *state = (struct server_state *)private_data;
uint8_t c;
if (read(0, &c, 1) == 0) {
DEBUG(0,("%s: EOF on stdin - PID %d terminating\n",
- binary_name, (int)getpid()));
+ state->binary_name, (int)getpid()));
#if HAVE_GETPGRP
if (getpgrp() == getpid()) {
DEBUG(0,("Sending SIGTERM from pid %d\n",
@@ -497,7 +497,7 @@ static int binary_smbd_main(const char *binary_name,
0,
stdin_event_flags,
server_stdin_handler,
- discard_const(binary_name));
+ state);
}
if (max_runtime) {
--
2.1.4
From 48776ae24ae668e0588d8eed8926531b3872b383 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 11 Apr 2017 07:44:38 +0200
Subject: [PATCH 06/15] SQ: Use type-safe cast
---
source4/smbd/server.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index 0353bfa..97f0c02 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -165,7 +165,8 @@ static void server_stdin_handler(struct tevent_context *event_ctx,
uint16_t flags,
void *private_data)
{
- struct server_state *state = (struct server_state *)private_data;
+ struct server_state *state = talloc_get_type_abort(
+ private_data, struct server_state);
uint8_t c;
if (read(0, &c, 1) == 0) {
DEBUG(0,("%s: EOF on stdin - PID %d terminating\n",
--
2.1.4
From 1a8d198609578c2e5ce74bc9d7ff18f152f2efdc Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 31 Mar 2017 12:00:29 -0700
Subject: [PATCH 07/15] s4: server: Use server_state as a parameter to
max_runtime_handler, not just name.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source4/smbd/server.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index 97f0c02..f08556c 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -189,10 +189,10 @@ _NORETURN_ static void max_runtime_handler(struct tevent_context *ev,
struct tevent_timer *te,
struct timeval t, void *private_data)
{
- const char *binary_name = (const char *)private_data;
+ struct server_state *state = (struct server_state *)private_data;
DEBUG(0,("%s: maximum runtime exceeded - "
"terminating PID %d at %llu, current ts: %llu\n",
- binary_name,
+ state->binary_name,
(int)getpid(),
(unsigned long long)t.tv_sec,
(unsigned long long)time(NULL)));
@@ -509,7 +509,7 @@ static int binary_smbd_main(const char *binary_name,
tevent_add_timer(state->event_ctx, state->event_ctx,
timeval_current_ofs(max_runtime, 0),
max_runtime_handler,
- discard_const(binary_name));
+ state);
}
if (lpcfg_server_role(cmdline_lp_ctx) != ROLE_ACTIVE_DIRECTORY_DC
--
2.1.4
From c434937316cf61298fa4320bc854d6a90470793f Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 11 Apr 2017 07:45:25 +0200
Subject: [PATCH 08/15] SQ: Use type-safe cast
---
source4/smbd/server.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index f08556c..96f9d9d 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -189,7 +189,8 @@ _NORETURN_ static void max_runtime_handler(struct tevent_context *ev,
struct tevent_timer *te,
struct timeval t, void *private_data)
{
- struct server_state *state = (struct server_state *)private_data;
+ struct server_state *state = talloc_get_type_abort(
+ private_data, struct server_state);
DEBUG(0,("%s: maximum runtime exceeded - "
"terminating PID %d at %llu, current ts: %llu\n",
state->binary_name,
--
2.1.4
From 13a0aee943f66f2a07b321626511acdd71b11a47 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 31 Mar 2017 12:23:56 -0700
Subject: [PATCH 09/15] s4: server: Plumb server_state through the irpc
messaging for samba_terminate().
Use it in the message print to avoid a "unused variable" compile error.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source4/smbd/server.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index 96f9d9d..1315d6b 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -227,7 +227,10 @@ static void prime_ldb_databases(struct tevent_context *event_ctx)
static NTSTATUS samba_terminate(struct irpc_message *msg,
struct samba_terminate *r)
{
- DEBUG(0,("samba_terminate of %d: %s\n",
+ struct server_state *state = talloc_get_type(msg->private_data,
+ struct server_state);
+ DEBUG(0,("samba_terminate of %s %d: %s\n",
+ state->binary_name,
(int)getpid(), r->in.reason));
exit(1);
}
@@ -235,7 +238,7 @@ static NTSTATUS samba_terminate(struct irpc_message *msg,
/*
setup messaging for the top level samba (parent) task
*/
-static NTSTATUS setup_parent_messaging(struct tevent_context *event_ctx,
+static NTSTATUS setup_parent_messaging(struct server_state *state,
struct loadparm_context *lp_ctx)
{
struct imessaging_context *msg;
@@ -243,7 +246,8 @@ static NTSTATUS setup_parent_messaging(struct tevent_context *event_ctx,
msg = imessaging_init(talloc_autofree_context(),
lp_ctx,
- cluster_id(0, SAMBA_PARENT_TASKID), event_ctx);
+ cluster_id(0, SAMBA_PARENT_TASKID),
+ state->event_ctx);
NT_STATUS_HAVE_NO_MEMORY(msg);
status = irpc_add_name(msg, "samba");
@@ -252,7 +256,7 @@ static NTSTATUS setup_parent_messaging(struct tevent_context *event_ctx,
}
status = IRPC_REGISTER(msg, irpc, SAMBA_TERMINATE,
- samba_terminate, NULL);
+ samba_terminate, state);
return status;
}
@@ -535,7 +539,7 @@ static int binary_smbd_main(const char *binary_name,
prime_ldb_databases(state->event_ctx);
- status = setup_parent_messaging(state->event_ctx, cmdline_lp_ctx);
+ status = setup_parent_messaging(state, cmdline_lp_ctx);
if (!NT_STATUS_IS_OK(status)) {
exit_daemon("Samba failed to setup parent messaging",
NT_STATUS_V(status));
--
2.1.4
From 0b354a0de8af2726e41b41ce0214360c2977183b Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 11 Apr 2017 07:37:23 +0200
Subject: [PATCH 10/15] SQ: Use modern DBG macro
---
source4/smbd/server.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index 1315d6b..d80a94d 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -229,9 +229,8 @@ static NTSTATUS samba_terminate(struct irpc_message *msg,
{
struct server_state *state = talloc_get_type(msg->private_data,
struct server_state);
- DEBUG(0,("samba_terminate of %s %d: %s\n",
- state->binary_name,
- (int)getpid(), r->in.reason));
+ DBG_ERR("samba_terminate of %s %d: %s\n",
+ state->binary_name, (int)getpid(), r->in.reason);
exit(1);
}
--
2.1.4
From 28af867efca400fc4dec3f543bd6ebdf8f76fd28 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 31 Mar 2017 12:29:03 -0700
Subject: [PATCH 11/15] s4: server: Add error return checks for tevent_add_fde,
tevent_add_timer.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source4/smbd/server.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index d80a94d..1e5d579 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -497,23 +497,30 @@ static int binary_smbd_main(const char *binary_name,
}
if (S_ISFIFO(st.st_mode) || S_ISSOCK(st.st_mode)) {
- tevent_add_fd(state->event_ctx,
+ struct tevent_fd *fde = tevent_add_fd(state->event_ctx,
state->event_ctx,
0,
stdin_event_flags,
server_stdin_handler,
state);
+ if (fde == NULL) {
+ exit_daemon("Initializing stdin failed", ENOMEM);
+ }
}
if (max_runtime) {
+ struct tevent_timer *te;
DEBUG(0,("%s PID %d was called with maxruntime %d - "
"current ts %llu\n",
binary_name, (int)getpid(),
max_runtime, (unsigned long long) time(NULL)));
- tevent_add_timer(state->event_ctx, state->event_ctx,
+ te = tevent_add_timer(state->event_ctx, state->event_ctx,
timeval_current_ofs(max_runtime, 0),
max_runtime_handler,
state);
+ if (te == NULL) {
+ exit_daemon("Maxruntime handler failed", ENOMEM);
+ }
}
if (lpcfg_server_role(cmdline_lp_ctx) != ROLE_ACTIVE_DIRECTORY_DC
--
2.1.4
From 8300be6a60af0f323cab37b328c6d032b46b27db Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 31 Mar 2017 12:38:14 -0700
Subject: [PATCH 12/15] s4: server: Add a tevent signal handler for SIGTERM.
Simplify by removing global state we don't need now
we're called by tevent (and in the short window where
we're installed by CatchSignal but before we install
the tevent handler we don't need the complex global
state handling as we have no forked children).
We now have access to struct server_state on all
exit paths - next commits will stop using talloc autofree context.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source4/smbd/server.c | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index 1e5d579..9814837 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -115,10 +115,12 @@ static void sig_hup(int sig)
static void sig_term(int sig)
{
#if HAVE_GETPGRP
- static int done_sigterm;
- if (done_sigterm == 0 && getpgrp() == getpid()) {
+ if (getpgrp() == getpid()) {
+ /*
+ * We're the process group leader, send
+ * SIGTERM to our process group.
+ */
DEBUG(0,("SIGTERM: killing children\n"));
- done_sigterm = 1;
kill(-getpgrp(), SIGTERM);
}
#endif
@@ -126,6 +128,18 @@ static void sig_term(int sig)
exit(127);
}
+static void sigterm_signal_handler(struct tevent_context *ev,
+ struct tevent_signal *se,
+ int signum, int count, void *siginfo,
+ void *private_data)
+{
+ struct server_state *state = talloc_get_type_abort(
+ private_data, struct server_state);
+
+ DEBUG(10,("Process %s got SIGTERM\n", state->binary_name));
+ sig_term(SIGTERM);
+}
+
/*
setup signal masks
*/
@@ -358,6 +372,7 @@ static int binary_smbd_main(const char *binary_name,
{ NULL }
};
struct server_state *state = NULL;
+ struct tevent_signal *se = NULL;
pc = poptGetContext(binary_name, argc, argv, long_options, 0);
while((opt = poptGetNextOpt(pc)) != -1) {
@@ -523,6 +538,16 @@ static int binary_smbd_main(const char *binary_name,
}
}
+ se = tevent_add_signal(state->event_ctx,
+ state->event_ctx,
+ SIGTERM,
+ 0,
+ sigterm_signal_handler,
+ state);
+ if (se == NULL) {
+ exit_daemon("Initialize SIGTERM handler failed", ENOMEM);
+ }
+
if (lpcfg_server_role(cmdline_lp_ctx) != ROLE_ACTIVE_DIRECTORY_DC
&& !lpcfg_parm_bool(cmdline_lp_ctx, NULL,
"server role check", "inhibit", false)
--
2.1.4
From 812fdd87face4d0f0731d01aba294fb21dbe88d7 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 3 Apr 2017 17:58:24 +0000
Subject: [PATCH 13/15] VL: I'd love to see why this is necessary. What fails
without it?
I can see it's cleaner and the next commit (probably) builds upon it,
but the actual use case would be great to understand
s4: messaging: When talloc_free()'ing an event context, only remove msg_dgm_ref's that point to *that* context.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source4/lib/messaging/messaging.c | 8 +++++---
source4/lib/messaging/messaging.h | 2 +-
source4/smbd/server.c | 2 +-
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/source4/lib/messaging/messaging.c b/source4/lib/messaging/messaging.c
index 19873c5..2da0acc 100644
--- a/source4/lib/messaging/messaging.c
+++ b/source4/lib/messaging/messaging.c
@@ -256,18 +256,20 @@ static int imessaging_context_destructor(struct imessaging_context *msg)
}
/*
- * Cleanup messaging dgm contexts
+ * Cleanup messaging dgm contexts on a specific event context.
*
* We must make sure to unref all messaging_dgm_ref's *before* the
* tevent context goes away. Only when the last ref is freed, the
* refcounted messaging dgm context will be freed.
*/
-void imessaging_dgm_unref_all(void)
+void imessaging_dgm_unref_ev(struct tevent_context *ev)
{
struct imessaging_context *msg = NULL;
for (msg = msg_ctxs; msg != NULL; msg = msg->next) {
- TALLOC_FREE(msg->msg_dgm_ref);
+ if (msg->ev == ev) {
+ TALLOC_FREE(msg->msg_dgm_ref);
+ }
}
}
diff --git a/source4/lib/messaging/messaging.h b/source4/lib/messaging/messaging.h
index e587fdf..668464d 100644
--- a/source4/lib/messaging/messaging.h
+++ b/source4/lib/messaging/messaging.h
@@ -44,7 +44,7 @@ struct imessaging_context *imessaging_init(TALLOC_CTX *mem_ctx,
struct loadparm_context *lp_ctx,
struct server_id server_id,
struct tevent_context *ev);
-void imessaging_dgm_unref_all(void);
+void imessaging_dgm_unref_ev(struct tevent_context *ev);
NTSTATUS imessaging_reinit_all(void);
int imessaging_cleanup(struct imessaging_context *msg);
struct imessaging_context *imessaging_client_init(TALLOC_CTX *mem_ctx,
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index 9814837..ec56d10 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -324,7 +324,7 @@ static void show_build(void)
static int event_ctx_destructor(struct tevent_context *event_ctx)
{
- imessaging_dgm_unref_all();
+ imessaging_dgm_unref_ev(event_ctx);
return 0;
}
--
2.1.4
From 41f71451b70a894c2d3cca5724add9836a79bd7b Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 3 Apr 2017 18:04:31 +0000
Subject: [PATCH 14/15] s4: server: Remove use of talloc_autofree_context as
the parent of event_ctx.
Use state->event_ctx as the parent of the initial imessaging context.
Now we control all exit paths, we can call TALLOC_FREE(state)
on all of them.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source4/smbd/server.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index ec56d10..22b22a7 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -137,6 +137,7 @@ static void sigterm_signal_handler(struct tevent_context *ev,
private_data, struct server_state);
DEBUG(10,("Process %s got SIGTERM\n", state->binary_name));
+ TALLOC_FREE(state);
sig_term(SIGTERM);
}
@@ -192,6 +193,7 @@ static void server_stdin_handler(struct tevent_context *event_ctx,
kill(-getpgrp(), SIGTERM);
}
#endif
+ TALLOC_FREE(state);
exit(0);
}
}
@@ -211,6 +213,7 @@ _NORETURN_ static void max_runtime_handler(struct tevent_context *ev,
(int)getpid(),
(unsigned long long)t.tv_sec,
(unsigned long long)time(NULL)));
+ TALLOC_FREE(state);
exit(0);
}
@@ -245,6 +248,7 @@ static NTSTATUS samba_terminate(struct irpc_message *msg,
struct server_state);
DBG_ERR("samba_terminate of %s %d: %s\n",
state->binary_name, (int)getpid(), r->in.reason);
+ TALLOC_FREE(state);
exit(1);
}
@@ -257,7 +261,7 @@ static NTSTATUS setup_parent_messaging(struct server_state *state,
struct imessaging_context *msg;
NTSTATUS status;
- msg = imessaging_init(talloc_autofree_context(),
+ msg = imessaging_init(state->event_ctx,
lp_ctx,
cluster_id(0, SAMBA_PARENT_TASKID),
state->event_ctx);
@@ -485,7 +489,7 @@ static int binary_smbd_main(const char *binary_name,
/* the event context is the top level structure in smbd. Everything else
should hang off that */
- state->event_ctx = s4_event_context_init(talloc_autofree_context());
+ state->event_ctx = s4_event_context_init(state);
if (state->event_ctx == NULL) {
exit_daemon("Initializing event context failed", EACCES);
--
2.1.4
From 7bbade8f7a9e9a595bf579fec1badef99f87d30c Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 3 Apr 2017 18:16:02 +0000
Subject: [PATCH 15/15] s4: server: Use state as the talloc context for
open_schannel_session_store.
Ensure it's freed on all error paths.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source4/smbd/server.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index 22b22a7..fdc36de 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -463,8 +463,9 @@ static int binary_smbd_main(const char *binary_name,
pidfile_create(lpcfg_pid_directory(cmdline_lp_ctx), binary_name);
if (lpcfg_server_role(cmdline_lp_ctx) == ROLE_ACTIVE_DIRECTORY_DC) {
- if (!open_schannel_session_store(talloc_autofree_context(),
+ if (!open_schannel_session_store(state,
cmdline_lp_ctx)) {
+ TALLOC_FREE(state);
exit_daemon("Samba cannot open schannel store "
"for secured NETLOGON operations.", EACCES);
}
@@ -472,6 +473,7 @@ static int binary_smbd_main(const char *binary_name,
/* make sure we won't go through nss_winbind */
if (!winbind_off()) {
+ TALLOC_FREE(state);
exit_daemon("Samba failed to disable recusive "
"winbindd calls.", EACCES);
}
@@ -492,6 +494,7 @@ static int binary_smbd_main(const char *binary_name,
state->event_ctx = s4_event_context_init(state);
if (state->event_ctx == NULL) {
+ TALLOC_FREE(state);
exit_daemon("Initializing event context failed", EACCES);
}
@@ -511,6 +514,7 @@ static int binary_smbd_main(const char *binary_name,
#endif
if (fstat(0, &st) != 0) {
+ TALLOC_FREE(state);
exit_daemon("Samba failed to set standard input handler",
ENOTTY);
}
@@ -523,6 +527,7 @@ static int binary_smbd_main(const char *binary_name,
server_stdin_handler,
state);
if (fde == NULL) {
+ TALLOC_FREE(state);
exit_daemon("Initializing stdin failed", ENOMEM);
}
}
@@ -538,6 +543,7 @@ static int binary_smbd_main(const char *binary_name,
max_runtime_handler,
state);
if (te == NULL) {
+ TALLOC_FREE(state);
exit_daemon("Maxruntime handler failed", ENOMEM);
}
}
@@ -549,6 +555,7 @@ static int binary_smbd_main(const char *binary_name,
sigterm_signal_handler,
state);
if (se == NULL) {
+ TALLOC_FREE(state);
exit_daemon("Initialize SIGTERM handler failed", ENOMEM);
}
@@ -576,6 +583,7 @@ static int binary_smbd_main(const char *binary_name,
status = setup_parent_messaging(state, cmdline_lp_ctx);
if (!NT_STATUS_IS_OK(status)) {
+ TALLOC_FREE(state);
exit_daemon("Samba failed to setup parent messaging",
NT_STATUS_V(status));
}
@@ -585,6 +593,7 @@ static int binary_smbd_main(const char *binary_name,
status = server_service_startup(state->event_ctx, cmdline_lp_ctx, model,
lpcfg_server_services(cmdline_lp_ctx));
if (!NT_STATUS_IS_OK(status)) {
+ TALLOC_FREE(state);
exit_daemon("Samba failed to start services",
NT_STATUS_V(status));
}
--
2.1.4
More information about the samba-technical
mailing list