From: Roy Marples <roy@marples.name>
Date: Tue, 28 Jun 2016 12:22:51 +0000 (+0000)
Subject: Revert [da213b6490] because the fd we're reading from could be a generic
X-Git-Tag: v6.11.2~36
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9d751d945ef523602b9ecf39b32b92b521a5400a;p=thirdparty%2Fdhcpcd.git

Revert [da213b6490] because the fd we're reading from could be a generic
file descriptor and have no concept of packet boundaries
(this is true for BPF).
Re-implement, but cleaner and with more commentary.
---

diff --git a/arp.c b/arp.c
index 5b35ccdd..04cfc17b 100644
--- a/arp.c
+++ b/arp.c
@@ -119,32 +119,19 @@ arp_report_conflicted(const struct arp_state *astate,
 }
 
 static void
-arp_packet(void *arg)
+arp_packet(struct interface *ifp, uint8_t *data, size_t len)
 {
-	struct interface *ifp = arg;
 	const struct interface *ifn;
-	uint8_t buf[ARP_LEN];
 	struct arphdr ar;
 	struct arp_msg arm;
-	ssize_t bytes;
-	struct iarp_state *state;
+	const struct iarp_state *state;
 	struct arp_state *astate, *astaten;
-	unsigned char *hw_s, *hw_t;
-	int flags;
+	uint8_t *hw_s, *hw_t;
 
-	state = ARP_STATE(ifp);
-	flags = 0;
-	bytes = if_readraw(ifp, state->fd, buf, sizeof(buf), &flags);
-	if (bytes == -1) {
-		logger(ifp->ctx, LOG_ERR,
-		    "%s: arp if_readrawpacket: %m", ifp->name);
-		arp_close(ifp);
-		return;
-	}
 	/* We must have a full ARP header */
-	if ((size_t)bytes < sizeof(ar))
+	if (len < sizeof(ar))
 		return;
-	memcpy(&ar, buf, sizeof(ar));
+	memcpy(&ar, data, sizeof(ar));
 	/* Families must match */
 	if (ar.ar_hrd != htons(ifp->family))
 		return;
@@ -162,10 +149,10 @@ arp_packet(void *arg)
 		return;
 
 	/* Get pointers to the hardware addreses */
-	hw_s = buf + sizeof(ar);
+	hw_s = data + sizeof(ar);
 	hw_t = hw_s + ar.ar_hln + ar.ar_pln;
 	/* Ensure we got all the data */
-	if ((hw_t + ar.ar_hln + ar.ar_pln) - buf > bytes)
+	if ((size_t)((hw_t + ar.ar_hln + ar.ar_pln) - data) > len)
 		return;
 	/* Ignore messages from ourself */
 	TAILQ_FOREACH(ifn, ifp->ctx->ifaces, next) {
@@ -187,12 +174,39 @@ arp_packet(void *arg)
 	memcpy(&arm.tip.s_addr, hw_t + ar.ar_hln, ar.ar_pln);
 
 	/* Run the conflicts */
+	state = ARP_CSTATE(ifp);
 	TAILQ_FOREACH_SAFE(astate, &state->arp_states, next, astaten) {
 		if (astate->conflicted_cb)
 			astate->conflicted_cb(astate, &arm);
 	}
 }
 
+static void
+arp_read(void *arg)
+{
+	struct interface *ifp = arg;
+	const struct iarp_state *state;
+	uint8_t buf[ARP_LEN];
+	int flags;
+	ssize_t bytes;
+
+	/* Some RAW mechanisms are generic file descriptors, not sockets.
+	 * This means we have no kernel call to just get one packet,
+	 * so we have to process the entire buffer. */
+	state = ARP_CSTATE(ifp);
+	flags = 0;
+	while (!(flags & RAW_EOF)) {
+		bytes = if_readraw(ifp, state->fd, buf, sizeof(buf), &flags);
+		if (bytes == -1) {
+			logger(ifp->ctx, LOG_ERR,
+			    "%s: arp if_readrawpacket: %m", ifp->name);
+			arp_close(ifp);
+			return;
+		}
+		arp_packet(ifp, buf, (size_t)bytes);
+	}
+}
+
 int
 arp_open(struct interface *ifp)
 {
@@ -206,7 +220,7 @@ arp_open(struct interface *ifp)
 			    __func__, ifp->name);
 			return -1;
 		}
-		eloop_event_add(ifp->ctx->eloop, state->fd, arp_packet, ifp);
+		eloop_event_add(ifp->ctx->eloop, state->fd, arp_read, ifp);
 	}
 	return state->fd;
 }
diff --git a/dhcp.c b/dhcp.c
index 0d473882..f1fba03f 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -3125,27 +3125,15 @@ valid_udp_packet(uint8_t *data, size_t data_len, struct in_addr *from,
 }
 
 static void
