[PATCH] Remove use of talloc_autofree_context() from source4/smbd/server.c
Jeremy Allison
jra at samba.org
Tue Apr 11 03:11:44 UTC 2017
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.
Here is a patchset that does some cleanups, and
moves the top level event context into a state
struct that is allocated and freed up correctly
without having to use the autofree context.
Again, this won't affect Andrews's ldap multi-process
patch and can go in before or after.
Description follows:
------------------------------------------------------
The two below are merely code cleanups:
#1: Add missing error return checks in imessaging_init().
#2: Whitespace and 80+ column cleanup in source4/smbd/server.c
------------------------------------------------------
The meat of the patch follows below, split into
micro-changes for easy digestion :-):
#3: Create a server_state struct. Use it to hold a pointer
to the top level event context, but leave that allocated
on autofree for now.
------------------------------------------------------
Minor changes:
#4: Plumb server_state into server_stdin_handler()
#5: Plumb server_state into max_runtime_handler()
#6: Plumb server_state into setup_parent_messaging()
#7: Add error return check for tevent_add_fd() tevent_add_timer()
#8: Add a tevent SIGTERM handler - simplify by removing global state.
------------------------------------------------------
Core changes:
#9: Change the event context destructor to unref
only msg_dgm_ref entries that are pointing to it.
#10: Make state the parent of the top level event
context, not the autofree context. Ensure that
TALLOC_FREE(state) is called on all exit paths.
#11: Make state the parent of open_schannel_session_store().
Don't use the autofree context anymore.
------------------------------------------------------
Passes full autobuild.
Please review and push if happy !
Cheers,
Jeremy.
-------------- next part --------------
>From 3afd3acb2fbc4cbc3b4b5589e4416a8d0cc35e7e 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/11] 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 844466d41c0..a1c790ff55d 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.12.2.715.g7642488e1d-goog
>From 440f366bdcf4086b3eaa42dcb98c1c55e83c439b 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 02/11] 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 b8545b89c72..4242fe6b7c7 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.12.2.715.g7642488e1d-goog
>From 72820c7840a961da035f9ca8e327a5b67df496e9 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 03/11] 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 4242fe6b7c7..8e807ca21a0 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.12.2.715.g7642488e1d-goog
>From 9a295894e4c4e3f4f53b902cfcbb9dfd2acaf52e 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 04/11] 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 8e807ca21a0..0353bfa15f7 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.12.2.715.g7642488e1d-goog
>From 83901ed423b4fb871f9f7fed06a6aceaba70e8d2 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 05/11] 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 0353bfa15f7..b395786f3ca 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -188,10 +188,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)));
@@ -508,7 +508,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.12.2.715.g7642488e1d-goog
>From 5bc80542888ad66b19f357b2a92e94660e63768c 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 06/11] 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 b395786f3ca..b000bfc2463 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -225,7 +225,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);
}
@@ -233,7 +236,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;
@@ -241,7 +244,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");
@@ -250,7 +254,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;
}
@@ -533,7 +537,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.12.2.715.g7642488e1d-goog
>From 2bcda18caf8f9bbd7894fdf3b83e698b2bbc9ccd 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 07/11] 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 b000bfc2463..4e141bfe797 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -496,23 +496,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.12.2.715.g7642488e1d-goog
>From 4da62dc7be60b40ce35c687b7bd63e64ef2a281a 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 08/11] 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 4e141bfe797..bd391e4cc3b 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
*/
@@ -357,6 +371,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) {
@@ -522,6 +537,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.12.2.715.g7642488e1d-goog
>From fa09834978573d43502e279c701e1ecfe8c1c007 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 09/11] 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 a1c790ff55d..3723837e17d 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 e587fdfb3aa..668464dc061 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 bd391e4cc3b..c107df04bf0 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -323,7 +323,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.12.2.715.g7642488e1d-goog
>From cdaf6cecd815f501d1767a93094a61da5d9e87bb 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 10/11] 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 c107df04bf0..4bf716591d3 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);
}
@@ -191,6 +192,7 @@ static void server_stdin_handler(struct tevent_context *event_ctx,
kill(-getpgrp(), SIGTERM);
}
#endif
+ TALLOC_FREE(state);
exit(0);
}
}
@@ -209,6 +211,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);
}
@@ -244,6 +247,7 @@ static NTSTATUS samba_terminate(struct irpc_message *msg,
DEBUG(0,("samba_terminate of %s %d: %s\n",
state->binary_name,
(int)getpid(), r->in.reason));
+ TALLOC_FREE(state);
exit(1);
}
@@ -256,7 +260,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);
@@ -484,7 +488,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.12.2.715.g7642488e1d-goog
>From 4647406c480d34c772772076e9935ddf733827c6 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 11/11] 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 4bf716591d3..73bc497b1ae 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -462,8 +462,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);
}
@@ -471,6 +472,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);
}
@@ -491,6 +493,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);
}
@@ -510,6 +513,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);
}
@@ -522,6 +526,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);
}
}
@@ -537,6 +542,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);
}
}
@@ -548,6 +554,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);
}
@@ -575,6 +582,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));
}
@@ -584,6 +592,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.12.2.715.g7642488e1d-goog
More information about the samba-technical
mailing list