The exit tests hangs, as expected. I didn't try the patch yet.

Update a helper function name

I think this is ok, the selinfo is global so the recent lifetime problems we fixed don't apply here. If it were possible to destroy this device file you'd want to issue seldrain(), but I think that never happens.

Is there any reason this didn't land?

Commit 72ea8f41e19d3 subsumes this diff.

Committed in 7906084ba2fd50022f38ce2e8d0bcef212a4ff19.

It's hard for me to judge whether this is the best way to handle it, but it seems to work.

In D57411#1315567, @emaste wrote:

Aside from fixing compatibility with symlink/hard link creation we'll also need to address certctl, which currently has ${METALOG_INSTALLFLAGS} passed to it in installworld.

In D57419#1315595, @kib wrote:

This is a part of D57163, FWIW

Do you have a patch for share/mk which I can use to test this?

Unlock the vnode earlier.

In D57016#1315004, @igoro wrote:

It seems the main goal here was not about parallelism, so maybe it's not so important, but I just wanted to mention that the newly added test cases are still marked as exclusive as the Makefile makes it so for all of them:

# cat /usr/tests/sys/net/routing/Kyuafile
-- Automatically generated by bsd.test.mk.

syntax(2)

test_suite("FreeBSD")

atf_test_program{name="test_routing", is_exclusive=true, execenv="jail", execenv_jail_params="vnet"}
atf_test_program{name="test_routing_l3.py", is_exclusive=true, required_programs="pytest"}
atf_test_program{name="test_rtsock_l3", is_exclusive=true}
atf_test_program{name="test_rtsock_lladdr", is_exclusive=true}
atf_test_program{name="test_rtsock_multipath.py", is_exclusive=true, required_programs="pytest"}
atf_test_program{name="test_rtsock_ops", is_exclusive=true}

In D57372#1314941, @ivy wrote:

I'm not sure offhand which files go in this directory, but in general bhyve should be unlinking preexisting files owned by the current VM.

well, perhaps that's so; my main concern here is that if i give a user access to run bhyve VMs, that doesn't imply i want them to have access to previously-restricted directories used by root. those are two separate things that should be configured separately, imo.

if this is specific to snapshots, perhaps a new bhyve argument could be used to specify the socket directory it uses for that.

Same as D57387.

• increment more counters
• add a branch annotation

In D56942#1314723, @zlei wrote:

In D56942#1314704, @markj wrote:

In D56942#1314685, @zlei wrote:

I'm OK with this revision, although I think an explicit memory barrier while increasing / decreasing sc->sc_count will make the code more clear than a NULL check in the reader side (lagg_rr_start).

A memory barrier isn't sufficient. For sc_count and length(sc_ports) to be consistent with each other, you need a mutex.

The writer side is lagg_port_create() and lagg_port_destroy(). They are serialized. The reader side always checks sc_count before accessing sc_ports, so this order should be sufficient for the writer side,

diff --git a/sys/net/if_lagg.c b/sys/net/if_lagg.c
index 0333162da0d4..226565633b3a 100644
--- a/sys/net/if_lagg.c
+++ b/sys/net/if_lagg.c
@@ -879,7 +879,7 @@ lagg_port_create(struct lagg_softc *sc, struct ifnet *ifp)
                CK_SLIST_INSERT_AFTER(tlp, lp, lp_entries);
        else
                CK_SLIST_INSERT_HEAD(&sc->sc_ports, lp, lp_entries);
-       sc->sc_count++;
+       atomic_add_rel_int(&sc->sc_count, 1);
 
        lagg_setmulti(lp);
 
@@ -962,8 +962,8 @@ lagg_port_destroy(struct lagg_port *lp, int rundelport)
        }
 
        /* Finally, remove the port from the lagg */
+       atomic_add_rel_int(&sc->sc_count, -1);
        CK_SLIST_REMOVE(&sc->sc_ports, lp, lagg_port, lp_entries);
-       sc->sc_count--;
 
        /* Update the primary interface */
        if (lp == sc->sc_primary) {

The reader side use atomic_load_acq_int() to get sc_count, then iterate sc_ports.

In D57372#1314709, @novel wrote:

In D57372#1314650, @markj wrote:

in this case

You might further specify which case this is?

In case of running bhyve as a non-root user.

In D56942#1314685, @zlei wrote:

I'm OK with this revision, although I think an explicit memory barrier while increasing / decreasing sc->sc_count will make the code more clear than a NULL check in the reader side (lagg_rr_start).

in this case

Is it possible to add a regression test?

Ping? I've converted the locking here as requested.

Ping? I'd like to commit this if there are no objections. The bug it fixes is real, and this patch or something like it is required no matter how other if_lagg lifecycle issues are handled.

In D57274#1312947, @emaste wrote:

I mean, sure, I guess, but what's the threat model here? I sure hope you trust the tarballs you're unpacking to install a system not to be malicious, otherwise what's the point?

I could come up with a contrived exploit scenario, but yes if someone controls the tarballs being unpacked during install it's likely much easier to just provide a trojaned binary.

This came to secteam and I thought "sure, why not."