On Fri, Aug 29, 2014 at 11:25:52PM +0200, Alexander Bluhm wrote:
> So I will write more tests before committing this.
My regression tests found a bug in syslogd. When adding the maximum
number of paths with the -a option, the arrays for unix domain
socket paths and the poll file descriptors overflow by one.
- increase pfd array by one
- operate on size of arrays not length
- null check for path not necessary when doing fd -1 check
- rename variables consistently ...unix
- make number of unix fds a local variable
ok?
bluhm
Index: usr.sbin/syslogd/privsep.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/privsep.c,v
retrieving revision 1.43
diff -u -p -r1.43 privsep.c
--- usr.sbin/syslogd/privsep.c 25 Aug 2014 20:19:14 -0000 1.43
+++ usr.sbin/syslogd/privsep.c 3 Sep 2014 16:08:03 -0000
@@ -172,7 +172,7 @@ priv_init(char *conf, int numeric, int l
close(socks[1]);
/* Close descriptors that only the unpriv child needs */
- for (i = 0; i < nfunix; i++)
+ for (i = 0; i < MAXUNIX + 1; i++)
if (pfd[PFD_UNIX_0 + i].fd != -1)
close(pfd[PFD_UNIX_0 + i].fd);
if (pfd[PFD_INET].fd != -1)
@@ -369,9 +369,9 @@ priv_init(char *conf, int numeric, int l
close(socks[0]);
/* Unlink any domain sockets that have been opened */
- for (i = 0; i < nfunix; i++)
- if (funixn[i] != NULL && pfd[PFD_UNIX_0 + i].fd != -1)
- (void)unlink(funixn[i]);
+ for (i = 0; i < MAXUNIX; i++)
+ if (pfd[PFD_UNIX_0 + i].fd != -1)
+ (void)unlink(path_unix[i]);
if (ctlsock_path != NULL && pfd[PFD_CTLSOCK].fd != -1)
(void)unlink(ctlsock_path);
Index: usr.sbin/syslogd/syslogd.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.121
diff -u -p -r1.121 syslogd.c
--- usr.sbin/syslogd/syslogd.c 31 Aug 2014 22:11:43 -0000 1.121
+++ usr.sbin/syslogd/syslogd.c 3 Sep 2014 16:40:54 -0000
@@ -183,8 +183,7 @@ char *TypeNames[9] = {
struct filed *Files;
struct filed consfile;
-int nfunix = 1; /* Number of Unix domain sockets requested */
-char *funixn[MAXFUNIX] = { _PATH_LOG }; /* Paths to Unix domain sockets */
+char *path_unix[MAXUNIX] = { _PATH_LOG }; /* Paths to Unix domain sockets */
int Debug; /* debug flag */
int Startup = 1; /* startup flag */
char LocalHostName[MAXHOSTNAMELEN]; /* our hostname */
@@ -282,7 +281,7 @@ void logto_ctlconn(char *);
int
main(int argc, char *argv[])
{
- int ch, i, linesize, fd;
+ int ch, i, linesize, fd, nunix = 1;
struct sockaddr_un fromunix;
struct sockaddr_storage from;
socklen_t len;
@@ -292,6 +291,9 @@ main(int argc, char *argv[])
struct addrinfo hints, *res, *res0;
FILE *fp;
+ for (i = 0; i < N_PFD; i++)
+ pfd[i].fd = -1;
+
while ((ch = getopt(argc, argv, "46dhnuf:m:p:a:s:")) != -1)
switch (ch) {
case '4': /* disable IPv6 */
@@ -318,18 +320,18 @@ main(int argc, char *argv[])
NoDNS = 1;
break;
case 'p': /* path */
- funixn[0] = optarg;
+ path_unix[0] = optarg;
break;
case 'u': /* allow udp input port */
SecureMode = 0;
break;
case 'a':
- if (nfunix >= MAXFUNIX)
+ if (nunix >= MAXUNIX)
fprintf(stderr, "syslogd: "
"out of descriptors, ignoring %s\n",
optarg);
else
- funixn[nfunix++] = optarg;
+ path_unix[nunix++] = optarg;
break;
case 's':
ctlsock_path = optarg;
@@ -439,8 +441,8 @@ main(int argc, char *argv[])
#ifndef SUN_LEN
#define SUN_LEN(unp) (strlen((unp)->sun_path) + 2)
#endif
- for (i = 0; i < nfunix; i++) {
- if ((fd = unix_socket(funixn[i], SOCK_DGRAM, 0666)) == -1) {
+ for (i = 0; i < nunix; i++) {
+ if ((fd = unix_socket(path_unix[i], SOCK_DGRAM, 0666)) == -1) {
if (i == 0 && !Debug)
die(0);
continue;
@@ -450,7 +452,7 @@ main(int argc, char *argv[])
pfd[PFD_UNIX_0 + i].events = POLLIN;
}
- nfunix++;
+ nunix++;
if (socketpair(AF_UNIX, SOCK_DGRAM, PF_UNSPEC, pair) == -1)
die(0);
fd = pair[0];
@@ -575,7 +577,7 @@ main(int argc, char *argv[])
dprintf("syslogd: restarted\n");
}
- switch (poll(pfd, PFD_UNIX_0 + nfunix, -1)) {
+ switch (poll(pfd, PFD_UNIX_0 + nunix, -1)) {
case 0:
continue;
case -1:
@@ -628,7 +630,7 @@ main(int argc, char *argv[])
if ((pfd[PFD_CTLCONN].revents & POLLOUT) != 0)
ctlconn_write_handler();
- for (i = 0; i < nfunix; i++) {
+ for (i = 0; i < nunix; i++) {
if ((pfd[PFD_UNIX_0 + i].revents & POLLIN) != 0) {
ssize_t rlen;
Index: usr.sbin/syslogd/syslogd.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.h,v
retrieving revision 1.12
diff -u -p -r1.12 syslogd.h
--- usr.sbin/syslogd/syslogd.h 25 Aug 2014 18:19:18 -0000 1.12
+++ usr.sbin/syslogd/syslogd.h 3 Sep 2014 16:08:03 -0000
@@ -39,9 +39,8 @@ void send_fd(int, int);
int receive_fd(int);
/* The list of domain sockets */
-#define MAXFUNIX 21
-extern int nfunix;
-extern char *funixn[MAXFUNIX];
+#define MAXUNIX 21
+extern char *path_unix[MAXUNIX];
extern char *ctlsock_path;
#define dprintf(_f...) do { if (Debug) printf(_f); } while (0)
@@ -55,7 +54,7 @@ extern int Startup;
#define PFD_CTLCONN 3 /* Offset of control connection entry */
#define PFD_INET6 4 /* Offset of inet6 socket entry */
#define PFD_UNIX_0 5 /* Start of Unix socket entries */
-#define N_PFD (PFD_UNIX_0 + MAXFUNIX) /* # of pollfd entries */
+#define N_PFD (PFD_UNIX_0 + MAXUNIX + 1) /* # of pollfd entries */
extern struct pollfd pfd[N_PFD];
struct ringbuf {
