Attachment 5610 Details for Bug 7317 – new Patch for v3-5 (including a fix for smbcontrol winbindd validate-cache)

[patch] new Patch for v3-5 (including a fix for smbcontrol winbindd validate-cache)

tmp.diff (text/plain), 5.39 KB, created by Stefan Metzmacher on 2010-04-08 06:02:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2010-04-08 06:02:33 UTC

Size: 5.39 KB

patch

obsolete

>From f55562aa86900ab9ad0f672ec8dd67a4aa5f0c1c Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 1 Apr 2010 16:23:06 +0200
>Subject: [PATCH 1/3] s3:winbindd: fix problems with SIGCHLD handling (bug #7317)
>
>The main problem is that we call CatchChild() within the
>parent winbindd, which overwrites the signal handler
>that was registered by winbindd_setup_sig_chld_handler().
>
>That means winbindd_sig_chld_handler() and winbind_child_died()
>are never triggered when a winbindd domain child dies.
>As a result will get "broken pipe" for all requests to that domain.
>
>To reduce the risk of similar bugs in future we call
>CatchChild() in winbindd_reinit_after_fork() now.
>
>We also use a full winbindd_reinit_after_fork() in the
>cache validation child now instead instead of just resetting
>the SIGCHLD handler by hand. This will also fix possible
>tdb problems on systems without pread/pwrite and disabled mmap
>as we now correctly reopen the tdb handle for the child.
>
>metze
>(cherry picked from commit 73577205cf81644e7fe853eaf3e6459f7f443096)
>---
> source3/winbindd/winbindd.c      |   14 +++-----------
> source3/winbindd/winbindd_cm.c   |    3 ---
> source3/winbindd/winbindd_dual.c |    6 +++---
> 3 files changed, 6 insertions(+), 17 deletions(-)
>
>diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
>index 300c78e..0258f3c 100644
>--- a/source3/winbindd/winbindd.c
>+++ b/source3/winbindd/winbindd.c
>@@ -383,7 +383,6 @@ static void winbind_msg_validate_cache(struct messaging_context *msg_ctx,
> 	 * so we don't block the main winbindd and the validation
> 	 * code can safely use fork/waitpid...
> 	 */
>-	CatchChild();
> 	child_pid = sys_fork();
> 
> 	if (child_pid == -1) {
>@@ -401,16 +400,9 @@ static void winbind_msg_validate_cache(struct messaging_context *msg_ctx,
> 
> 	/* child */
> 
>-	/* install default SIGCHLD handler: validation code uses fork/waitpid */
>-	ZERO_STRUCT(act);
>-	act.sa_handler = SIG_DFL;
>-#ifdef SA_RESTART
>-	/* We *want* SIGALRM to interrupt a system call. */
>-	act.sa_flags = SA_RESTART;
>-#endif
>-	sigemptyset(&act.sa_mask);
>-	sigaddset(&act.sa_mask,SIGCHLD);
>-	sigaction(SIGCHLD,&act,&oldact);
>+	if (!winbindd_reinit_after_fork(NULL)) {
>+		_exit(0);
>+	}
> 
> 	ret = (uint8)winbindd_validate_cache_nobackup();
> 	DEBUG(10, ("winbindd_msg_validata_cache: got return value %d\n", ret));
>diff --git a/source3/winbindd/winbindd_cm.c b/source3/winbindd/winbindd_cm.c
>index 97b4870..d9e5d9a 100644
>--- a/source3/winbindd/winbindd_cm.c
>+++ b/source3/winbindd/winbindd_cm.c
>@@ -179,9 +179,6 @@ static bool fork_child_dc_connect(struct winbindd_domain *domain)
> 	pid_t parent_pid = sys_getpid();
> 	char *lfile = NULL;
> 
>-	/* Stop zombies */
>-	CatchChild();
>-
> 	if (domain->dc_probe_pid != (pid_t)-1) {
> 		/*
> 		 * We might already have a DC probe
>diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c
>index afca1a2..3204664 100644
>--- a/source3/winbindd/winbindd_dual.c
>+++ b/source3/winbindd/winbindd_dual.c
>@@ -1215,6 +1215,9 @@ bool winbindd_reinit_after_fork(const char *logfilename)
> 					    logfilename))
> 		return false;
> 
>+	/* Stop zombies in children */
>+	CatchChild();
>+
> 	/* Don't handle the same messages as our parent. */
> 	messaging_deregister(winbind_messaging_context(),
> 			     MSG_SMB_CONF_UPDATED, NULL);
>@@ -1338,9 +1341,6 @@ static bool fork_domain_child(struct winbindd_child *child)
> 
> 	DEBUG(10, ("Child process %d\n", (int)sys_getpid()));
> 
>-	/* Stop zombies in children */
>-	CatchChild();
>-
> 	state.sock = fdpair[0];
> 	close(fdpair[1]);
> 
>-- 
>1.6.3.3
>
>
>From e690a9461209168ba5a49d44db541d88ace31de1 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 1 Apr 2010 18:10:47 +0200
>Subject: [PATCH 2/3] s3:winbindd: remove unused variables
>
>metze
>(cherry picked from commit e18ddb6036f5e0a2211e89a7c9b5514c30a653cf)
>---
> source3/winbindd/winbindd.c |    2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
>
>diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
>index 0258f3c..19561f4 100644
>--- a/source3/winbindd/winbindd.c
>+++ b/source3/winbindd/winbindd.c
>@@ -372,8 +372,6 @@ static void winbind_msg_validate_cache(struct messaging_context *msg_ctx,
> {
> 	uint8 ret;
> 	pid_t child_pid;
>-	struct sigaction act;
>-	struct sigaction oldact;
> 
> 	DEBUG(10, ("winbindd_msg_validate_cache: got validate-cache "
> 		   "message.\n"));
>-- 
>1.6.3.3
>
>
>From 2ea2701c3ff699cb1ae00a5215c16001ed581eae Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 8 Apr 2010 12:45:54 +0200
>Subject: [PATCH 3/3] s3:winbindd: make "smbcontrol winbindd validate-cache" reliable again
>
>commit 73577205cf81644e7fe853eaf3e6459f7f443096
>(s3:winbindd: fix problems with SIGCHLD handling (bug #7317))
>broke this.
>
>metze
>(cherry picked from commit eb9b7d0363669574de8ec380089407890f15eac2)
>---
> source3/winbindd/winbindd.c |    3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
>diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
>index 19561f4..9c676f0 100644
>--- a/source3/winbindd/winbindd.c
>+++ b/source3/winbindd/winbindd.c
>@@ -402,6 +402,9 @@ static void winbind_msg_validate_cache(struct messaging_context *msg_ctx,
> 		_exit(0);
> 	}
> 
>+	/* install default SIGCHLD handler: validation code uses fork/waitpid */
>+	CatchSignal(SIGCHLD, SIG_DFL);
>+
> 	ret = (uint8)winbindd_validate_cache_nobackup();
> 	DEBUG(10, ("winbindd_msg_validata_cache: got return value %d\n", ret));
> 	messaging_send_buf(msg_ctx, server_id, MSG_WINBIND_VALIDATE_CACHE, &ret,
>-- 
>1.6.3.3
>

Flags:

jra: review+

Actions: View

Attachments on bug 7317: 5586 | 5600 | 5601 | 5610 | 5611