Mercurial > hg > dhcpcd

changeset 4169:07bfee79bd7a draft

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
bpf: store flags in state for a better abort There are cases when dhcp may close and open an ARP socket during an ARP read. This means the fd will not be -1, so we need to set BPF_EOF when closing the socket.
author Roy Marples <roy@marples.name>
date Tue, 24 Oct 2017 23:11:15 +0100
parents e8510a89cdb2
children cc4fbeafc390
files src/arp.c src/arp.h src/bpf.c src/bpf.h src/dhcp.c src/dhcp.h src/if-linux.c
diffstat 7 files changed, 47 insertions(+), 38 deletions(-) [+]
line wrap: on
line diff
--- a/src/arp.c	Tue Oct 24 00:13:09 2017 +0100
+++ b/src/arp.c	Tue Oct 24 23:11:15 2017 +0100
@@ -98,7 +98,7 @@
 	APPEND(&tip, sizeof(tip));
 
 	state = ARP_CSTATE(ifp);
-	return bpf_send(ifp, state->fd, ETHERTYPE_ARP, arp_buffer, len);
+	return bpf_send(ifp, state->bpf_fd, ETHERTYPE_ARP, arp_buffer, len);
 
 eexit:
 	errno = ENOBUFS;
@@ -177,10 +177,11 @@
 {
 	struct iarp_state *state;
 
-	if ((state = ARP_STATE(ifp)) != NULL && state->fd != -1) {
-		eloop_event_delete(ifp->ctx->eloop, state->fd);
-		bpf_close(ifp, state->fd);
-		state->fd = -1;
+	if ((state = ARP_STATE(ifp)) != NULL && state->bpf_fd != -1) {
+		eloop_event_delete(ifp->ctx->eloop, state->bpf_fd);
+		bpf_close(ifp, state->bpf_fd);
+		state->bpf_fd = -1;
+		state->bpf_flags |= BPF_EOF;
 	}
 }
 
@@ -188,18 +189,18 @@
 arp_read(void *arg)
 {
 	struct interface *ifp = arg;
-	const struct iarp_state *state;
+	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 & BPF_EOF)) {
-		bytes = bpf_read(ifp, state->fd, buf, sizeof(buf), &flags);
+	state = ARP_STATE(ifp);
+	state->bpf_flags &= ~BPF_EOF;
+	while (!(state->bpf_flags & BPF_EOF)) {
+		bytes = bpf_read(ifp, state->bpf_fd, buf, sizeof(buf),
+				 &state->bpf_flags);
 		if (bytes == -1) {
 			logerr("%s: %s", __func__, ifp->name);
 			arp_close(ifp);
@@ -207,7 +208,7 @@
 		}
 		arp_packet(ifp, buf, (size_t)bytes);
 		/* Check we still have a state after processing. */
-		if ((state = ARP_CSTATE(ifp)) == NULL || state->fd == -1)
+		if ((state = ARP_STATE(ifp)))
 			break;
 	}
 }
@@ -218,15 +219,15 @@
 	struct iarp_state *state;
 
 	state = ARP_STATE(ifp);
-	if (state->fd == -1) {
-		state->fd = bpf_open(ifp, bpf_arp);
-		if (state->fd == -1) {
+	if (state->bpf_fd == -1) {
+		state->bpf_fd = bpf_open(ifp, bpf_arp);
+		if (state->bpf_fd == -1) {
 			logerr("%s: %s", __func__, ifp->name);
 			return -1;
 		}
-		eloop_event_add(ifp->ctx->eloop, state->fd, arp_read, ifp);
+		eloop_event_add(ifp->ctx->eloop, state->bpf_fd, arp_read, ifp);
 	}
-	return state->fd;
+	return state->bpf_fd;
 }
 
 static void
@@ -273,7 +274,7 @@
 	} else {
 		const struct iarp_state *state = ARP_CSTATE(astate->iface);
 
-		if (bpf_arp(astate->iface, state->fd) == -1)
+		if (bpf_arp(astate->iface, state->bpf_fd) == -1)
 			logerr(__func__);
 	}
 	astate->probes = 0;
@@ -451,7 +452,8 @@
 			logerr(__func__);
 			return NULL;
 		}
-		state->fd = -1;
+		state->bpf_fd = -1;
+		state->bpf_flags = 0;
 		TAILQ_INIT(&state->arp_states);
 	} else {
 		if (addr && (astate = arp_find(ifp, addr)))
@@ -468,7 +470,7 @@
 	state = ARP_STATE(ifp);
 	TAILQ_INSERT_TAIL(&state->arp_states, astate, next);
 
-	if (bpf_arp(ifp, state->fd) == -1)
+	if (bpf_arp(ifp, state->bpf_fd) == -1)
 		logerr(__func__); /* try and continue */
 
 	return astate;
@@ -504,7 +506,7 @@
 		free(state);
 		ifp->if_data[IF_DATA_ARP] = NULL;
 	} else
-		if (bpf_arp(ifp, state->fd) == -1)
+		if (bpf_arp(ifp, state->bpf_fd) == -1)
 			logerr(__func__);
 }
 
--- a/src/arp.h	Tue Oct 24 00:13:09 2017 +0100
+++ b/src/arp.h	Tue Oct 24 23:11:15 2017 +0100
@@ -76,7 +76,8 @@
 TAILQ_HEAD(arp_statehead, arp_state);
 
 struct iarp_state {
-	int fd;
+	int bpf_fd;
+	unsigned int bpf_flags;
 	struct arp_statehead arp_states;
 };
 
--- a/src/bpf.c	Tue Oct 24 00:13:09 2017 +0100
+++ b/src/bpf.c	Tue Oct 24 23:11:15 2017 +0100
@@ -194,7 +194,8 @@
 /* BPF requires that we read the entire buffer.
  * So we pass the buffer in the API so we can loop on >1 packet. */
 ssize_t
