Make sure no read routine process more than INT_MAX packets.

Some read routines don't read a single bufferful of packets and process
just those packets; if packets continue to be made available, they could
conceivably process an arbitrary number of packets.

That would mean that the packet count overflows; either that makes it
look like a negative number, making it look as if an error occurred, or
makes it look like a too-small positive number.

This can't be fixed by making the count 64-bit, as it ultimately gets
returned by pcap_dispatch(), which is defined to return an int.

Instead, if the maximum packet count argument to those routines is a
value that means "no maximum", we set the maximum to INT_MAX.  Those
routines are *not* defined to loop forever, so this isn't an issue.

This should fix issue #1087.

1 files changed, 37 insertions, 11 deletions

diff --git a/pcap-linux.c b/pcap-linux.c
index e931f84f..bd8f2b24 100644
--- a/pcap-linux.c
+++ b/pcap-linux.c
@@ -4064,9 +4064,22 @@ pcap_read_linux_mmap_v2(pcap_t *handle, int max_packets, pcap_handler callback,
 		}
 	}
 
-	/* non-positive values of max_packets are used to require all
-	 * packets currently available in the ring */
-	while ((pkts < max_packets) || PACKET_COUNT_IS_UNLIMITED(max_packets)) {
+	/*
+	 * This can conceivably process more than INT_MAX packets,
+	 * which would overflow the packet count, causing it either
+	 * to look like a negative number, and thus cause us to
+	 * return a value that looks like an error, or overflow
+	 * back into positive territory, and thus cause us to
+	 * return a too-low count.
+	 *
+	 * Therefore, if the packet count is unlimited, we clip
+	 * it at INT_MAX; this routine is not expected to
+	 * process packets indefinitely, so that's not an issue.
+	 */
+	if (PACKET_COUNT_IS_UNLIMITED(max_packets))
+		max_packets = INT_MAX;
+
+	while (pkts < max_packets) {
 		/*
 		 * Get the current ring buffer frame, and break if
 		 * it's still owned by the kernel.
@@ -4159,9 +4172,22 @@ again:
 		return pkts;
 	}
 
-	/* non-positive values of max_packets are used to require all
-	 * packets currently available in the ring */
-	while ((pkts < max_packets) || PACKET_COUNT_IS_UNLIMITED(max_packets)) {
+	/*
+	 * This can conceivably process more than INT_MAX packets,
+	 * which would overflow the packet count, causing it either
+	 * to look like a negative number, and thus cause us to
+	 * return a value that looks like an error, or overflow
+	 * back into positive territory, and thus cause us to
+	 * return a too-low count.
+	 *
+	 * Therefore, if the packet count is unlimited, we clip
+	 * it at INT_MAX; this routine is not expected to
+	 * process packets indefinitely, so that's not an issue.
+	 */
+	if (PACKET_COUNT_IS_UNLIMITED(max_packets))
+		max_packets = INT_MAX;
+
+	while (pkts < max_packets) {
 		int packets_to_read;
 
 		if (handlep->current_packet == NULL) {
@@ -4174,12 +4200,12 @@ again:
 		}
 		packets_to_read = handlep->packets_left;
 
-		if (!PACKET_COUNT_IS_UNLIMITED(max_packets) &&
-		    packets_to_read > (max_packets - pkts)) {
+		if (packets_to_read > (max_packets - pkts)) {
 			/*
-			 * We've been given a maximum number of packets
-			 * to process, and there are more packets in
-			 * this buffer than that.  Only process enough
+			 * There are more packets in the buffer than
+			 * the number of packets we have left to
+			 * process to get up to the maximum number
+			 * of packets to process.  Only process enough
 			 * of them to get us up to that maximum.
 			 */
 			packets_to_read = max_packets - pkts;