On Wed, Sep 03, 2014 at 06:46:26PM +0200, Alexander Bluhm wrote:
> 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?
After merging with my previous commit, here is the updated diff.
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 4 Sep 2014 15:49:45 -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);
? usr.sbin/syslogd/flags
? usr.sbin/syslogd/obj
Index: usr.sbin/syslogd/privsep.c
===================================================================
RCS file: /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 4 Sep 2014 16:00:26 -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: /cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.122
diff -u -p -r1.122 syslogd.c
--- usr.sbin/syslogd/syslogd.c 4 Sep 2014 15:19:05 -0000 1.122
+++ usr.sbin/syslogd/syslogd.c 4 Sep 2014 16:00:26 -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 */
@@ -289,12 +288,15 @@ void logto_ctlconn(char *);
int
main(int argc, char *argv[])
{
- int ch, i, fd;
+ int ch, i, fd, nunix = 1;
char *p;
int lockpipe[2] = { -1, -1}, pair[2], nullfd;
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 */
@@ -321,18 +323,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;
@@ -442,8 +444,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;
@@ -453,7 +455,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];
@@ -578,7 +580,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:
@@ -603,7 +605,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) {
unix_read_handler(pfd[PFD_UNIX_0 + i].fd);
}
Index: usr.sbin/syslogd/syslogd.h
===================================================================
RCS file: /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 4 Sep 2014 16:00:26 -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 {
