linux: fix code path to properly return warnings.

Don't overwrite PCAP_WARNING_ codes with 0 (success).

While we're at it, clean up some leftovers from when we supported older
Linux kernels and failures to, for example, set up PF_PACKET sockets or
to set up a memory-mapped buffer meant "fall back on older mechanisms"
rather than "fail".

1 files changed, 72 insertions, 64 deletions

diff --git a/pcap-linux.c b/pcap-linux.c
index b72fdbc1..743e17a3 100644
--- a/pcap-linux.c
+++ b/pcap-linux.c
@@ -224,7 +224,7 @@ static int is_wifi(const char *);
 static void map_arphrd_to_dlt(pcap_t *, int, const char *, int);
 static int pcap_activate_linux(pcap_t *);
 static int setup_socket(pcap_t *, int);
-static int setup_mmapped(pcap_t *, int *);
+static int setup_mmapped(pcap_t *);
 static int pcap_can_set_rfmon_linux(pcap_t *);
 static int pcap_inject_linux(pcap_t *, const void *, int);
 static int pcap_stats_linux(pcap_t *, struct pcap_stat *);
@@ -245,7 +245,7 @@ union thdr {
 #define RING_GET_CURRENT_FRAME(h) RING_GET_FRAME_AT(h, h->offset)
 
 static void destroy_ring(pcap_t *handle);
-static int create_ring(pcap_t *handle, int *status);
+static int create_ring(pcap_t *handle);
 static int prepare_tpacket_socket(pcap_t *handle);
 static int pcap_read_linux_mmap_v2(pcap_t *, int, pcap_handler , u_char *);
 #ifdef HAVE_TPACKET3
@@ -996,13 +996,17 @@ pcap_activate_linux(pcap_t *handle)
 	const char	*device;
 	int		is_any_device;
 	struct ifreq	ifr;
-	int		status = 0;
-	int		status2 = 0;
+	int		status;
 	int		ret;
 
 	device = handle->opt.device;
 
 	/*
+	 * Start out assuming no warnings.
+	 */
+	status = 0;
+
+	/*
 	 * Make sure the name we were handed will fit into the ioctls we
 	 * might perform on the device; if not, return a "No such device"
 	 * indication, as the Linux kernel shouldn't support creating
@@ -1084,22 +1088,38 @@ pcap_activate_linux(pcap_t *handle)
 		status = ret;
 		goto fail;
 	}
+	if (ret > 0) {
+		/*
+		 * We got a warning; return that, as handle->errbuf
+		 * might have been overwritten by this warning.
+		 */
+		status = ret;
+	}
+
 	/*
-	 * Success.
+	 * Success (possibly with a warning).
 	 * Try to set up memory-mapped access.
 	 */
