3293bc146

Denys Vlasenko

2018-03-10

SUCCESS Duration: 139s

Subject

udhcpd: fix "not dying on SIGTERM"

No binary (build failed)

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

(log yok)

examples/var_service/dhcpd_if/run+2 -2

@@ -11,13 +11,13 @@ echo "* Upping iface $if"
	ip link set dev $if up

	>>udhcpd.leases
-	sed 's/^interface.*$/interface '"$if/" -i udhcpc.conf
+	sed 's/^interface.*$/interface '"$if/" -i udhcpd.conf

	echo "* Starting udhcpd"
	exec \
	env - PATH="$PATH" \
	softlimit \
	setuidgid root \
-	udhcpd -f -vv udhcpc.conf
+	udhcpd -f -vv udhcpd.conf

	exit $?

?+0 -0

networking/udhcp/common.h+1 -1

@@ -314,7 +314,7 @@ int udhcp_send_kernel_packet(struct dhcp_packet *dhcp_pkt,

	void udhcp_sp_setup(void) FAST_FUNC;
	void udhcp_sp_fd_set(struct pollfd *pfds, int extra_fd) FAST_FUNC;
-	int udhcp_sp_read(struct pollfd *pfds) FAST_FUNC;
+	int udhcp_sp_read(void) FAST_FUNC;

	int udhcp_read_interface(const char interface, int ifindex, uint32_t nip, uint8_t mac) FAST_FUNC;

networking/udhcp/d6_dhcpc.c+4 -5

@@ -1375,13 +1375,12 @@ int udhcpc6_main(int argc UNUSED_PARAM, char **argv)
	/* yah, I know, you say it would never happen */
	timeout = INT_MAX;
	continue; /* back to main loop */
-	} /* if select timed out */
+	} /* if poll timed out */

-	/* select() didn't timeout, something happened */
+	/* poll() didn't timeout, something happened */

	/* Is it a signal? */
-	/* note: udhcp_sp_read checks poll result before reading */
-	switch (udhcp_sp_read(pfds)) {
+	switch (udhcp_sp_read()) {
	case SIGUSR1:
	client_config.first_secs = 0; /* make secs field count from 0 */
	already_waited_sec = 0;
@@ -1416,7 +1415,7 @@ int udhcpc6_main(int argc UNUSED_PARAM, char **argv)
	}

	/* Is it a packet? */
-	if (listen_mode == LISTEN_NONE \|\| !pfds[1].revents)
+	if (!pfds[1].revents)
	continue; /* no */

	{

networking/udhcp/dhcpc.c+4 -5

@@ -1574,13 +1574,12 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
	/* yah, I know, you say it would never happen */
	timeout = INT_MAX;
	continue; /* back to main loop */
-	} /* if select timed out */
+	} /* if poll timed out */

-	/* select() didn't timeout, something happened */
+	/* poll() didn't timeout, something happened */

	/* Is it a signal? */
-	/* note: udhcp_sp_read checks poll result before reading */
-	switch (udhcp_sp_read(pfds)) {
+	switch (udhcp_sp_read()) {
	case SIGUSR1:
	client_config.first_secs = 0; /* make secs field count from 0 */
	already_waited_sec = 0;
@@ -1615,7 +1614,7 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
	}

	/* Is it a packet? */
-	if (listen_mode == LISTEN_NONE \|\| !pfds[1].revents)
+	if (!pfds[1].revents)
	continue; /* no */

	{

networking/udhcp/dhcpd.c+18 -16

@@ -915,20 +915,18 @@ int udhcpd_main(int argc UNUSED_PARAM, char **argv)

	udhcp_sp_fd_set(pfds, server_socket);
	tv = timeout_end - monotonic_sec();
-	retval = 0;
-	if (!server_config.auto_time \|\| tv > 0) {
-	retval = poll(pfds, 2, server_config.auto_time ? tv * 1000 : -1);
-	}
-	if (retval == 0) {
-	write_leases();
-	goto continue_with_autotime;
-	}
-	if (retval < 0 && errno != EINTR) {
-	log1("error on select");
-	continue;
+	/* Block here waiting for either signal or packet */
+	retval = safe_poll(pfds, 2, server_config.auto_time ? tv * 1000 : -1);
+	if (retval <= 0) {
+	if (retval == 0) {
+	write_leases();
+	goto continue_with_autotime;
+	}
+	/* < 0 and not EINTR: should not happen */
+	bb_perror_msg_and_die("poll");
	}

-	switch (udhcp_sp_read(pfds)) {
+	if (pfds[0].revents) switch (udhcp_sp_read()) {
	case SIGUSR1:
	bb_error_msg("received %s", "SIGUSR1");
	write_leases();
@@ -938,12 +936,16 @@ int udhcpd_main(int argc UNUSED_PARAM, char **argv)
	bb_error_msg("received %s", "SIGTERM");
	write_leases();
	goto ret0;
-	case 0: /* no signal: read a packet */
-	break;
-	default: /* signal or error (probably EINTR): back to select */
-	continue;
	}

+	/* Is it a packet? */
+	if (!pfds[1].revents)
+	continue; /* no */
+
+	/* Note: we do not block here, we block on poll() instead.
+	* Blocking here would prevent SIGTERM from working:
+	* socket read inside this call is restarted on caught signals.
+	*/
	bytes = udhcp_recv_kernel_packet(&packet, server_socket);
	if (bytes < 0) {
	/* bytes can also be -2 ("bad packet data") */

networking/udhcp/signalpipe.c+6 -5

@@ -40,6 +40,7 @@ void FAST_FUNC udhcp_sp_setup(void)
	xpiped_pair(signal_pipe);
	close_on_exec_on(signal_pipe.rd);
	close_on_exec_on(signal_pipe.wr);
+	ndelay_on(signal_pipe.rd);
	ndelay_on(signal_pipe.wr);
	bb_signals(0
	+ (1 << SIGUSR1)
@@ -61,20 +62,20 @@ void FAST_FUNC udhcp_sp_fd_set(struct pollfd pfds[2], int extra_fd)
	pfds[1].fd = extra_fd;
	pfds[1].events = POLLIN;
	}
+	/* this simplifies "is extra_fd ready?" tests elsewhere: */
+	pfds[1].revents = 0;
	}

	/* Read a signal from the signal pipe. Returns 0 if there is
	* no signal, -1 on error (and sets errno appropriately), and
	* your signal on success */
-	int FAST_FUNC udhcp_sp_read(struct pollfd pfds[2])
+	int FAST_FUNC udhcp_sp_read(void)
	{
	unsigned char sig;

-	if (!pfds[0].revents)
-	return 0;
-
+	/* Can't block here, fd is in nonblocking mode */
	if (safe_read(signal_pipe.rd, &sig, 1) != 1)
-	return -1;
+	return 0;

	return sig;
	}