URL: https://github.com/SSSD/sssd/pull/924
Author: alexey-tikhonov
Title: #924: [WiP] Amend util/server.c:become_daemon()
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/924/head:pr924
git checkout pr924
From a8bd9c4b773e83fe06f1ae370605866fb6371870 Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikh...@redhat.com>
Date: Fri, 1 Nov 2019 13:36:37 +0100
Subject: [PATCH 1/7] util/server.c: become_daemon() made static
become_daemon() is not intended to be used outside of server.c
hence marked as static function.
---
src/util/server.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/server.c b/src/util/server.c
index 8b927069d2..0a44baa18d 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -67,7 +67,7 @@ static void daemon_parent_sigterm(int sig)
Become a daemon, discarding the controlling terminal.
**/
-void become_daemon(bool Fork)
+static void become_daemon(bool Fork)
{
pid_t pid, cpid;
int status;
From e1dd4c0513851742c775ee32dad6d32eddac4f28 Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikh...@redhat.com>
Date: Fri, 1 Nov 2019 13:39:48 +0100
Subject: [PATCH 2/7] server:become_daemon(): got rid of unused codepath
become_daemon() was never called with "Fork == false" (and there are
seems to be no reasons to), so this argument was removed.
---
src/util/server.c | 70 +++++++++++++++++++++++------------------------
1 file changed, 34 insertions(+), 36 deletions(-)
diff --git a/src/util/server.c b/src/util/server.c
index 0a44baa18d..f6f74b32e7 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -67,50 +67,48 @@ static void daemon_parent_sigterm(int sig)
Become a daemon, discarding the controlling terminal.
**/
-static void become_daemon(bool Fork)
+static void become_daemon(void)
{
pid_t pid, cpid;
int status;
int ret, error;
- if (Fork) {
- pid = fork();
- if (pid != 0) {
- /* Terminate parent process on demand so we can hold systemd
- * or initd from starting next service until SSSD is initialized.
- * We use signals directly here because we don't have a tevent
- * context yet. */
- CatchSignal(SIGTERM, daemon_parent_sigterm);
-
- /* or exit when sssd monitor is terminated */
- do {
- errno = 0;
- cpid = waitpid(pid, &status, 0);
- if (cpid == -1) {
- /* An error occurred while waiting */
- error = errno;
- if (error != EINTR) {
- DEBUG(SSSDBG_CRIT_FAILURE,
- "Error [%d][%s] while waiting for child\n",
- error, strerror(error));
- /* Forcibly kill this child */
- kill(pid, SIGKILL);
- ret = 1;
- }
+ pid = fork();
+ if (pid != 0) {
+ /* Terminate parent process on demand so we can hold systemd
+ * or initd from starting next service until SSSD is initialized.
+ * We use signals directly here because we don't have a tevent
+ * context yet. */
+ CatchSignal(SIGTERM, daemon_parent_sigterm);
+
+ /* or exit when sssd monitor is terminated */
+ do {
+ errno = 0;
+ cpid = waitpid(pid, &status, 0);
+ if (cpid == -1) {
+ /* An error occurred while waiting */
+ error = errno;
+ if (error != EINTR) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Error [%d][%s] while waiting for child\n",
+ error, strerror(error));
+ /* Forcibly kill this child */
+ kill(pid, SIGKILL);
+ ret = 1;
}
+ }
- error = 0;
- /* return error if we didn't exited normally */
- ret = 1;
+ error = 0;
+ /* return error if we didn't exited normally */
+ ret = 1;
- if (WIFEXITED(status)) {
- /* but return our exit code otherwise */
- ret = WEXITSTATUS(status);
- }
- } while (error == EINTR);
+ if (WIFEXITED(status)) {
+ /* but return our exit code otherwise */
+ ret = WEXITSTATUS(status);
+ }
+ } while (error == EINTR);
- _exit(ret);
- }
+ _exit(ret);
}
/* detach from the terminal */
@@ -495,7 +493,7 @@ int server_setup(const char *name, int flags,
if (flags & FLAGS_DAEMON) {
DEBUG(SSSDBG_IMPORTANT_INFO, "Becoming a daemon.\n");
- become_daemon(true);
+ become_daemon();
}
if (flags & FLAGS_PID_FILE) {
From 42dd1b1732b7bdc0c724742364a8673a05693160 Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikh...@redhat.com>
Date: Fri, 1 Nov 2019 13:56:56 +0100
Subject: [PATCH 3/7] server:become_daemon(): handle fail of fork()
---
src/util/server.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/util/server.c b/src/util/server.c
index f6f74b32e7..cedb18aa7c 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -74,6 +74,12 @@ static void become_daemon(void)
int ret, error;
pid = fork();
+ if (pid == -1) {
+ DEBUG(SSSDBG_FATAL_FAILURE, "fork() failed: %d [%s]\n",
+ errno, strerror(errno));
+ sss_log(SSS_LOG_ERR, "can't start: fork() failed");
+ _exit(1);
+ }
if (pid != 0) {
/* Terminate parent process on demand so we can hold systemd
* or initd from starting next service until SSSD is initialized.
From 69921775bfea93476d52579d428f95ad4eb317ed Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikh...@redhat.com>
Date: Fri, 1 Nov 2019 16:06:51 +0100
Subject: [PATCH 4/7] server:become_daemon(): fixed waitpid()-loop
Setting "error = 0" right before "while (error == EINTR);"
didn't make any sense (or rather was a bug).
---
src/util/server.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/util/server.c b/src/util/server.c
index cedb18aa7c..b11b9580a2 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -87,9 +87,9 @@ static void become_daemon(void)
* context yet. */
CatchSignal(SIGTERM, daemon_parent_sigterm);
- /* or exit when sssd monitor is terminated */
+ /* or exit when child process (i.e. sssd monitor) is terminated */
do {
- errno = 0;
+ error = 0;
cpid = waitpid(pid, &status, 0);
if (cpid == -1) {
/* An error occurred while waiting */
@@ -104,7 +104,6 @@ static void become_daemon(void)
}
}
- error = 0;
/* return error if we didn't exited normally */
ret = 1;
From bf9f3b3958717df58968ce98203bf1dc46214308 Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikh...@redhat.com>
Date: Fri, 1 Nov 2019 16:32:31 +0100
Subject: [PATCH 5/7] server:become_daemon(): fix read of uninitialized value
It was wrong to use `status` in case of failed waitpid()
since value was uninitialized in this case.
---
src/util/server.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/src/util/server.c b/src/util/server.c
index b11b9580a2..52d4414f39 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -87,7 +87,9 @@ static void become_daemon(void)
* context yet. */
CatchSignal(SIGTERM, daemon_parent_sigterm);
- /* or exit when child process (i.e. sssd monitor) is terminated */
+ /* or exit when child process (i.e. sssd monitor) is terminated
+ * and return error in this case */
+ ret = 1;
do {
error = 0;
cpid = waitpid(pid, &status, 0);
@@ -100,16 +102,12 @@ static void become_daemon(void)
error, strerror(error));
/* Forcibly kill this child */
kill(pid, SIGKILL);
- ret = 1;
}
- }
-
- /* return error if we didn't exited normally */
- ret = 1;
-
- if (WIFEXITED(status)) {
- /* but return our exit code otherwise */
- ret = WEXITSTATUS(status);
+ } else {
+ if (WIFEXITED(status)) {
+ /* return our exit code if available */
+ ret = WEXITSTATUS(status);
+ }
}
} while (error == EINTR);
From 942cd4a1f21cff94e0aa103afdd47576308c4ab4 Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikh...@redhat.com>
Date: Fri, 1 Nov 2019 16:36:54 +0100
Subject: [PATCH 6/7] server:become_daemon(): change handling of chdir() fail
It didn't make any sense to skip close_low_fds() but keep working
in case of chdir() fail.
---
src/util/server.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/util/server.c b/src/util/server.c
index 52d4414f39..d04610367e 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -123,7 +123,6 @@ static void become_daemon(void)
ret = errno;
DEBUG(SSSDBG_FATAL_FAILURE, "Cannot change directory (%d [%s])\n",
ret, strerror(ret));
- return;
}
/* Close FDs 0,1,2. Needed if started by rsh */
From 1a391e65d7472376e1ef379b873ca370e640687d Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikh...@redhat.com>
Date: Fri, 1 Nov 2019 16:51:33 +0100
Subject: [PATCH 7/7] server:become_daemon(): handle fail of setsid()
---
src/util/server.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/util/server.c b/src/util/server.c
index d04610367e..57971cd620 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -114,8 +114,14 @@ static void become_daemon(void)
_exit(ret);
}
- /* detach from the terminal */
- setsid();
+ /* create new session, process group and detach from the terminal */
+ if (setsid() == (pid_t) -1) {
+ ret = errno;
+ DEBUG(SSSDBG_FATAL_FAILURE, "setsid() failed: %d [%s]\n",
+ ret, strerror(ret));
+ sss_log(SSS_LOG_ERR, "can't start: setsid() failed");
+ _exit(1);
+ }
/* chdir to / to be sure we're not on a remote filesystem */
errno = 0;
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
