When the autoconf flag flaps around we might end up with multiple bpf
authorflorian <florian@openbsd.org>
Thu, 14 Jul 2022 15:23:09 +0000 (15:23 +0000)
committerflorian <florian@openbsd.org>
Thu, 14 Jul 2022 15:23:09 +0000 (15:23 +0000)
FDs in flight. Things then get confusing. The kernel tells us we can
read from the bpf FD but the data is actually "on the other FD", so
read(2) returns 0.

Found the hard way by, and patiently debugged with weerd@

One way to trigger this is booting a vmm VM where dhcpleased(8)'s
init_ifaces() loses a race against netstart(8). init_ifaces() would
already see the autoconf flag and request a bpf FD.
But then it would receive a RTM_IFINFO message without the autoconf flag
set from when the interface came up. Then it will see another RTM_IFINFO
message with the autoconf flag set and request yet another bpf FD. If
the first bpf FD had not arrived yet we ended up with two in the frontend
process.

While here make sure a bpf FD has been received for an iface before
trying to close(2) it.

tweak & OK dv

sbin/dhcpleased/frontend.c

index 2959a6c..e94a359 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: frontend.c,v 1.29 2022/04/26 14:50:04 florian Exp $   */
+/*     $OpenBSD: frontend.c,v 1.30 2022/07/14 15:23:09 florian Exp $   */
 
 /*
  * Copyright (c) 2017, 2021 Florian Obser <florian@openbsd.org>
@@ -1170,8 +1170,10 @@ remove_iface(uint32_t if_index)
                return;
 
        LIST_REMOVE(iface, entries);
-       event_del(&iface->bpfev.ev);
-       close(EVENT_FD(&iface->bpfev.ev));
+       if (event_initialized(&iface->bpfev.ev)) {
+               event_del(&iface->bpfev.ev);
+               close(EVENT_FD(&iface->bpfev.ev));
+       }
        if (iface->udpsock != -1)
                close(iface->udpsock);
        free(iface);
@@ -1182,10 +1184,18 @@ set_bpfsock(int bpfsock, uint32_t if_index)
 {
        struct iface    *iface;
 
-       if ((iface = get_iface_by_id(if_index)) == NULL) {
+       iface = get_iface_by_id(if_index);
+
+       if (iface == NULL) {
                /*
                 * The interface disappeared while we were waiting for the
-                * parent process to open the raw socket.
+                * parent process to open the bpf socket.
+                */
+               close(bpfsock);
+       } else if (event_initialized(&iface->bpfev.ev)) {
+               /*
+                * The autoconf flag is flapping and we have multiple bpf sockets in
+                * flight. We don't need this one because we already got one.
                 */
                close(bpfsock);
        } else {