[PATCH] Remove use of talloc_autofree_context() from source4/smbd/server.c
Jeremy Allison
jra at samba.org
Tue Apr 11 16:58:29 UTC 2017
On Tue, Apr 11, 2017 at 08:02:15AM +0200, Volker Lendecke wrote:
> 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).
Updated patchset with your cleanups incorporated.
Extra explaination on the defensive programming
patch.
Let me know if this works !
Cheers,
Jeremy.
-------------- next part --------------
From 79d3f54c0889bf96887bc4d0c3bfbb92f3e44010 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 | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/source4/lib/messaging/messaging.c b/source4/lib/messaging/messaging.c
index 844466d41c0..19873c5bc39 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,28 @@ 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 8c8525d1be4defb9cabb0140ab0263b256f1958f 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 a771cdf2fd0567acd13bfbf42b8372b5474df23f 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 efdb04ce46ba11aa29382cb1c9ab43732b627222 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 | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index 8e807ca21a0..97f0c02cac6 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -165,11 +165,12 @@ 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 = 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",
- 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 +498,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 ae86b57da17acb94eb3c2d4adff3ad72e87b5766 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 | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index 97f0c02cac6..96f9d9d2347 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -189,10 +189,11 @@ _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 = talloc_get_type_abort(
+ private_data, struct server_state);
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 +510,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 aacf57b386046062f8eed5d19e4cb76305848784 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 | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index 96f9d9d2347..d80a94db785 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -227,15 +227,17 @@ 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",
- (int)getpid(), r->in.reason));
+ struct server_state *state = talloc_get_type(msg->private_data,
+ struct server_state);
+ DBG_ERR("samba_terminate of %s %d: %s\n",
+ state->binary_name, (int)getpid(), r->in.reason);
exit(1);
}
/*
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 +245,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 +255,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 +538,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 1346f3b600b1ab922921217eef605507d7cdb243 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 d80a94db785..1e5d57940ce 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.12.2.715.g7642488e1d-goog
From 2e4f3192771af178085329b0525e83427604a02e 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 1e5d57940ce..98148374e5b 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.12.2.715.g7642488e1d-goog
From ad819dd0811bbbb1e63c971bc7da7b159e1566c2 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.
Defensive programming change. Not strictly needed to prevent
any crash/error.
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 19873c5bc39..2da0acc0c7a 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 98148374e5b..ec56d1025ea 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.12.2.715.g7642488e1d-goog
From 3bfd85a6a0e12b9920323fa79a39f36e0faf707f 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 ec56d1025ea..22b22a79fd9 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.12.2.715.g7642488e1d-goog
From b756ee57250f838e613503a54f59bb2c752c8d62 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 22b22a79fd9..fdc36dec129 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.12.2.715.g7642488e1d-goog
More information about the samba-technical
mailing list