Close the eventfd if we are non-blocking

The eventfd is used to break out of a poll() before it
times out, used by pcap_breakloop().  If we are non-blocking,
then the eventfd is never needed, so we close it.  (And
open a new eventfd if we switch to blocking).

3 files changed, 231 insertions, 5 deletions

diff --git a/pcap-linux.c b/pcap-linux.c
index 8dbf89bc..42ad9249 100644
--- a/pcap-linux.c
+++ b/pcap-linux.c
@@ -842,7 +842,10 @@ static void	pcap_cleanup_linux( pcap_t *handle )
 		handlep->device = NULL;
 	}
 
-	close(handlep->poll_breakloop_fd);
+	if (handlep->poll_breakloop_fd != -1) {
+		close(handlep->poll_breakloop_fd);
+		handlep->poll_breakloop_fd = -1;
+	}
 	pcap_cleanup_live_common(handle);
 }
 
@@ -941,7 +944,8 @@ static void pcap_breakloop_linux(pcap_t *handle)
 
 	uint64_t value = 1;
 	/* XXX - what if this fails? */
-	(void)write(handlep->poll_breakloop_fd, &value, sizeof(value));
+	if (handlep->poll_breakloop_fd != -1)
+		(void)write(handlep->poll_breakloop_fd, &value, sizeof(value));
 }
 
 /*
@@ -3375,7 +3379,23 @@ pcap_setnonblock_linux(pcap_t *handle, int nonblock)
 			 */
 			handlep->timeout = ~handlep->timeout;
 		}
