Skip to content

Commit

Permalink
- Fix that the server does not chown the pidfile.
Browse files Browse the repository at this point in the history
  • Loading branch information
wcawijngaards committed Mar 27, 2024
1 parent 192f1b0 commit 6f82b5b
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 19 deletions.
9 changes: 9 additions & 0 deletions contrib/rc_d_unbound
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,13 @@ pidfile=${unbound_pidfile:-"/usr/local/etc/unbound/unbound.pid"}
command_args=${unbound_flags:-"-c /usr/local/etc/unbound/unbound.conf"}
extra_commands="reload"

if test "$1" = "stop" ; then
run_rc_command "$1"
ret=$?
if test $ret -eq 0; then
rm -f "$pidfile"
fi
exit $ret
fi

run_rc_command "$1"
1 change: 1 addition & 0 deletions contrib/unbound.init
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ stop() {
retval=$?
echo
[ $retval -eq 0 ] && rm -f $lockfile
[ $retval -eq 0 ] && rm -f $pidfile
if egrep -q '^/[^[:space:]]+[[:space:]]+'${rootdir}'/dev/log' /proc/mounts; then
umount ${rootdir}/dev/log >/dev/null 2>&1
fi;
Expand Down
1 change: 1 addition & 0 deletions contrib/unbound.init_fedora
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ stop() {
killproc -p $pidfile unbound
retval=$?
[ $retval -eq 0 ] && rm -f $lockfile
[ $retval -eq 0 ] && rm -f $pidfile
for mountfile in /dev/log /dev/urandom /etc/localtime /etc/resolv.conf /var/run/unbound
do
if egrep -q '^/[^[:space:]]+[[:space:]]+'${rootdir}''${mountfile}'' /proc/mounts; then
Expand Down
1 change: 1 addition & 0 deletions contrib/unbound.init_yocto
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ stop() {
retval=$?
echo
[ $retval -eq 0 ] && rm -f $lockfile
[ $retval -eq 0 ] && rm -f $pidfile
if egrep -q '^/[^[:space:]]+[[:space:]]+'${rootdir}'/dev/log' /proc/mounts; then
umount ${rootdir}/dev/log >/dev/null 2>&1
fi;
Expand Down
33 changes: 14 additions & 19 deletions daemon/unbound.c
Original file line number Diff line number Diff line change
Expand Up @@ -366,9 +366,8 @@ readpid (const char* file)
/** write pid to file.
* @param pidfile: file name of pid file.
* @param pid: pid to write to file.
* @return false on failure
*/
static int
static void
writepid (const char* pidfile, pid_t pid)
{
int fd;
Expand All @@ -383,7 +382,7 @@ writepid (const char* pidfile, pid_t pid)
, 0644)) == -1) {
log_err("cannot open pidfile %s: %s",
pidfile, strerror(errno));
return 0;
return;
}
while(count < strlen(pidbuf)) {
ssize_t r = write(fd, pidbuf+count, strlen(pidbuf)-count);
Expand All @@ -393,17 +392,16 @@ writepid (const char* pidfile, pid_t pid)
log_err("cannot write to pidfile %s: %s",
pidfile, strerror(errno));
close(fd);
return 0;
return;
} else if(r == 0) {
log_err("cannot write any bytes to pidfile %s: "
"write returns 0 bytes written", pidfile);
close(fd);
return 0;
return;
}
count += r;
}
close(fd);
return 1;
}

/**
Expand Down Expand Up @@ -545,7 +543,15 @@ perform_setup(struct daemon* daemon, struct config_file* cfg, int debug_mode,
cfg, 1);
if(!daemon->pidfile)
fatal_exit("pidfile alloc: out of memory");
checkoldpid(daemon->pidfile, pidinchroot);
/* Check old pid if there is no username configured.
* With a username, the assumption is that the privilege
* drop makes a pidfile not removed when the server stopped
* last time. The server does not chown the pidfile for it,
* because that creates privilege escape problems, with the
* pidfile writable by unprivileged users, but used by
* privileged users. */
if(cfg->username && cfg->username[0])
checkoldpid(daemon->pidfile, pidinchroot);
}
#endif

Expand All @@ -557,18 +563,7 @@ perform_setup(struct daemon* daemon, struct config_file* cfg, int debug_mode,
/* write new pidfile (while still root, so can be outside chroot) */
#ifdef HAVE_KILL
if(cfg->pidfile && cfg->pidfile[0] && need_pidfile) {
if(writepid(daemon->pidfile, getpid())) {
if(cfg->username && cfg->username[0] && cfg_uid != (uid_t)-1 &&
pidinchroot) {
# ifdef HAVE_CHOWN
if(chown(daemon->pidfile, cfg_uid, cfg_gid) == -1) {
verbose(VERB_QUERY, "cannot chown %u.%u %s: %s",
(unsigned)cfg_uid, (unsigned)cfg_gid,
daemon->pidfile, strerror(errno));
}
# endif /* HAVE_CHOWN */
}
}
writepid(daemon->pidfile, getpid());
}
#else
(void)daemon;
Expand Down
1 change: 1 addition & 0 deletions doc/Changelog
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- Fix to add unit test for lruhash space that exercises the routines.
- Fix that when the server truncates the pidfile, it does not follow
symbolic links.
- Fix that the server does not chown the pidfile.

25 March 2024: Yorgos
- Merge #831 from Pierre4012: Improve Windows NSIS installer
Expand Down

0 comments on commit 6f82b5b

Please sign in to comment.