Bug 8596

Summary:	start-stop-daemon has race condition on --make-pidfile option
Product:	Busybox	Reporter:	craig+busybox
Component:	Other	Assignee:	unassigned
Status:	RESOLVED FIXED
Severity:	normal	CC:	busybox-cvs
Priority:	P5
Version:	1.22.x
Target Milestone:	---
Hardware:	All
OS:	Linux
Host:		Target:
Build:

Description craig+busybox 2016-01-13 04:05:37 UTC

For start-stop-daemon, when the --make-pidfile (-m) option is used, ideally the PID file should be created before start-stop-daemon exits. But that is not the case--the PID file is made by the background process at some undetermined time before or after the foreground process exits.

This is a "race condition" if there is a subsequent process that needs to access the PID file. For example, the 'watchdog' program that monitors a process is running via its PID file. I have found that 'watchdog' program can reboot my system because of such a race condition: It is monitoring a process via its PID file, but the PID file is not yet created when 'watchdog' starts running.

One fix is for the parent process to write the PID file, after getting the child process' PID returned from the bb_daemon() call.

An alternative fix would be for the parent process to wait until it gets some sort of signal from the child to indicate that the child has written the PID.

Comment 1 craig+busybox 2016-01-13 04:07:09 UTC

I should have also said, I'm talking about the scenario when also using the --background (-b) option.

Comment 2 craig+busybox 2016-04-06 08:12:58 UTC

Is the following code any good as a reference/idea? The essential idea is:

* After first fork, the parent waits for the child to exit.
* After the second fork, the child writes the PID of the grandchild to the pidfile, then exits.

int daemon_with_pid(int pid_fd)
{
	int	fd;
	pid_t	pid;
	pid_t	pid_wait;
	int	stat;
	int	file_bytes;
	char	pidfile_buffer[32];

	pid = fork();
	if (pid < 0) {
		perror("daemon fork");
		exit(20);
	}
	if (pid > 0) {
		/* We are the parent.
		 * Wait for child to exit. The child will do a second fork,
		 * write the PID of the grandchild to the pidfile, then exit.
		 * We wait for this to avoid race condition on pidfile writing.
		 * I.e. when we exit, pidfile contents are guaranteed valid. */
		for (;;) {
			pid_wait = waitpid(pid, &stat, 0);
			if (pid_wait == -1 && errno == EINTR)
				continue;
			if (WIFSTOPPED(stat) || WIFCONTINUED(stat))
				continue;
			break;
		}
		if (WIFEXITED(stat)) {
			if (WEXITSTATUS(stat) != 0) {
				fprintf(stderr, "Error in child process\n");
				exit(WEXITSTATUS(stat));
			}
			_exit(0);
		}
		_exit(21);
	}

	/* We are the child. Set up for daemon and then do second fork. */
	/* Set current directory to / */
	chdir("/");

	/* Redirect STDIN, STDOUT, STDERR to /dev/null */
	fd = open("/dev/null", O_RDWR);
	if (fd < 0)
		_exit(22);
	stat = dup2(fd, STDIN_FILENO);
	if (stat < 0)
		_exit(23);
	stat = dup2(fd, STDOUT_FILENO);
	if (stat < 0)
		_exit(23);
	stat = dup2(fd, STDERR_FILENO);
	if (stat < 0)
		_exit(23);

	/* Start a new session for the daemon. */
	setsid();

	/* Do a second fork */
	pid = fork();
	if (pid < 0) {
		_exit(24);
	}
	if (pid > 0) {
		/* We are the parent in this second fork; child of the first fork.
		 * Write the PID to the pidfile, then exit. */
		if (pid_fd >= 0) {
			file_bytes = snprintf(pidfile_buffer, sizeof(pidfile_buffer), "%d\n", pid);
			if (file_bytes <= 0)
				_exit(25);
			stat = ftruncate(pid_fd, 0);
			if (stat < 0)
				_exit(26);
			stat = lseek(pid_fd, 0, SEEK_SET);
			if (stat < 0)
				_exit(27);
			stat = write(pid_fd, pidfile_buffer, file_bytes);
			if (stat < file_bytes)
				_exit(28);
		}
		_exit(0);

	}
	/* We are the child of the second fork; grandchild of the first fork. */
	return 0;
}

Comment 3 Denys Vlasenko 2019-01-14 13:49:10 UTC

Fixed in git.