-	ret = setup_mmapped(handle, &status);
-	if (ret == -1) {
+	ret = setup_mmapped(handle);
+	if (ret < 0) {
 		/*
 		 * We failed to set up to use it, or the
 		 * kernel supports it, but we failed to
-		 * enable it.  status has been set to the
+		 * enable it.  The return value is the
 		 * error status to return and, if it's
 		 * PCAP_ERROR, handle->errbuf contains
 		 * the error message.
 		 */
+		status = ret;
 		goto fail;
 	}
+	if (ret > 0) {
+		/*
+		 * We got a warning; return that, as handle->errbuf
+		 * might have been overwritten by this warning.
+		 */
+		status = ret;
+	}
 
 	/*
 	 * We succeeded.  status has been set to the status to return,
@@ -1109,9 +1129,9 @@ pcap_activate_linux(pcap_t *handle)
 	 * Now that we have activated the mmap ring, we can
 	 * set the correct protocol.
 	 */
-	if ((status2 = iface_bind(handle->fd, handlep->ifindex,
+	if ((ret = iface_bind(handle->fd, handlep->ifindex,
 	    handle->errbuf, pcap_protocol(handle))) != 0) {
-		status = status2;
+		status = ret;
 		goto fail;
 	}
 
@@ -2283,7 +2303,8 @@ set_dlt_list_cooked(pcap_t *handle)
 
 /*
  * Try to set up a PF_PACKET socket.
- * Returns 0 on success and a PCAP_ERROR_ value on failure.
+ * Returns 0 or a PCAP_WARNING_ value on success and a PCAP_ERROR_ value
+ * on failure.
  */
 static int
 setup_socket(pcap_t *handle, int is_any_device)
@@ -2636,17 +2657,18 @@ setup_socket(pcap_t *handle, int is_any_device)
 /*
  * Attempt to setup memory-mapped access.
  *
- * On success, returns 1, and sets *status to 0 if there are no warnings
- * or to a PCAP_WARNING_ code if there is a warning.
+ * On success, returns 0 if there are no warnings or a PCAP_WARNING_ code
+ * if there is a warning.
  *
- * On error, returns -1, and sets *status to the appropriate error code;
- * if that is PCAP_ERROR, sets handle->errbuf to the appropriate message.
+ * On error, returns the appropriate error code; if that is PCAP_ERROR,
+ * sets handle->errbuf to the appropriate message.
  */
 static int
-setup_mmapped(pcap_t *handle, int *status)
+setup_mmapped(pcap_t *handle)
 {
 	struct pcap_linux *handlep = handle->priv;
-	int ret, flags = MAP_ANONYMOUS | MAP_PRIVATE;
+	int flags = MAP_ANONYMOUS | MAP_PRIVATE;
+	int status;
 
 	/*
 	 * Attempt to allocate a buffer to hold the contents of one
@@ -2659,34 +2681,32 @@ setup_mmapped(pcap_t *handle, int *status)
 	if (handlep->oneshot_buffer == MAP_FAILED) {
 		pcap_fmt_errmsg_for_errno(handle->errbuf, PCAP_ERRBUF_SIZE,
 		    errno, "can't allocate oneshot buffer");
-		*status = PCAP_ERROR;
-		return -1;
+		return PCAP_ERROR;
 	}
 
 	if (handle->opt.buffer_size == 0) {
 		/* by default request 2M for the ring buffer */
 		handle->opt.buffer_size = 2*1024*1024;
 	}
-	ret = prepare_tpacket_socket(handle);
-	if (ret == -1) {
+	status = prepare_tpacket_socket(handle);
+	if (status == -1) {
 		munmap(handlep->oneshot_buffer, handle->snapshot);
 		handlep->oneshot_buffer = NULL;
-		*status = PCAP_ERROR;
-		return ret;
+		return PCAP_ERROR;
 	}
-	ret = create_ring(handle, status);
-	if (ret == -1) {
+	status = create_ring(handle);
+	if (status < 0) {
 		/*
 		 * Error attempting to enable memory-mapped capture;
-		 * fail.  create_ring() has set *status.
+		 * fail.  The return value is the status to return.
 		 */
 		munmap(handlep->oneshot_buffer, handle->snapshot);
 		handlep->oneshot_buffer = NULL;
-		return -1;
+		return status;
 	}
 
 	/*
-	 * Success.  *status has been set either to 0 if there are no
+	 * Success.  status has been set either to 0 if there are no
 	 * warnings or to a PCAP_WARNING_ value if there is a warning.
 	 *
 	 * handle->offset is used to get the current position into the rx ring.
@@ -2698,7 +2718,7 @@ setup_mmapped(pcap_t *handle, int *status)
 	 */
 	set_poll_timeout(handlep);
 
-	return 1;
+	return status;
 }
 
 /*
@@ -2847,14 +2867,14 @@ prepare_tpacket_socket(pcap_t *handle)
 /*
  * Attempt to set up memory-mapped access.
  *
- * On success, returns 1, and sets *status to 0 if there are no warnings
- * or to a PCAP_WARNING_ code if there is a warning.
+ * On success, returns 0 if there are no warnings or to a PCAP_WARNING_ code
+ * if there is a warning.
  *
- * On error, returns -1, and sets *status to the appropriate error code;
- * if that is PCAP_ERROR, sets handle->errbuf to the appropriate message.
+ * On error, returns the appropriate error code; if that is PCAP_ERROR,
+ * sets handle->errbuf to the appropriate message.
  */
 static int
-create_ring(pcap_t *handle, int *status)
+create_ring(pcap_t *handle)
 {
 	struct pcap_linux *handlep = handle->priv;
 	unsigned i, j, frames_per_block;
@@ -2872,11 +2892,12 @@ create_ring(pcap_t *handle, int *status)
 	socklen_t len;
 	unsigned int sk_type, tp_reserve, maclen, tp_hdrlen, netoff, macoff;
 	unsigned int frame_size;
+	int status;
 
 	/*
-	 * Start out assuming no warnings or errors.
+	 * Start out assuming no warnings.
 	 */
-	*status = 0;
+	status = 0;
 
 	/*
 	 * Reserve space for VLAN tag reconstruction.
@@ -2911,8 +2932,7 @@ create_ring(pcap_t *handle, int *status)
 		pcap_fmt_errmsg_for_errno(handle->errbuf,
 		    PCAP_ERRBUF_SIZE, errno,
 		    "setsockopt (PACKET_RESERVE)");
-		*status = PCAP_ERROR;
-		return -1;
+		return PCAP_ERROR;
 	}
 
 	switch (handlep->tp_version) {
@@ -2957,15 +2977,11 @@ create_ring(pcap_t *handle, int *status)
 
 			mtu = iface_get_mtu(handle->fd, handle->opt.device,
 			    handle->errbuf);
-			if (mtu == -1) {
-				*status = PCAP_ERROR;
-				return -1;
-			}
+			if (mtu == -1)
+				return PCAP_ERROR;
 			offload = iface_get_offload(handle);
-			if (offload == -1) {
-				*status = PCAP_ERROR;
-				return -1;
-			}
+			if (offload == -1)
+				return PCAP_ERROR;
 			if (offload)
 				max_frame_len = MAX(mtu, 65535);
 			else
@@ -2984,8 +3000,7 @@ create_ring(pcap_t *handle, int *status)
 		    &len) < 0) {
 			pcap_fmt_errmsg_for_errno(handle->errbuf,
 			    PCAP_ERRBUF_SIZE, errno, "getsockopt (SO_TYPE)");
-			*status = PCAP_ERROR;
-			return -1;
+			return PCAP_ERROR;
 		}
 		maclen = (sk_type == SOCK_DGRAM) ? 0 : MAX_LINKHEADER_SIZE;
 			/* XXX: in the kernel maclen is calculated from
@@ -3050,8 +3065,7 @@ create_ring(pcap_t *handle, int *status)
 		snprintf(handle->errbuf, PCAP_ERRBUF_SIZE,
 		    "Internal error: unknown TPACKET_ value %u",
 		    handlep->tp_version);
-		*status = PCAP_ERROR;
-		return -1;
+		return PCAP_ERROR;
 	}
 
 	/* compute the minimum block size that will handle this frame.
@@ -3117,10 +3131,9 @@ create_ring(pcap_t *handle, int *status)
 				 * and, if they can't, shouldn't
 				 * try requesting hardware time stamps.
 				 */
-				*status = PCAP_ERROR_PERM_DENIED;
 				snprintf(handle->errbuf, PCAP_ERRBUF_SIZE,
 				    "Attempt to set hardware timestamp failed - CAP_NET_ADMIN may be required");
-				return -1;
+				return PCAP_ERROR_PERM_DENIED;
 
 			case EOPNOTSUPP:
 			case ERANGE:
@@ -3138,15 +3151,14 @@ create_ring(pcap_t *handle, int *status)
 				 * We'll just fall back on the standard
 				 * host time stamps.
 				 */
-				*status = PCAP_WARNING_TSTAMP_TYPE_NOTSUP;
+				status = PCAP_WARNING_TSTAMP_TYPE_NOTSUP;
 				break;
 
 			default:
 				pcap_fmt_errmsg_for_errno(handle->errbuf,
 				    PCAP_ERRBUF_SIZE, errno,
 				    "SIOCSHWTSTAMP failed");
-				*status = PCAP_ERROR;
-				return -1;
+				return PCAP_ERROR;
 			}
 		} else {
 			/*
@@ -3173,8 +3185,7 @@ create_ring(pcap_t *handle, int *status)
 				pcap_fmt_errmsg_for_errno(handle->errbuf,
 				    PCAP_ERRBUF_SIZE, errno,
 				    "can't set PACKET_TIMESTAMP");
-				*status = PCAP_ERROR;
-				return -1;
+				return PCAP_ERROR;
 			}
 		}
 	}
@@ -3235,8 +3246,7 @@ retry:
 		}
 		pcap_fmt_errmsg_for_errno(handle->errbuf, PCAP_ERRBUF_SIZE,
 		    errno, "can't create rx ring on packet socket");
-		*status = PCAP_ERROR;
-		return -1;
+		return PCAP_ERROR;
 	}
 
 	/* memory map the rx ring */
@@ -3251,8 +3261,7 @@ retry:
 
 		/* clear the allocated ring on error*/
 		destroy_ring(handle);
-		*status = PCAP_ERROR;
-		return -1;
+		return PCAP_ERROR;
 	}
 
 	/* allocate a ring for each frame header pointer*/
@@ -3263,8 +3272,7 @@ retry:
 		    errno, "can't allocate ring of frame headers");
 
 		destroy_ring(handle);
-		*status = PCAP_ERROR;
-		return -1;
+		return PCAP_ERROR;
 	}
 
 	/* fill the header ring with proper frame ptr*/
@@ -3279,7 +3287,7 @@ retry:
 
 	handle->bufsize = req.tp_frame_size;
 	handle->offset = 0;
-	return 1;
+	return status;
 }
 
 /* free all ring related resources*/