[PATCH] Fix bug #10831 - SIGCLD Signal handler not correctly reinstalled on old library code use - smbrun etc.
Jeremy Allison
jra at samba.org
Tue Sep 23 16:44:25 MDT 2014
Some of the old library util code can
overwrite the SIGCHLD signal handler
carefully set up by smbd and winbindd
via tevent :-(.
It's used in blocking calls when launching
scripts as in smbrun(), but it leaves
the new handler in place when it exits
from smbrun(), rather than restoring
the original signal handler.
The first patch changes CatchChild()
and CatchChildLeaveStatus() to return
the existing handler so it can be
saved and restored.
The second patch fixes smbrun() and
the change password code (which
also invokes a child process) to
store and restore the existing
handler.
Review most appreciated !
Thanks,
Jeremy.
-------------- next part --------------
From 6c4331f96544bb97075251041b77d19c10b047bd Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 23 Sep 2014 14:48:35 -0700
Subject: [PATCH 1/2] lib: util [ctdb]: Signal handling - change CatchChild()
and CatchChildLeaveStatus() to return the previous handler.
Bug #10831 - SIGCLD Signal handler not correctly reinstalled on old library code use - smbrun etc.
https://bugzilla.samba.org/show_bug.cgi?id=10831
Signed-off-by: Jeremy Allison <jra at samba.org>
---
ctdb/lib/util/signal.c | 8 ++++----
ctdb/lib/util/util.h | 4 ++--
lib/util/samba_util.h | 4 ++--
lib/util/signal.c | 8 ++++----
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/ctdb/lib/util/signal.c b/ctdb/lib/util/signal.c
index ead947e..33a9900 100644
--- a/ctdb/lib/util/signal.c
+++ b/ctdb/lib/util/signal.c
@@ -129,16 +129,16 @@ void (*CatchSignal(int signum,void (*handler)(int )))(int)
Ignore SIGCLD via whatever means is necessary for this OS.
**/
-void CatchChild(void)
+void (*CatchChild(void))(int)
{
- CatchSignal(SIGCLD, sig_cld);
+ return CatchSignal(SIGCLD, sig_cld);
}
/**
Catch SIGCLD but leave the child around so it's status can be reaped.
**/
-void CatchChildLeaveStatus(void)
+void (*CatchChildLeaveStatus(void))(int)
{
- CatchSignal(SIGCLD, sig_cld_leave_status);
+ return CatchSignal(SIGCLD, sig_cld_leave_status);
}
diff --git a/ctdb/lib/util/util.h b/ctdb/lib/util/util.h
index 33f46bd..57a6a82 100644
--- a/ctdb/lib/util/util.h
+++ b/ctdb/lib/util/util.h
@@ -100,12 +100,12 @@ void (*CatchSignal(int signum,void (*handler)(int )))(int);
/**
Ignore SIGCLD via whatever means is necessary for this OS.
**/
-void CatchChild(void);
+void (*CatchChild(void))(int);
/**
Catch SIGCLD but leave the child around so it's status can be reaped.
**/
-void CatchChildLeaveStatus(void);
+void (*CatchChildLeaveStatus(void))(int);
/* The following definitions come from lib/util/util_str.c */
diff --git a/lib/util/samba_util.h b/lib/util/samba_util.h
index 9da61fa..e9de6fa 100644
--- a/lib/util/samba_util.h
+++ b/lib/util/samba_util.h
@@ -108,12 +108,12 @@ void (*CatchSignal(int signum,void (*handler)(int )))(int);
/**
Ignore SIGCLD via whatever means is necessary for this OS.
**/
-void CatchChild(void);
+void (*CatchChild(void))(int);
/**
Catch SIGCLD but leave the child around so it's status can be reaped.
**/
-void CatchChildLeaveStatus(void);
+void (*CatchChildLeaveStatus(void))(int);
struct sockaddr;
diff --git a/lib/util/signal.c b/lib/util/signal.c
index ead947e..33a9900 100644
--- a/lib/util/signal.c
+++ b/lib/util/signal.c
@@ -129,16 +129,16 @@ void (*CatchSignal(int signum,void (*handler)(int )))(int)
Ignore SIGCLD via whatever means is necessary for this OS.
**/
-void CatchChild(void)
+void (*CatchChild(void))(int)
{
- CatchSignal(SIGCLD, sig_cld);
+ return CatchSignal(SIGCLD, sig_cld);
}
/**
Catch SIGCLD but leave the child around so it's status can be reaped.
**/
-void CatchChildLeaveStatus(void)
+void (*CatchChildLeaveStatus(void))(int)
{
- CatchSignal(SIGCLD, sig_cld_leave_status);
+ return CatchSignal(SIGCLD, sig_cld_leave_status);
}
--
2.1.0.rc2.206.gedb03e5
From 71b96439498456cc25936ae662e3bc6c0c29f0b3 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 23 Sep 2014 14:51:18 -0700
Subject: [PATCH 2/2] s3: lib: Signal handling - ensure smbrun and change
password code save and restore existing SIGCHLD handlers.
Bug #10831 - SIGCLD Signal handler not correctly reinstalled on old library code use - smbrun etc.
https://bugzilla.samba.org/show_bug.cgi?id=10831
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/lib/smbrun.c | 18 ++++++++++--------
source3/rpc_server/samr/srv_samr_chgpasswd.c | 9 +++++----
2 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/source3/lib/smbrun.c b/source3/lib/smbrun.c
index 15a0c88..55f7a87 100644
--- a/source3/lib/smbrun.c
+++ b/source3/lib/smbrun.c
@@ -73,6 +73,7 @@ static int smbrun_internal(const char *cmd, int *outfd, bool sanitize)
pid_t pid;
uid_t uid = current_user.ut.uid;
gid_t gid = current_user.ut.gid;
+ void (*saved_handler)(int);
/*
* Lose any elevated privileges.
@@ -94,11 +95,11 @@ static int smbrun_internal(const char *cmd, int *outfd, bool sanitize)
* SIGCLD signals as it also eats the exit status code. JRA.
*/
- CatchChildLeaveStatus();
+ saved_handler = CatchChildLeaveStatus();
if ((pid=fork()) < 0) {
DEBUG(0,("smbrun: fork failed with error %s\n", strerror(errno) ));
- CatchChild();
+ (void)CatchSignal(SIGCLD, saved_handler);
if (outfd) {
close(*outfd);
*outfd = -1;
@@ -123,7 +124,7 @@ static int smbrun_internal(const char *cmd, int *outfd, bool sanitize)
break;
}
- CatchChild();
+ (void)CatchSignal(SIGCLD, saved_handler);
if (wpid != pid) {
DEBUG(2,("waitpid(%d) : %s\n",(int)pid,strerror(errno)));
@@ -148,7 +149,7 @@ static int smbrun_internal(const char *cmd, int *outfd, bool sanitize)
return status;
}
- CatchChild();
+ (void)CatchChild();
/* we are in the child. we exec /bin/sh to do the work for us. we
don't directly exec the command we want because it may be a
@@ -237,6 +238,7 @@ int smbrunsecret(const char *cmd, const char *secret)
uid_t uid = current_user.ut.uid;
gid_t gid = current_user.ut.gid;
int ifd[2];
+ void (*saved_handler)(int);
/*
* Lose any elevated privileges.
@@ -257,11 +259,11 @@ int smbrunsecret(const char *cmd, const char *secret)
* SIGCLD signals as it also eats the exit status code. JRA.
*/
- CatchChildLeaveStatus();
+ saved_handler = CatchChildLeaveStatus();
if ((pid=fork()) < 0) {
DEBUG(0, ("smbrunsecret: fork failed with error %s\n", strerror(errno)));
- CatchChild();
+ (void)CatchSignal(SIGCLD, saved_handler);
return errno;
}
@@ -293,7 +295,7 @@ int smbrunsecret(const char *cmd, const char *secret)
break;
}
- CatchChild();
+ (void)CatchSignal(SIGCLD, saved_handler);
if (wpid != pid) {
DEBUG(2, ("waitpid(%d) : %s\n", (int)pid, strerror(errno)));
@@ -309,7 +311,7 @@ int smbrunsecret(const char *cmd, const char *secret)
return status;
}
- CatchChild();
+ (void)CatchChild();
/* we are in the child. we exec /bin/sh to do the work for us. we
don't directly exec the command we want because it may be a
diff --git a/source3/rpc_server/samr/srv_samr_chgpasswd.c b/source3/rpc_server/samr/srv_samr_chgpasswd.c
index 1c9c33a..684ccee 100644
--- a/source3/rpc_server/samr/srv_samr_chgpasswd.c
+++ b/source3/rpc_server/samr/srv_samr_chgpasswd.c
@@ -391,6 +391,7 @@ static bool chat_with_program(char *passwordprogram, const struct passwd *pass,
pid_t pid, wpid;
int wstat;
bool chstat = False;
+ void (*saved_handler)(int);
if (pass == NULL) {
DEBUG(0, ("chat_with_program: user doesn't exist in the UNIX password database.\n"));
@@ -408,13 +409,13 @@ static bool chat_with_program(char *passwordprogram, const struct passwd *pass,
* SIGCLD signals as it also eats the exit status code. JRA.
*/
- CatchChildLeaveStatus();
+ saved_handler = CatchChildLeaveStatus();
if ((pid = fork()) < 0) {
DEBUG(3, ("chat_with_program: Cannot fork() child for password change: %s\n", pass->pw_name));
SAFE_FREE(slavedev);
close(master);
- CatchChild();
+ (void)CatchSignal(SIGCLD, saved_handler);
return (False);
}
@@ -439,14 +440,14 @@ static bool chat_with_program(char *passwordprogram, const struct passwd *pass,
if (wpid < 0) {
DEBUG(3, ("chat_with_program: The process is no longer waiting!\n\n"));
close(master);
- CatchChild();
+ (void)CatchSignal(SIGCLD, saved_handler);
return (False);
}
/*
* Go back to ignoring children.
*/
- CatchChild();
+ (void)CatchSignal(SIGCLD, saved_handler);
close(master);
--
2.1.0.rc2.206.gedb03e5
More information about the samba-technical
mailing list