Signed-off-by: Visser Sander <sander.visse...@consultant.volvo.com> Signed-off-by: Andrej Valek <andrej.va...@siemens.com> --- .../busybox-udhcpd-fix-not-dying-on-SIGTERM.patch | 272 +++++++++++++++++++++ meta/recipes-core/busybox/busybox_1.27.2.bb | 1 + 2 files changed, 273 insertions(+) create mode 100644 meta/recipes-core/busybox/busybox/busybox-udhcpd-fix-not-dying-on-SIGTERM.patch
diff --git a/meta/recipes-core/busybox/busybox/busybox-udhcpd-fix-not-dying-on-SIGTERM.patch b/meta/recipes-core/busybox/busybox/busybox-udhcpd-fix-not-dying-on-SIGTERM.patch new file mode 100644 index 0000000000..96be486c61 --- /dev/null +++ b/meta/recipes-core/busybox/busybox/busybox-udhcpd-fix-not-dying-on-SIGTERM.patch @@ -0,0 +1,272 @@ +From 1384459d88cb0f5f47b6a36b8346dcf425a643f5 Mon Sep 17 00:00:00 2001 +From: Denys Vlasenko <vda.li...@googlemail.com> +Date: Sat, 10 Mar 2018 19:01:48 +0100 +Subject: [PATCH] udhcpd: fix "not dying on SIGTERM" + +Upstream-status: Backport [https://git.busybox.net/busybox/commit/?id=3293bc146985c56790033409facc0ad64a471084] + +Fixes: + commit 52a515d18724bbb34e3ccbbb0218efcc4eccc0a8 + "udhcp: use poll() instead of select()" + Feb 16 2017 + +udhcp_sp_read() is meant to check whether signal pipe indeed has some data to read. +In the above commit, it was changed as follows: + +- if (!FD_ISSET(signal_pipe.rd, rfds)) ++ if (!pfds[0].revents) + return 0; + +The problem is, the check was working for select() purely by accident. +Caught signal interrupts select()/poll() syscalls, they return with EINTR +(regardless of SA_RESTART flag in sigaction). _Then_ signal handler is invoked. +IOW: they can't see any changes to fd state caused by signal haldler +(in our case, signal handler makes signal pipe ready to be read). + +For select(), it means that rfds[] bit array is unmodified, bit of signal +pipe's read fd is still set, and the above check "works": it thinks select() +says there is data to read. + +This accident does not work for poll(): .revents stays clear, and we do not +try reading signal pipe as we should. In udhcpd, we fall through and block +in socket read. Further SIGTERM signals simply cause socket read to be +interrupted and then restarted (since SIGTERM handler has SA_RESTART=1). + +Fixing this as follows: remove the check altogether. Set signal pipe read fd +to nonblocking mode. Always read it in udhcp_sp_read(). +If read fails, assume it's EAGAIN and return 0 ("no signal seen"). + +udhcpd avoids reading signal pipe on every recvd packet by looping if EINTR +(using safe_poll()) - thus ensuring we have correct .revents for all fds - +and calling udhcp_sp_read() only if pfds[0].revents!=0. + +udhcpc performs much fewer reads (typically it sleeps >99.999% of the time), +there is no need to optimize it: can call udhcp_sp_read() after each poll +unconditionally. + +To robustify socket reads, unconditionally set pfds[1].revents=0 +in udhcp_sp_fd_set() (which is before poll), and check it before reading +network socket in udhcpd. + +TODO: +This might still fail: if pfds[1].revents=POLLIN, socket read may still block. +There are rare cases when select/poll indicates that data can be read, +but then actual read still blocks (one such case is UDP packets with +wrong checksum). General advise is, if you use a poll/select loop, +keep all your fds nonblocking. +Maybe we should also do that to our network sockets? + +function old new delta +udhcp_sp_setup 55 65 +10 +udhcp_sp_fd_set 54 60 +6 +udhcp_sp_read 46 36 -10 +udhcpd_main 1451 1437 -14 +udhcpc_main 2723 2708 -15 +------------------------------------------------------------------------------ +(add/remove: 0/0 grow/shrink: 2/3 up/down: 16/-39) Total: -23 bytes + +Signed-off-by: Denys Vlasenko <vda.li...@googlemail.com> +--- + examples/var_service/dhcpd_if/run | 4 +-- + .../dhcpd_if/{udhcpc.conf => udhcpd.conf} | 0 + networking/udhcp/common.h | 2 +- + networking/udhcp/d6_dhcpc.c | 9 +++--- + networking/udhcp/dhcpc.c | 9 +++--- + networking/udhcp/dhcpd.c | 34 ++++++++++++---------- + networking/udhcp/signalpipe.c | 11 +++---- + 7 files changed, 35 insertions(+), 34 deletions(-) + rename examples/var_service/dhcpd_if/{udhcpc.conf => udhcpd.conf} (100%) + +diff --git a/examples/var_service/dhcpd_if/run b/examples/var_service/dhcpd_if/run +index de85dece0..a603bdc71 100755 +--- a/examples/var_service/dhcpd_if/run ++++ b/examples/var_service/dhcpd_if/run +@@ -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 $? +diff --git a/examples/var_service/dhcpd_if/udhcpc.conf b/examples/var_service/dhcpd_if/udhcpd.conf +similarity index 100% +rename from examples/var_service/dhcpd_if/udhcpc.conf +rename to examples/var_service/dhcpd_if/udhcpd.conf +diff --git a/networking/udhcp/common.h b/networking/udhcp/common.h +index a9c23a186..866f3d8b1 100644 +--- a/networking/udhcp/common.h ++++ b/networking/udhcp/common.h +@@ -312,7 +312,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; + +diff --git a/networking/udhcp/d6_dhcpc.c b/networking/udhcp/d6_dhcpc.c +index f6d3fb98b..bdf8dad17 100644 +--- a/networking/udhcp/d6_dhcpc.c ++++ b/networking/udhcp/d6_dhcpc.c +@@ -1371,13 +1371,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; +@@ -1412,7 +1411,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 */ + + { +diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c +index 1a66c610e..c8027983e 100644 +--- a/networking/udhcp/dhcpc.c ++++ b/networking/udhcp/dhcpc.c +@@ -1588,13 +1588,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; +@@ -1629,7 +1628,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 */ + + { +diff --git a/networking/udhcp/dhcpd.c b/networking/udhcp/dhcpd.c +index 3a5fc2db7..c4e990a48 100644 +--- a/networking/udhcp/dhcpd.c ++++ b/networking/udhcp/dhcpd.c +@@ -912,20 +912,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(); +@@ -935,12 +933,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") */ +diff --git a/networking/udhcp/signalpipe.c b/networking/udhcp/signalpipe.c +index b101b4ce4..2ff78f0f2 100644 +--- a/networking/udhcp/signalpipe.c ++++ b/networking/udhcp/signalpipe.c +@@ -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; + } +-- +2.16.4 diff --git a/meta/recipes-core/busybox/busybox_1.27.2.bb b/meta/recipes-core/busybox/busybox_1.27.2.bb index 716a0650fc..45369808ac 100644 --- a/meta/recipes-core/busybox/busybox_1.27.2.bb +++ b/meta/recipes-core/busybox/busybox_1.27.2.bb @@ -49,6 +49,7 @@ SRC_URI = "http://www.busybox.net/downloads/busybox-${PV}.tar.bz2;name=tarball \ file://busybox-fix-lzma-segfaults.patch \ file://umount-ignore-c.patch \ file://CVE-2017-15874.patch \ + file://busybox-udhcpd-fix-not-dying-on-SIGTERM.patch \ " SRC_URI_append_libc-musl = " file://musl.cfg " -- 2.11.0 -- _______________________________________________ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
