Many smbd forked processes are not resetting am_parent to NULL
Ralph Böhme
rb at sernet.de
Wed Apr 22 04:02:55 MDT 2015
On Wed, Apr 15, 2015 at 09:06:45AM -0700, Jeremy Allison wrote:
> On Wed, Apr 15, 2015 at 11:28:11AM +0200, Ralph Böhme wrote:
> > Hi folks,
> >
> > exit_server_common() checks the am_parent global and does a few things
> > differently depending whether am_parent is NULL or not.
> >
> > What are the supposed semantics? If I'm not mislead am_parent should
> > be non NULL in the main smbd process and NULL in any forked child, be
> > it a forked sesssion smbd process or some other forked subsystem.
> >
> > The thing is, only smbd sessions childs reset am_parent to NULL as
> > does the scavenger child, other subsystems like the printing subsystem
> > seem to miss this step.
> >
> > Shouldn't we basically set am_parent to NULL in anything that forks
> > from the main smbd?
>
> Yes.
>
> > I'm currently looking at adding some cleanup code to
> > exit_server_common() that is supposed to execute some code only in the
> > am_parent != NULL case in the main smbd process. At the moment the
> > code gets executed too from the printing subsystem. :)
>
> That's a sad thing :-(.
>
> > Happy to provide a patch that fixes this if I'm right about this.
>
> Please do :-).
patch attached. Please review&push if ok. Thanks!
-Ralph
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de,mailto:kontakt@sernet.de
-------------- next part --------------
From d9b5cc091a50fc9b3d5ab4578bf5e731c80123a3 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 22 Apr 2015 11:57:24 +0200
Subject: [PATCH 1/2] s3:smbd: add wrapper around reinit_after_fork
smbd_reinit_after_fork is a simple wrapper around reinit_after_fork that
should be used after forking from the main smbd.
At the moment the only additional step it performs is resetting
am_parent to NULL.
A subsequent commit will make use of this function.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/include/proto.h | 3 +++
source3/smbd/server_exit.c | 12 ++++++++++++
2 files changed, 15 insertions(+)
diff --git a/source3/include/proto.h b/source3/include/proto.h
index c66283b..f9c22a0 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -374,6 +374,9 @@ NTSTATUS init_before_fork(void);
NTSTATUS reinit_after_fork(struct messaging_context *msg_ctx,
struct tevent_context *ev_ctx,
bool parent_longlived);
+NTSTATUS smbd_reinit_after_fork(struct messaging_context *msg_ctx,
+ struct tevent_context *ev_ctx,
+ bool parent_longlived);
void *malloc_(size_t size);
void *Realloc(void *p, size_t size, bool free_old_on_error);
void add_to_large_array(TALLOC_CTX *mem_ctx, size_t element_size,
diff --git a/source3/smbd/server_exit.c b/source3/smbd/server_exit.c
index 69d0fdd..700f595 100644
--- a/source3/smbd/server_exit.c
+++ b/source3/smbd/server_exit.c
@@ -268,3 +268,15 @@ void smbd_exit_server_cleanly(const char *const explanation)
{
exit_server_common(SERVER_EXIT_NORMAL, explanation);
}
+
+/*
+ * reinit_after_fork() wrapper that should be called when forking from
+ * smbd.
+ */
+NTSTATUS smbd_reinit_after_fork(struct messaging_context *msg_ctx,
+ struct tevent_context *ev_ctx,
+ bool parent_longlived)
+{
+ am_parent = NULL;
+ return reinit_after_fork(msg_ctx, ev_ctx, parent_longlived);
+}
--
2.1.0
From d4f5c06114070b67a2ecf55bab8e6d2196dfc96e Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 22 Apr 2015 12:00:10 +0200
Subject: [PATCH 2/2] s3:smbd: use smbd_reinit_after_fork
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/printing/queue_process.c | 2 +-
source3/printing/spoolssd.c | 4 +---
source3/rpc_server/epmd.c | 4 +---
source3/rpc_server/fssd.c | 4 +---
source3/rpc_server/lsasd.c | 4 +---
source3/smbd/process.c | 4 +---
source3/smbd/scavenger.c | 4 +---
source3/smbd/server.c | 7 +------
8 files changed, 8 insertions(+), 25 deletions(-)
diff --git a/source3/printing/queue_process.c b/source3/printing/queue_process.c
index 88196b4..50622dc 100644
--- a/source3/printing/queue_process.c
+++ b/source3/printing/queue_process.c
@@ -373,7 +373,7 @@ pid_t start_background_queue(struct tevent_context *ev,
close(pause_pipe[0]);
pause_pipe[0] = -1;
- status = reinit_after_fork(msg_ctx, ev, true);
+ status = smbd_reinit_after_fork(msg_ctx, ev, true);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(0,("reinit_after_fork() failed\n"));
diff --git a/source3/printing/spoolssd.c b/source3/printing/spoolssd.c
index b850578..be7c6ac 100644
--- a/source3/printing/spoolssd.c
+++ b/source3/printing/spoolssd.c
@@ -645,9 +645,7 @@ pid_t start_spoolssd(struct tevent_context *ev_ctx,
return pid;
}
- status = reinit_after_fork(msg_ctx,
- ev_ctx,
- true);
+ status = smbd_reinit_after_fork(msg_ctx, ev_ctx, true);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(0,("reinit_after_fork() failed\n"));
smb_panic("reinit_after_fork() failed");
diff --git a/source3/rpc_server/epmd.c b/source3/rpc_server/epmd.c
index 6b6119b..dad67ae 100644
--- a/source3/rpc_server/epmd.c
+++ b/source3/rpc_server/epmd.c
@@ -162,9 +162,7 @@ void start_epmd(struct tevent_context *ev_ctx,
return;
}
- status = reinit_after_fork(msg_ctx,
- ev_ctx,
- true);
+ status = smbd_reinit_after_fork(msg_ctx, ev_ctx, true);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(0,("reinit_after_fork() failed\n"));
smb_panic("reinit_after_fork() failed");
diff --git a/source3/rpc_server/fssd.c b/source3/rpc_server/fssd.c
index eb9b7e6..fc1f630 100644
--- a/source3/rpc_server/fssd.c
+++ b/source3/rpc_server/fssd.c
@@ -171,9 +171,7 @@ void start_fssd(struct tevent_context *ev_ctx,
}
/* child */
- status = reinit_after_fork(msg_ctx,
- ev_ctx,
- true);
+ status = smbd_reinit_after_fork(msg_ctx, ev_ctx, true);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(0,("reinit_after_fork() failed\n"));
smb_panic("reinit_after_fork() failed");
diff --git a/source3/rpc_server/lsasd.c b/source3/rpc_server/lsasd.c
index 375f409..9c816d3 100644
--- a/source3/rpc_server/lsasd.c
+++ b/source3/rpc_server/lsasd.c
@@ -861,9 +861,7 @@ void start_lsasd(struct tevent_context *ev_ctx,
return;
}
- status = reinit_after_fork(msg_ctx,
- ev_ctx,
- true);
+ status = smbd_reinit_after_fork(msg_ctx, ev_ctx, true);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(0,("reinit_after_fork() failed\n"));
smb_panic("reinit_after_fork() failed");
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index 38edb02..aa7f419 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -3306,9 +3306,7 @@ bool fork_echo_handler(struct smbXsrv_connection *xconn)
close(listener_pipe[0]);
set_blocking(listener_pipe[1], false);
- status = reinit_after_fork(xconn->msg_ctx,
- xconn->ev_ctx,
- true);
+ status = smbd_reinit_after_fork(xconn->msg_ctx, xconn->ev_ctx, true);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(1, ("reinit_after_fork failed: %s\n",
nt_errstr(status)));
diff --git a/source3/smbd/scavenger.c b/source3/smbd/scavenger.c
index 013b4d2..19f9b43 100644
--- a/source3/smbd/scavenger.c
+++ b/source3/smbd/scavenger.c
@@ -244,11 +244,9 @@ static bool smbd_scavenger_start(struct smbd_scavenger_state *state)
close(fds[0]);
- am_parent = NULL;
-
set_my_unique_id(unique_id);
- status = reinit_after_fork(state->msg, state->ev, true);
+ status = smbd_reinit_after_fork(state->msg, state->ev, true);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(2, ("reinit_after_fork failed: %s\n",
nt_errstr(status)));
diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index 030f760..6921719 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -590,9 +590,6 @@ static void smbd_accept_connection(struct tevent_context *ev,
if (pid == 0) {
NTSTATUS status = NT_STATUS_OK;
- /* Child code ... */
- am_parent = NULL;
-
/*
* Can't use TALLOC_FREE here. Nulling out the argument to it
* would overwrite memory we've just freed.
@@ -606,9 +603,7 @@ static void smbd_accept_connection(struct tevent_context *ev,
* them, counting worker smbds. */
CatchChild();
- status = reinit_after_fork(msg_ctx,
- ev,
- true);
+ status = smbd_reinit_after_fork(msg_ctx, ev, true);
if (!NT_STATUS_IS_OK(status)) {
if (NT_STATUS_EQUAL(status,
NT_STATUS_TOO_MANY_OPENED_FILES)) {
--
2.1.0
More information about the samba-technical
mailing list