unwind(8): open ports before daemonizing

[Dave Voutila-2]

I noticed on my laptop that if I tried to start unwind(8) via rcctl(8)
while I already had some other process bound to tcp/udp port 53
(e.g. unbound(8)), rcctl(8) would falsely report the service started
"ok" while in /var/log/daemon unwind(8) would report:

  fatal in main: could not bind to 127.0.0.1 or ::1 on port 53

Since unwind(8) daemonizes before the exit(3) call, rcctl(8) doesn't
identify the failure.

The below diff splits unwind.c:open_ports() and moves the socket
bind/listen calls to before calling daemon(3). The imsg composition is
kept in the location of the original call to open_ports() in main(). The
sockets have SOCK_CLOEXEC added to them since they're now created before
the fork/execvp dance creating the frontend and resolver processes.

Feedback or OK?

-dv@


Index: sbin/unwind/unwind.c
===================================================================
RCS file: /cvs/src/sbin/unwind/unwind.c,v
retrieving revision 1.61
diff -u -p -r1.61 unwind.c
--- sbin/unwind/unwind.c 27 Feb 2021 10:32:28 -0000 1.61
+++ sbin/unwind/unwind.c 4 Apr 2021 14:57:52 -0000
@@ -56,6 +56,13 @@ enum uw_process {
  PROC_FRONTEND,
 };

+struct uw_socks {
+ int tcp4sock;
+ int tcp6sock;
+ int udp4sock;
+ int udp6sock;
+};
+
 __dead void usage(void);
 __dead void main_shutdown(void);

@@ -71,7 +78,7 @@ static int main_imsg_send_config(struct

 int main_reload(void);
 int main_sendall(enum imsg_type, void *, uint16_t);
-void open_ports(void);
+void open_ports(struct uw_socks *);
 void solicit_dns_proposals(void);
 void send_blocklist_fd(void);

@@ -127,6 +134,7 @@ main(int argc, char *argv[])
  int pipe_main2frontend[2], pipe_main2resolver[2];
  int control_fd, ta_fd;
  char *csock, *saved_argv0;
+ struct uw_socks socks = { -1, -1, -1, -1 };

  csock = _PATH_UNWIND_SOCKET;

@@ -203,6 +211,8 @@ main(int argc, char *argv[])
  log_init(debug, LOG_DAEMON);
  log_setverbose(cmd_opts & (OPT_VERBOSE | OPT_VERBOSE2 | OPT_VERBOSE3));

+ open_ports(&socks);
+
  if (!debug)
  daemon(1, 0);

@@ -259,7 +269,15 @@ main(int argc, char *argv[])
     &iev_resolver->ibuf))
  fatal("could not establish imsg links");

- open_ports();
+ /* Send opened sockets to the frontend */
+ if (socks.udp4sock != -1)
+ main_imsg_compose_frontend_fd(IMSG_UDP4SOCK, 0, socks.udp4sock);
+ if (socks.udp6sock != -1)
+ main_imsg_compose_frontend_fd(IMSG_UDP6SOCK, 0, socks.udp6sock);
+ if (socks.tcp4sock != -1)
+ main_imsg_compose_frontend_fd(IMSG_TCP4SOCK, 0, socks.tcp4sock);
+ if (socks.tcp6sock != -1)
+ main_imsg_compose_frontend_fd(IMSG_TCP6SOCK, 0, socks.tcp6sock);

  if ((control_fd = control_init(csock)) == -1)
  fatalx("control socket setup failed");
@@ -724,7 +742,7 @@ config_clear(struct uw_conf *conf)
 }

 void
-open_ports(void)
+open_ports(struct uw_socks *socks)
 {
  struct addrinfo hints, *res0;
  int udp4sock = -1, udp6sock = -1, error, bsize = 65535;
@@ -738,8 +756,9 @@ open_ports(void)

  error = getaddrinfo("127.0.0.1", "domain", &hints, &res0);
  if (!error && res0) {
- if ((udp4sock = socket(res0->ai_family, res0->ai_socktype,
-    res0->ai_protocol)) != -1) {
+ if ((udp4sock = socket(res0->ai_family,
+    res0->ai_socktype | SOCK_CLOEXEC,
+    res0->ai_protocol)) != -1) {
  if (setsockopt(udp4sock, SOL_SOCKET, SO_REUSEADDR,
     &opt, sizeof(opt)) == -1)
  log_warn("setting SO_REUSEADDR on socket");
@@ -759,8 +778,9 @@ open_ports(void)
  hints.ai_family = AF_INET6;
  error = getaddrinfo("::1", "domain", &hints, &res0);
  if (!error && res0) {
- if ((udp6sock = socket(res0->ai_family, res0->ai_socktype,
-    res0->ai_protocol)) != -1) {
+ if ((udp6sock = socket(res0->ai_family,
+    res0->ai_socktype | SOCK_CLOEXEC,
+    res0->ai_protocol)) != -1) {
  if (setsockopt(udp6sock, SOL_SOCKET, SO_REUSEADDR,
     &opt, sizeof(opt)) == -1)
  log_warn("setting SO_REUSEADDR on socket");
@@ -783,8 +803,8 @@ open_ports(void)
  error = getaddrinfo("127.0.0.1", "domain", &hints, &res0);
  if (!error && res0) {
  if ((tcp4sock = socket(res0->ai_family,
-    res0->ai_socktype | SOCK_NONBLOCK,
-    res0->ai_protocol)) != -1) {
+    res0->ai_socktype | SOCK_NONBLOCK | SOCK_CLOEXEC,
+    res0->ai_protocol)) != -1) {
  if (setsockopt(tcp4sock, SOL_SOCKET, SO_REUSEADDR,
     &opt, sizeof(opt)) == -1)
  log_warn("setting SO_REUSEADDR on socket");
@@ -809,8 +829,8 @@ open_ports(void)
  error = getaddrinfo("::1", "domain", &hints, &res0);
  if (!error && res0) {
  if ((tcp6sock = socket(res0->ai_family,
-    res0->ai_socktype | SOCK_NONBLOCK,
-    res0->ai_protocol)) != -1) {
+    res0->ai_socktype | SOCK_NONBLOCK | SOCK_CLOEXEC,
+    res0->ai_protocol)) != -1) {
  if (setsockopt(tcp6sock, SOL_SOCKET, SO_REUSEADDR,
     &opt, sizeof(opt)) == -1)
  log_warn("setting SO_REUSEADDR on socket");
@@ -835,14 +855,10 @@ open_ports(void)
     tcp6sock == -1))
  fatalx("could not bind to 127.0.0.1 or ::1 on port 53");

- if (udp4sock != -1)
- main_imsg_compose_frontend_fd(IMSG_UDP4SOCK, 0, udp4sock);
- if (udp6sock != -1)
- main_imsg_compose_frontend_fd(IMSG_UDP6SOCK, 0, udp6sock);
- if (tcp4sock != -1)
- main_imsg_compose_frontend_fd(IMSG_TCP4SOCK, 0, tcp4sock);
- if (tcp6sock != -1)
- main_imsg_compose_frontend_fd(IMSG_TCP6SOCK, 0, tcp6sock);
+ socks->tcp4sock = tcp4sock;
+ socks->tcp6sock = tcp6sock;
+ socks->udp4sock = udp4sock;
+ socks->udp6sock = udp6sock;
 }

 void