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.

diff --git a/src/arp.c b/src/arp.c
index 03e30b8c5ba929d66a1622efbde5101a4265bab5..fd3cdfe307868884cafc9d7cc744aa81185c701c 100644 (file)
--- a/src/arp.c
+++ b/src/arp.c
@@ -98,7 +98,7 @@ arp_request(const struct interface *ifp, in_addr_t sip, in_addr_t tip)
        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 @@ arp_close(struct interface *ifp)
 {
        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 @@ static void
 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_read(void *arg)
                }
                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 @@ arp_open(struct interface *ifp)
        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 @@ arp_probe(struct arp_state *astate)
        } 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 @@ arp_new(struct interface *ifp, const struct in_addr *addr)
                        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 @@ arp_new(struct interface *ifp, const struct in_addr *addr)
        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 @@ arp_free(struct arp_state *astate)
                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__);
 }
 

diff --git a/src/arp.h b/src/arp.h
index f2746965ac5e2431bfff9b385ada1e6a579924c0..9737ba9cc3c9caee572a8d21b06001364ffe69b7 100644 (file)
--- a/src/arp.h
+++ b/src/arp.h
@@ -76,7 +76,8 @@ struct arp_state {
 TAILQ_HEAD(arp_statehead, arp_state);
 
 struct iarp_state {
-       int fd;
+       int bpf_fd;
+       unsigned int bpf_flags;
        struct arp_statehead arp_states;
 };
 

diff --git a/src/bpf.c b/src/bpf.c
index a6b7bdb8ed39748641a7d9db83fc7fb370017fcb..8efca21fe7a55a34e91f32c911d8d6421e517b06 100644 (file)
--- a/src/bpf.c
+++ b/src/bpf.c
@@ -194,7 +194,8 @@ eexit:
 /* 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 @@ bpf_read(struct interface *ifp, int fd, void *data, size_t len, int *flags)
        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);

diff --git a/src/bpf.h b/src/bpf.h
index 0af94f053bf1a1b75d28b60d60fdf84f84073306..89e69bcab8ec507c40c0b58557850a89dc5dd293 100644 (file)
--- a/src/bpf.h
+++ b/src/bpf.h
@@ -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_open(struct interface *, int (*)(struct interface *, int));
 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

diff --git a/src/dhcp.c b/src/dhcp.c
index b138a6fc328bb4328bd7fbacf76129f35e75c863..1e93078d47a171c6081f7adb26c12d898f68e6ac 100644 (file)
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -1551,6 +1551,7 @@ dhcp_close(struct interface *ifp)
                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 @@ valid_udp_packet(void *data, size_t data_len, struct in_addr *from,
 }
 
 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 @@ 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);
+       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 @@ dhcp_readpacket(void *arg)
                        }
                        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;
        }
 }

diff --git a/src/dhcp.h b/src/dhcp.h
index 08203dccf8565d04e008923ebdd990fbba7644a9..134aee26a618ca2f7889a040d76f0803e5e6a81e 100644 (file)
--- a/src/dhcp.h
+++ b/src/dhcp.h
@@ -215,6 +215,7 @@ struct dhcp_state {
        int socket;
 
        int bpf_fd;
+       unsigned int bpf_flags;
        struct ipv4_addr *addr;
        uint8_t added;
 

diff --git a/src/if-linux.c b/src/if-linux.c
index b5e30eb9aeffe83e101f4076bf086e8f1ffaf50d..0d49ab6b5b8c9769c707cbdceeb6a031223d1932 100644 (file)
--- a/src/if-linux.c
+++ b/src/if-linux.c
@@ -1385,7 +1385,8 @@ eexit:
 /* 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 @@ bpf_read(struct interface *ifp, int s, void *data, size_t len, int *flags)
        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);