I think this effectively leaves us (FreeBSD) open to a username enumeration vulnerability, because we do not have a blocklist probe in grace_alarm_handler().
I have been exploring simpler options to try to make our BLOCKLIST_NOTIFY() signal-safe.

Sure enough (https://reviews.freebsd.org/D58083?id=181447#inline-344443).
The patch for NetBSD would be:

diff
--- a/crypto/external/bsd/openssh/dist/sshd-session.c
+++ b/crypto/external/bsd/openssh/dist/sshd-session.c
@@ -1346,7 +1346,9 @@ cleanup_exit(int i)
                pfilter_notify(1);
                _exit(EXIT_AUTH_ATTEMPTED);
        }
-       if (i == 255 && monitor_invalid_user())
+       if (i == 255 && monitor_invalid_user()) {
+               pfilter_notify(1);
                _exit(EXIT_INVALID_USER);
+       }
        _exit(i);
 }

In D58150#1334946, @vmaffione wrote:

Do you need me to commit this?

LGTM.
I always get a few extra namespace defines, as I'm testing on arm64.

This looks good to me.
I tried re-creating it yesterday, and arrived at the same result (assuming SSH_VERSION_FREEBSD will be bumped upon commit).
Let me know your diff after freebsd-configure.sh and freebsd-namespace.sh, as there were a few changes there as well.

This is fine by me.
So, if git-rev-parse(1) fails, we just silently ignore it? Maybe erroring in the else branch is too aggressive.

Closed in cfd5bdcb4ee56c658ebc7cf7ea941261d1cd7f73

Note to self: Rebase D54585 after this has landed.

Nice! Thank you!
I think we can simplify and improve the while read ... block, but that is not related to this change.

In D57527#1324036, @jhb wrote:

In D57527#1323972, @jlduran wrote:

In D57527#1323967, @jhb wrote:

I tested this by building a little ISO and verifying the timestamps (including birthtime) of the files in the ISO matched the existing files used to build the ISO. I even had it wrong initially (I had forgotten to update the relative offsets to TF.timestamp. Arguably if would be more future-proof to follow the model of the kernel parsing code which sets a pointer to TF.timestamp and increments the pointer after encoding each timestamp, but I can't see us ever adding the three remaining timestamps).

Thank you, I'll rekindle a series of makefs updates and test changes (D54585, et al.) to include cd9660 as well. We kind of need these for accurate time-stamping of NanoBSD images.

Do you want me to merge this first or wait for your other patch to land?

In D57527#1323967, @jhb wrote:

I tested this by building a little ISO and verifying the timestamps (including birthtime) of the files in the ISO matched the existing files used to build the ISO. I even had it wrong initially (I had forgotten to update the relative offsets to TF.timestamp. Arguably if would be more future-proof to follow the model of the kernel parsing code which sets a pointer to TF.timestamp and increments the pointer after encoding each timestamp, but I can't see us ever adding the three remaining timestamps).

In D54144#1266186, @emaste wrote:

LGTM. Is this review waiting on something? (Perhaps just finding appropriate reviewers?)

I have the exact same feeling as @glebius: Ideally we'd check whether the route already exists before invoking route(8), but that's considerably more involved than filtering the expected EEXIST case.
While I'm comfortable blaming the lines and reading the commit messages, a brief comment like:

# Loopback routes may already be added by the kernel; ignore EEXIST.

As @markj suggested would be ideal (though not compulsory).
The commit message initially threw me off. I thought the change was only silencing the message for FIB 0, but as you've probably inferred from my setup (net.add_addr_allfibs=1), it also suppresses it for the other routing tables.
Thank you for all your work!

In D57470#1317912, @glebius wrote:

Maybe filter the very specific message only?

2>&1 >/dev/null | sed -e '/route: message indicates error: File exists/d'

As someone who gets:

...
route: message indicates error: File exists
route: message indicates error: File exists
route: message indicates error: File exists
route: message indicates error: File exists
route: message indicates error: File exists
add host 127.0.0.1: gateway lo0 fib 0: route already in table
add host 127.0.0.1: gateway lo0 fib 1: route already in table
add host 127.0.0.1: gateway lo0 fib 2: route already in table
add host 127.0.0.1: gateway lo0 fib 3: route already in table
add host 127.0.0.1: gateway lo0 fib 4: route already in table
...

on every boot, I greatly welcome this patch. So, the reason why we don't remove it above (see inline comment) is because the route won't be shown as static?:

127.0.0.1          link#3             UH              lo0

Update COMPONENTS examples:

I echo the words of @imp; I don't even have a real way to test it.
Having said that, this is really cool from a retro-computing perspective.
We need a "It's a U**X system, I know this!" GIF.

Update the manual page example mostly with @asomers' suggestion (a simpler version).
Note that the tmux package was chosen because it is known to be present on the DVD.

In D57481#1316903, @jlduran wrote:

In D57481#1316901, @asomers wrote:

Ahh, our comments crossed in the ether. But now that I see what you did, I request leaving "pkg install" in. IMHO it's a very important feature of the installerscript that it can install packages.

OK, In that case I must diagnose to see what is failing.

In D57481#1316901, @asomers wrote:

Ahh, our comments crossed in the ether. But now that I see what you did, I request leaving "pkg install" in. IMHO it's a very important feature of the installerscript that it can install packages.

In D57481#1316900, @asomers wrote:

What's this about "removing the pkg install step"? I don't see anything removed.

• Remove pkg install

OK, I will commit and MFC ASAP.
I also opted for removing the pkg install step, as it may fail on -CURRENT (see attached snapshot).

I tried to make it before 15.1-RC2, but I couldn't. Probably a good candidate for errata.

Also, note that PARTITIONS is not used in any other script, so there is no need to export (there has never been the need to export it either).

Hmm, I was about to submit a change that:

The files /boot/boot0 and /boot/boot0sio have 0444 permissions. chmod before when building unprivileged.

• Add more checks

Sorry for not testing this thoroughly, I just realized that in order for DISTRIBUTIONS to work, the variable must be exported (see inline). Otherwise the user should "manually" export it. If this is going to be the case, then the manual page should be updated to include export DISTRIBUTIONS in the example.

In D57311#1313282, @imp wrote:

boot0cfg ${NANO_BOOT0CFG} ${NANO_WORLDDIR}/${NANO_BOOTLOADER} doesn't look right. The only bare arg to boot0cfg is the disk, right?

I did this modification, mostly to allow using it in conjunction with mkimg (see the next review in the series). I cannot immediately see any downsides of also enabling a file for modification, I also didn't know whom to ask for a review, as boot0cfg's usage is slowly fading away.