+		if (handlep->poll_breakloop_fd != -1) {
+			/* Close the eventfd; we do not need it in nonblock mode. */
+			close(handlep->poll_breakloop_fd);
+			handlep->poll_breakloop_fd = -1;
+		}
 	} else {
+		if (handlep->poll_breakloop_fd == -1) {
+			/* If we did not have an eventfd, open one now that we are blocking. */
+			if ( ( handlep->poll_breakloop_fd = eventfd(0, EFD_NONBLOCK) ) == -1 ) {
+				int save_errno = errno;
+				snprintf(handle->errbuf, PCAP_ERRBUF_SIZE,
+						"Could not open eventfd: %s",
+						strerror(errno));
+				errno = save_errno;
+				return -1;
+			}
+		}
 		if (handlep->timeout < 0) {
 			handlep->timeout = ~handlep->timeout;
 		}
@@ -3419,10 +3439,24 @@ static int pcap_wait_for_frames_mmap(pcap_t *handle)
 	struct ifreq ifr;
 	int ret;
 	struct pollfd pollinfo[2];
+	int numpollinfo;
 	pollinfo[0].fd = handle->fd;
 	pollinfo[0].events = POLLIN;
-	pollinfo[1].fd = handlep->poll_breakloop_fd;
-	pollinfo[1].events = POLLIN;
+	if ( handlep->poll_breakloop_fd == -1 ) {
+		numpollinfo = 1;
+		pollinfo[1].revents = 0;
+		/*
+		 * We set pollinfo[1].revents to zero, even though
+		 * numpollinfo = 1 meaning that poll() doesn't see
+		 * pollinfo[1], so that we do not have to add a
+		 * conditional of numpollinfo > 1 below when we
+		 * test pollinfo[1].revents.
+		 */
+	} else {
+		pollinfo[1].fd = handlep->poll_breakloop_fd;
+		pollinfo[1].events = POLLIN;
+		numpollinfo = 2;
+	}
 
 	/*
 	 * Keep polling until we either get some packets to read, see
@@ -3487,7 +3521,7 @@ static int pcap_wait_for_frames_mmap(pcap_t *handle)
 			if (timeout != 0)
 				timeout = 1;
 		}
-		ret = poll(pollinfo, 2, timeout);
+		ret = poll(pollinfo, numpollinfo, timeout);
 		if (ret < 0) {
 			/*
 			 * Error.  If it's not EINTR, report it.
diff --git a/testprogs/Makefile.in b/testprogs/Makefile.in
index 45e1c73e..293ed888 100644
--- a/testprogs/Makefile.in
+++ b/testprogs/Makefile.in
@@ -85,6 +85,7 @@ SRC = @VALGRINDTEST_SRC@ \
 	findalldevstest-perf.c \
 	findalldevstest.c \
 	opentest.c \
+	nonblocktest.c \
 	reactivatetest.c \
 	selpolltest.c \
 	threadsignaltest.c \
@@ -126,6 +127,10 @@ opentest: $(srcdir)/opentest.c ../libpcap.a
 	$(CC) $(FULL_CFLAGS) -I. -L. -o opentest $(srcdir)/opentest.c \
 	    ../libpcap.a $(LIBS)
 
+nonblocktest: $(srcdir)/nonblocktest.c ../libpcap.a
+	$(CC) $(FULL_CFLAGS) -I. -L. -o nonblocktest $(srcdir)/nonblocktest.c \
+	    ../libpcap.a $(LIBS)
+
 reactivatetest: $(srcdir)/reactivatetest.c ../libpcap.a
 	$(CC) $(FULL_CFLAGS) -I. -L. -o reactivatetest \
 	    $(srcdir)/reactivatetest.c ../libpcap.a $(LIBS)
diff --git a/testprogs/nonblocktest.c b/testprogs/nonblocktest.c
new file mode 100644
index 00000000..72700a3b
--- /dev/null
+++ b/testprogs/nonblocktest.c
@@ -0,0 +1,187 @@
+/*
+ * Copyright (c) 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997, 2000
+ *	The Regents of the University of California.  All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that: (1) source code distributions
+ * retain the above copyright notice and this paragraph in its entirety, (2)
+ * distributions including binary code include the above copyright notice and
+ * this paragraph in its entirety in the documentation or other materials
+ * provided with the distribution, and (3) all advertising materials mentioning
+ * features or use of this software display the following acknowledgement:
+ * ``This product includes software developed by the University of California,
+ * Lawrence Berkeley Laboratory and its contributors.'' Neither the name of
+ * the University nor the names of its contributors may be used to endorse
+ * or promote products derived from this software without specific prior
+ * written permission.
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR IMPLIED
+ * WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE.
+ */
+
+#include "varattrs.h"
+
+/*
+ * Tests for pcap_set_nonblock / pcap_get_nonblock:
+ * - idempotency
+ * - set/get are symmetric
+ * - get returns the same before/after activate
+ * - pcap_breakloop works after setting nonblock on and then off
+ *
+ * Really this is meant to
+ * be run manually under strace, to check for extra
+ * calls to eventfd or close.
+ */
+#include <pcap.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdarg.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+static pcap_t *pd;
+static char *program_name = "nonblocktest";
+/* Forwards */
+static void PCAP_NORETURN usage(void);
+static void PCAP_NORETURN error(const char *, ...) PCAP_PRINTFLIKE(1, 2);
+static void warning(const char *, ...) PCAP_PRINTFLIKE(1, 2);
+
+/* VARARGS */
+static void
+error(const char *fmt, ...)
+{
+	va_list ap;
+
+	(void)fprintf(stderr, "%s: ", program_name);
+	va_start(ap, fmt);
+	(void)vfprintf(stderr, fmt, ap);
+	va_end(ap);
+	if (*fmt) {
+		fmt += strlen(fmt);
+		if (fmt[-1] != '\n')
+			(void)fputc('\n', stderr);
+	}
+	exit(1);
+	/* NOTREACHED */
+}
+
+/* VARARGS */
+static void
+warning(const char *fmt, ...)
+{
+	va_list ap;
+
+	(void)fprintf(stderr, "%s: WARNING: ", program_name);
+	va_start(ap, fmt);
+	(void)vfprintf(stderr, fmt, ap);
+	va_end(ap);
+	if (*fmt) {
+		fmt += strlen(fmt);
+		if (fmt[-1] != '\n')
+			(void)fputc('\n', stderr);
+	}
+}
+
+static void
+usage(void)
+{
+	(void)fprintf(stderr, "Usage: %s [ -i interface ]\n",
+	    program_name);
+	exit(1);
+}
+
+static void
+breakme(u_char *user _U_, const struct pcap_pkthdr *h _U_, const u_char *sp _U_)
+{
+	warning("using pcap_breakloop()");
+	pcap_breakloop(pd);
+}
+
+int
+main(int argc, char **argv)
+{
+	int status, op, i, ret;
+	char *device;
+	pcap_if_t *devlist;
+	char ebuf[PCAP_ERRBUF_SIZE];
+
+	device = NULL;
+	while ((op = getopt(argc, argv, "i:sptnq")) != -1) {
+		switch (op) {
+
+		case 'i':
+			device = optarg;
+			break;
+
+		default:
+			usage();
+			/* NOTREACHED */
+		}
+	}
+	if (device == NULL) {
+		if (pcap_findalldevs(&devlist, ebuf) == -1)
+			error("%s", ebuf);
+		if (devlist == NULL)
+			error("no interfaces available for capture");
+		device = strdup(devlist->name);
+		warning("listening on %s", device);
+		pcap_freealldevs(devlist);
+	}
+	*ebuf = '\0';
+	pd = pcap_create(device, ebuf);
+	if (pd == NULL)
+		error("%s", ebuf);
+	else if (*ebuf)
+		warning("%s", ebuf);
+	/* set nonblock before activate */
+	if (pcap_setnonblock(pd, 1, ebuf) < 0)
+		error("pcap_setnonblock failed: %s", ebuf);
+	/* getnonblock just returns "not activated yet" */
+	ret = pcap_getnonblock(pd, ebuf);
+	if (ret != PCAP_ERROR_NOT_ACTIVATED)
+		error("pcap_getnonblock unexpectedly succeeded");
+	if ((status = pcap_activate(pd)) < 0)
+		error("pcap_activate failed");
+	ret = pcap_getnonblock(pd, ebuf);
+	if (ret != 1)
+		error( "pcap_getnonblock did not return nonblocking" );
+
+	/* Set nonblock multiple times, ensure with strace that it's a noop */
+	for (i=0; i<10; i++) {
+		if (pcap_setnonblock(pd, 1, ebuf) < 0)
+			error("pcap_setnonblock failed: %s", ebuf);
+		ret = pcap_getnonblock(pd, ebuf);
+		if (ret != 1)
+			error( "pcap_getnonblock did not return nonblocking" );
+	}
+	/* Set block multiple times, ensure with strace that it's a noop */
+	for (i=0; i<10; i++) {
+		if (pcap_setnonblock(pd, 0, ebuf) < 0)
+			error("pcap_setnonblock failed: %s", ebuf);
+		ret = pcap_getnonblock(pd, ebuf);
+		if (ret != 0)
+			error( "pcap_getnonblock did not return blocking" );
+	}
+
+	/* Now pcap_loop forever, with a callback that
+	 * uses pcap_breakloop to get out of forever */
+	pcap_loop(pd, -1, breakme, NULL);
+
+        /* Now test that pcap_setnonblock fails if we can't open the
+         * eventfd. */
+        if (pcap_setnonblock(pd, 1, ebuf) < 0)
+                error("pcap_setnonblock failed: %s", ebuf);
+        while (1) {
+                ret = open("/dev/null", O_RDONLY);
+                if (ret < 0)
+                        break;
+        }
+        ret = pcap_setnonblock(pd, 0, ebuf);
+        if (ret == 0)
+                error("pcap_setnonblock succeeded even though file table is full");
+        else
+                warning("pcap_setnonblock failed as expected: %s", ebuf);
+}