Hi tech@,
sthen@ pointed out to me that dhcpd doesn't properly terminate the pf table
handler.
I reproduced the issue both on 6.1 and -current.
Minimal config I used on my server:
/etc/dhcpd.conf
subnet 45.63.9.186 netmask 255.255.255.224 {
range 45.63.9.186 45.63.9.186;
}
enabled dhcpd and used the -A flag to trigger the pf handler being spawned:
rcctl enable dhcpd
rcctl set dhcpd flags -A test
and performed the test
# rcctl start dhcpd; rcctl stop dhcpd; pgrep -lf dhcpd
dhcpd(ok)
dhcpd(ok)
96584 dhcpd: pf table handler
which demonstrates that indeed the pf table handler is left running.
Both the main program and the child spin in a for(;;) loop, there is no
code anticipating the need to signal a child to quit, there is also
no attempt to detect the parent dying by the child.
The child also uses a notreached exit(3) after the loop.
When the parent is not around the unattended child proceeds to puke all over
the logfiles:
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe closed
Jul 11 21:04:06 tintagel dhcpd[62506]: pf pipe error: Broken pipe
brynet@ noticed the exit(3) instead of _exit(2) and he also pointed
out that the pf table handler is a bit optimistic on warnings instead of bailing
out with an error and terminating.
Examples:
55 if ((fd = open(_PATH_DEV_PF, O_RDWR|O_NOFOLLOW, 0660)) == -1)
56 log_warn("can't open pf device");
this child is useless if it can't pf
57 if (chroot(_PATH_VAREMPTY) == -1)
58 log_warn("chroot %s", _PATH_VAREMPTY);
59 if (chdir("/") == -1)
60 log_warn("chdir(\"/\")");
61 if (setgroups(1, &pw->pw_gid) ||
62 setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) ||
63 setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid))
64 log_warn("can't drop privileges");
not able to chroot and drop privileges seems like good enough reason to die.
76 if (nfds > 0 && (pfd[0].revents & POLLHUP))
77 log_warnx("pf pipe closed");
we won't get any work to do as the pipe is closed, should we die?
Now, I did try a quick diff changing log_warn on those to fatal()
https://junk.tintagel.pl/dhcpd-pf-handler.diff
This of course results in the child dying when the parent is gone
but we believe it's not commitable as the following needs to be
decided:
- if the handler fails (ie. not able to open pf) should it signal
the main process to die or just stop working (like it did now)?
- fatal() can't be used as it uses exit(3) instead of _exit(2)
- anything else that we might have missed
looking for pointers/suggestions on how to move forward with this.
regards,
Adam
