Hi,
the privileged part of pflogd is responsible to move bad log files out
of the way, i.e. ones that cannot be parsed. The current code could end
up in an endless loop if all possible files exist. Also, it doesn't
clean up after a failed rename() attempt, leaving a dead file behind.
I have adjusted the "find a new file with random name" into an mkstemp
call, because it's exactly what it's supposed to do. Just look out for
the difference in permissions: old code uses 000, new one 600. I see
no issue in this because it's privileged code anyway, i.e. running as
root.
Tobias
Index: privsep.c
===================================================================
RCS file: /cvs/src/sbin/pflogd/privsep.c,v
retrieving revision 1.17
diff -u -p -r1.17 privsep.c
--- privsep.c 24 Dec 2009 10:06:35 -0000 1.17
+++ privsep.c 30 Mar 2013 22:44:04 -0000
@@ -193,21 +193,19 @@ move_log(const char *name)
for (;;) {
int fd;
- len = snprintf(ren, sizeof(ren), "%s.bad.%08x",
- name, arc4random());
+ len = snprintf(ren, sizeof(ren), "%s.bad.XXXXXXXX", name);
if (len >= sizeof(ren)) {
logmsg(LOG_ERR, "[priv] new name too long");
return (1);
}
/* lock destination */
- fd = open(ren, O_CREAT|O_EXCL, 0);
+ fd = mkstemp(ren);
if (fd >= 0) {
close(fd);
break;
}
- /* if file exists, try another name */
- if (errno != EEXIST && errno != EINTR) {
+ if (errno != EINTR) {
logmsg(LOG_ERR, "[priv] failed to create new name: %s",
strerror(errno));
return (1);
@@ -217,6 +215,7 @@ move_log(const char *name)
if (rename(name, ren)) {
logmsg(LOG_ERR, "[priv] failed to rename %s to %s: %s",
name, ren, strerror(errno));
+ unlink(ren);
return (1);
}
