Skip to content

Conversation

@martinetd
Copy link
Contributor

The signal handler in openrc-run expects service_pid to be set when the child
process exists, but there is no guarantee that the execution thread will have
been scheduled before the signal handler, leading to service not being detected
as started properly very occasionally.

This can be tested with the following reproducer:

#include <signal.h>
#include <stdio.h>
#include <string.h>
#include <sys/wait.h>
#include <unistd.h>

static pid_t service_pid;

static void
handle_signal(int sig)
{
	int status;
	pid_t pid;

	switch (sig) {
	case SIGCHLD:
		while ((pid = waitpid(-1, &status, WNOHANG)) > 0) {
			if (pid != service_pid)
				printf("Got %d != %d\n", pid, service_pid);
		}
		break;
	default:
		printf("caught unknown signal %d\n", sig);
	}
}

int
signal_setup(int sig, void (*handler)(int))
{
        struct sigaction sa;

        memset(&sa, 0, sizeof (sa));
        sigemptyset(&sa.sa_mask);
        sa.sa_handler = handler;
        return sigaction(sig, &sa, NULL);
}

int main(int argc, char **argv)
{
	signal_setup(SIGCHLD, handle_signal);

	while (1) {
#ifdef FIX
		sigset_t full, old;
		sigfillset(&full);
		sigprocmask(SIG_SETMASK, &full, &old);
#endif
		service_pid = fork();
#ifdef FIX
		sigprocmask(SIG_SETMASK, &old, NULL);
#endif

		if (service_pid < 0) {
			printf("fork fail %m\n");
			return 1;
		}
		// exit child immediately
		if (service_pid == 0)
			return 0;
		// wait for child process to stop a different way
		while (kill(service_pid, 0) >= 0);
	}

	return 0;
}

Fixes #895

martinetd added 2 commits July 5, 2025 09:54
The signal handler in openrc-run expects service_pid to be set when the child
process exists, but there is no guarantee that the execution thread will have
been scheduled before the signal handler, leading to service not being detected
as started properly very occasionally.

This can be tested with the following reproducer:
```
#include <signal.h>
#include <stdio.h>
#include <string.h>
#include <sys/wait.h>
#include <unistd.h>

static pid_t service_pid;

static void
handle_signal(int sig)
{
	int status;
	pid_t pid;

	switch (sig) {
	case SIGCHLD:
		while ((pid = waitpid(-1, &status, WNOHANG)) > 0) {
			if (pid != service_pid)
				printf("Got %d != %d\n", pid, service_pid);
		}
		break;
	default:
		printf("caught unknown signal %d\n", sig);
	}
}

int
signal_setup(int sig, void (*handler)(int))
{
        struct sigaction sa;

        memset(&sa, 0, sizeof (sa));
        sigemptyset(&sa.sa_mask);
        sa.sa_handler = handler;
        return sigaction(sig, &sa, NULL);
}

int main(int argc, char **argv)
{
	signal_setup(SIGCHLD, handle_signal);

	while (1) {
#ifdef FIX
		sigset_t full, old;
		sigfillset(&full);
		sigprocmask(SIG_SETMASK, &full, &old);
#endif
		service_pid = fork();
#ifdef FIX
		sigprocmask(SIG_SETMASK, &old, NULL);
#endif

		if (service_pid < 0) {
			printf("fork fail %m\n");
			return 1;
		}
		// exit child immediately
		if (service_pid == 0)
			return 0;
		// wait for child process to stop a different way
		while (kill(service_pid, 0) >= 0);
	}

	return 0;
}
```

Fixes OpenRC#895
In the unlikely case the service timed out and the child process exited
just as we closed the file descriptor, the signal handler could try
writing to an invalid fd.

There is no other thread around so this is harmless but might as well
unset the variable first.
@navi-desu navi-desu merged commit 03d0774 into OpenRC:master Jul 5, 2025
7 checks passed
@navi-desu
Copy link
Member

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

child sometimes not reaped properly in openrc-run svc_exec(), signal_pipe never written to

2 participants