-bpf_read(struct interface *ifp, int fd, void *data, size_t len, int *flags)
+bpf_read(struct interface *ifp, int fd, void *data, size_t len,
+    unsigned int *flags)
 {
 	ssize_t fl = (ssize_t)bpf_frame_header_len(ifp);
 	ssize_t bytes;
@@ -203,7 +204,7 @@
 	struct bpf_hdr packet;
 	const char *payload;
 
-	*flags = 0;
+	*flags &= ~BPF_EOF;
 	for (;;) {
 		if (state->buffer_len == 0) {
 			bytes = read(fd, state->buffer, state->buffer_size);
--- a/src/bpf.h	Tue Oct 24 00:13:09 2017 +0100
+++ b/src/bpf.h	Tue Oct 24 23:11:15 2017 +0100
@@ -28,8 +28,8 @@
 #ifndef BPF_HEADER
 #define BPF_HEADER
 
-#define BPF_EOF			1 << 0
-#define BPF_PARTIALCSUM		2 << 0
+#define BPF_EOF			(1U << 0)
+#define BPF_PARTIALCSUM		(2U << 0)
 
 #include "dhcpcd.h"
 
@@ -39,7 +39,7 @@
 int bpf_close(struct interface *, int);
 int bpf_attach(int, void *, unsigned int);
 ssize_t bpf_send(const struct interface *, int, uint16_t, const void *, size_t);
-ssize_t bpf_read(struct interface *, int, void *, size_t, int *);
+ssize_t bpf_read(struct interface *, int, void *, size_t, unsigned int *);
 int bpf_arp(struct interface *, int);
 int bpf_bootp(struct interface *, int);
 #endif
--- a/src/dhcp.c	Tue Oct 24 00:13:09 2017 +0100
+++ b/src/dhcp.c	Tue Oct 24 23:11:15 2017 +0100
@@ -1551,6 +1551,7 @@
 		eloop_event_delete(ifp->ctx->eloop, state->bpf_fd);
 		bpf_close(ifp, state->bpf_fd);
 		state->bpf_fd = -1;
+		state->bpf_flags |= BPF_EOF;
 	}
 
 	state->interval = 0;
@@ -3236,14 +3237,15 @@
 }
 
 static void
-dhcp_handlepacket(struct interface *ifp, uint8_t *data, size_t len, int flags)
+dhcp_handlepacket(struct interface *ifp, uint8_t *data, size_t len)
 {
 	struct bootp *bootp;
 	struct in_addr from;
 	size_t udp_len;
 	const struct dhcp_state *state = D_CSTATE(ifp);
 
-	if (valid_udp_packet(data, len, &from, flags & RAW_PARTIALCSUM) == -1)
+	if (valid_udp_packet(data, len, &from,
+			     state->bpf_flags & RAW_PARTIALCSUM) == -1)
 	{
 		if (errno == EINVAL)
 			logerrx("%s: UDP checksum failure from %s",
@@ -3292,15 +3294,15 @@
 	struct interface *ifp = arg;
 	uint8_t buf[MTU_MAX];
 	ssize_t bytes;
-	int flags;
-	const struct dhcp_state *state = D_CSTATE(ifp);
+	struct dhcp_state *state = D_STATE(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 & BPF_EOF)) {
-		bytes = bpf_read(ifp, state->bpf_fd, buf, sizeof(buf), &flags);
+	state->bpf_flags &= ~BPF_EOF;
+	while (!(state->bpf_flags & BPF_EOF)) {
+		bytes = bpf_read(ifp, state->bpf_fd, buf, sizeof(buf),
+				 &state->bpf_flags);
 		if (bytes == -1) {
 			if (state->state != DHS_NONE) {
 				logerr("%s: %s", __func__, ifp->name);
@@ -3308,9 +3310,9 @@
 			}
 			return;
 		}
-		dhcp_handlepacket(ifp, buf, (size_t)bytes, flags);
+		dhcp_handlepacket(ifp, buf, (size_t)bytes);
 		/* Check we still have a state after processing. */
-		if ((state = D_CSTATE(ifp)) == NULL || state->bpf_fd == -1)
+		if ((state = D_STATE(ifp)) == NULL)
 			break;
 	}
 }
--- a/src/dhcp.h	Tue Oct 24 00:13:09 2017 +0100
+++ b/src/dhcp.h	Tue Oct 24 23:11:15 2017 +0100
@@ -215,6 +215,7 @@
 	int socket;
 
 	int bpf_fd;
+	unsigned int bpf_flags;
 	struct ipv4_addr *addr;
 	uint8_t added;
 
--- a/src/if-linux.c	Tue Oct 24 00:13:09 2017 +0100
+++ b/src/if-linux.c	Tue Oct 24 23:11:15 2017 +0100
@@ -1385,7 +1385,8 @@
 /* BPF requires that we read the entire buffer.
  * So we pass the buffer in the API so we can loop on >1 packet. */
 ssize_t
-bpf_read(struct interface *ifp, int s, void *data, size_t len, int *flags)
+bpf_read(struct interface *ifp, int s, void *data, size_t len,
+    unsigned int *flags)
 {
 	ssize_t bytes;
 	struct ipv4_state *state = IPV4_STATE(ifp);
@@ -1409,7 +1410,8 @@
 	bytes = recvmsg(s, &msg, 0);
 	if (bytes == -1)
 		return -1;
-	*flags = BPF_EOF; /* We only ever read one packet. */
+	*flags |= BPF_EOF; /* We only ever read one packet. */
+	*flags &= ~BPF_PARTIALCSUM;
 	if (bytes) {
 		ssize_t fl = (ssize_t)bpf_frame_header_len(ifp);