-dhcp_handlepacket(void *arg)
+dhcp_handlepacket(struct interface *ifp, uint8_t *data, size_t len, int flags)
 {
-	struct interface *ifp = arg;
-	uint8_t *bootp, buf[MTU_MAX];
-	size_t bytes;
+	uint8_t *bootp;
 	struct in_addr from;
-	int i, flags;
+	int i;
+	size_t udp_len;
 	const struct dhcp_state *state = D_CSTATE(ifp);
 
-	/* Need this API due to BPF */
-	flags = 0;
-	bootp = NULL;
-	bytes = (size_t)if_readraw(ifp, state->raw_fd,buf, sizeof(buf), &flags);
-	if ((ssize_t)bytes == -1) {
-		logger(ifp->ctx, LOG_ERR,
-		    "%s: dhcp if_readrawpacket: %m", ifp->name);
-		dhcp_close(ifp);
-		arp_close(ifp);
-		return;
-	}
-	if (valid_udp_packet(buf, bytes, &from, flags & RAW_PARTIALCSUM) == -1)
+	if (valid_udp_packet(data, len, &from, flags & RAW_PARTIALCSUM) == -1)
 	{
 		logger(ifp->ctx, LOG_ERR, "%s: invalid UDP packet from %s",
 		    ifp->name, inet_ntoa(from));
@@ -3170,6 +3158,7 @@ dhcp_handlepacket(void *arg)
 		    "%s: server %s is not destination",
 		    ifp->name, inet_ntoa(from));
 	}
+
 	/*
 	 * DHCP has a variable option area rather than a fixed vendor area.
 	 * Because DHCP uses the BOOTP protocol it should still send BOOTP
@@ -3177,19 +3166,45 @@ dhcp_handlepacket(void *arg)
 	 * However some servers send a truncated vendor area.
 	 * dhcpcd can work fine without the vendor area being sent.
 	 */
-	bytes = get_udp_data(&bootp, buf);
-	if (bytes < offsetof(struct bootp, vend)) {
+	udp_len = get_udp_data(&bootp, data);
+	if (udp_len < offsetof(struct bootp, vend)) {
 		logger(ifp->ctx, LOG_ERR,
 		    "%s: truncated packet (%zu) from %s",
-		    ifp->name, bytes, inet_ntoa(from));
+		    ifp->name, udp_len, inet_ntoa(from));
 		return;
 	}
 	/* But to make our IS_DHCP macro easy, ensure the vendor
 	 * area has at least 4 octets. */
-	while (bytes < offsetof(struct bootp, vend) + 4)
-		bootp[bytes++] = '\0';
+	while (udp_len < offsetof(struct bootp, vend) + 4)
+		bootp[udp_len++] = '\0';
 
-	dhcp_handledhcp(ifp, (struct bootp *)bootp, bytes, &from);
+	dhcp_handledhcp(ifp, (struct bootp *)bootp, udp_len, &from);
+}
+
+static void
+dhcp_readpacket(void *arg)
+{
+	struct interface *ifp = arg;
+	uint8_t buf[MTU_MAX];
+	ssize_t bytes;
+	int flags;
+	const struct dhcp_state *state = D_CSTATE(ifp);
+
+	/* Some RAW mechanisms are generic file descriptors, not sockets.
+	 * This means we have no kernel call to just get one packet,
+	 * so we have to process the entire buffer. */
+	flags = 0;
+	while (!(flags & RAW_EOF)) {
+		bytes = if_readraw(ifp, state->raw_fd, buf,sizeof(buf), &flags);
+		if (bytes == -1) {
+			logger(ifp->ctx, LOG_ERR,
+			    "%s: dhcp if_readrawpacket: %m", ifp->name);
+			dhcp_close(ifp);
+			arp_close(ifp);
+			return;
+		}
+		dhcp_handlepacket(ifp, buf, (size_t)bytes, flags);
+	}
 }
 
 static void
@@ -3231,7 +3246,7 @@ dhcp_open(struct interface *ifp)
 			return -1;
 		}
 		eloop_event_add(ifp->ctx->eloop,
-		    state->raw_fd, dhcp_handlepacket, ifp);
+		    state->raw_fd, dhcp_readpacket, ifp);
 	}
 	return 0;
 }
diff --git a/if-bsd.c b/if-bsd.c
index 1628a121..ec46684f 100644
--- a/if-bsd.c
+++ b/if-bsd.c
@@ -526,14 +526,15 @@ if_readraw(struct interface *ifp, int fd, void *data, size_t len, int *flags)
 next:
 		state->buffer_pos += BPF_WORDALIGN(packet.bh_hdrlen +
 		    packet.bh_caplen);
-		if (state->buffer_pos >= state->buffer_len)
+		if (state->buffer_pos >= state->buffer_len) {
 			state->buffer_len = state->buffer_pos = 0;
+			*flags |= RAW_EOF;
+		}
 		if (bytes != -1)
 			return bytes;
 	}
 }
 
-
 int
 if_address(unsigned char cmd, const struct ipv4_addr *ia)
 {
diff --git a/if-linux.c b/if-linux.c
index 4bb800bc..f3dc5fe8 100644
--- a/if-linux.c
+++ b/if-linux.c
@@ -1334,7 +1334,7 @@ if_readraw(__unused struct interface *ifp, int fd,
 	bytes = recvmsg(fd, &msg, 0);
 	if (bytes == -1)
 		return -1;
-	*flags = 0;
+	*flags = RAW_EOF; /* We only ever read one packet. */
 	if (bytes) {
 #ifdef PACKET_AUXDATA
 		for (cmsg = CMSG_FIRSTHDR(&msg);
diff --git a/if.h b/if.h
index 79d04ed6..249dd2e4 100644
--- a/if.h
+++ b/if.h
@@ -80,7 +80,8 @@
 	    ((addr & IN_CLASSB_NET) == 0xc0a80000))
 #endif
 
-#define RAW_PARTIALCSUM		1 << 0
+#define RAW_EOF			1 << 0
+#define RAW_PARTIALCSUM		2 << 0
 
 #ifdef __sun
 /* Solaris getifaddrs is very un-suitable for dhcpcd.