Whilst reboot(8) will block whilst it runs, shutdown(8) does not,
daemonizing instead. This means that we must wait after running it,
otherwise we will exit and cause the system to attempt to go multi-user
in parallel with the shutdown daemon killing init. With the new
multi-console support in the installer, runconsoles will immediately
kill this daemon, racing with the daemon being able to signal init as
desired, and I have seen this race be lost in QEMU with a single CPU. In
the past this wasn't such an issue, since shutdown's daemon puts itself
in a new session group immediately after fork (and the parent doesn't
wait until that has happened, so whilst there's technically a race
condition in there where it could receive a SIGHUP from the death of the
parent's session leader, in practice this is very unlikely to be hit.
This means that the only consequence of this oversight before was that
you might get the beginnings of more console output on the way to
multi-user and thus the console would look a little confusing.
Reviewed by: gjb
Fixes: e4505364c0 ("release/rc.local: Provide option to shutdown after installation complete")
Fixes: a09af1b7fd ("bsdinstall release: Start installer on multiple consoles")
Differential Revision: https://reviews.freebsd.org/D36879
On the receive socket, recvmsg() and poll()/select() is called.
Therefore, CAP_EVENT is needed in addition to CAP_RECV..
While there, check the socket for readbility before calling recvmsg().
Reviewed by: markj@
MFC after: 3 days
Sponsored by: Netflix, Inc.
Differential Revision: https://reviews.freebsd.org/D36878
nvlist_find_string() would return a copy of the found value, but callers
assumed they would have to make their own copy. It's simpler to change
nvlist_find_string() than it is to change callers, so do that.
Reported by: Coverity
The netmap-specific data stored at be->opaque is freed by the caller
on error as part of freeing be.
Reviewed by: markj
Reported by: GCC -Wfree-nonheap-object
Differential Revision: https://reviews.freebsd.org/D36828
NLM 4 status values are a superset of the older NLM protocol so these
casts are safe while pacifying -Wenum-conversion warnings from GCC.
Reviewed by: rmacklem
Differential Revision: https://reviews.freebsd.org/D36816
Whilst febca0e643 added -N for both build and extract, it only fully
worked for extract; build would perform the actual tree build with
-DNO_ROOT and construct the intended METALOG, but the subsequent tarball
creation did not take this into account and just tarred up the the
directory as-is rather than using the METALOG. This resulted in the
permissions and ownership not being correct, as well as there being a
stray METALOG file in the tarball's root.
Reported by: avg
Reviewed by: avg, jhb, imp
Tested by: avg
Fixes: febca0e643 ("etcupdate: Add a -N flag to perform a NO_ROOT build")
Differential Revision: https://reviews.freebsd.org/D36845
Currently the installer is only started on the primary ("high level")
console. For systems where this is the video console and serial consoles
aren't of interest, and headless systems with just a serial console,
this works just fine, but for systems where both video and serial
consoles are present and meaningful this requires the user to select the
right primary console in loader, with the poor user experience of the
system appearing to hang if they leave the wrong one selected. This
notably differs from our multi-user behaviour of spawning getty on every
console, where the only issue with selecting the wrong primary console
is a quieter boot process until the login prompt appears (or the system
crashes).
Instead, use the newly-added runconsoles helper to run the installer on
every console (except for ttyv*, where only ttyv0 will be used). For
interactive installations, any of the consoles can be used, though only
one should be used at a time as no effort is made to avoid multiple
installations running at the same time clobbering each other. If the
Live CD option is selected, the other installers (which should, if the
user is well-behaved, be sitting at the welcome screen) will be killed.
If an automated install is in use, the primary console will be used to
display its output, and the others will direct the user to the primary
console.
Reviewed by: brooks, gjb
Differential Revision: https://reviews.freebsd.org/D36805
This separates out the install media-specific environment (creating
bsdinstall_etc) from actually running the installer on a given console.
This will be used by a future change to start the installer on multiple
consoles.
Reviewed by: brooks, gjb
Differential Revision: https://reviews.freebsd.org/D36803
This helper binary will run a given command on every on console, as
defined by /etc/ttys (except for ttyv*, where only ttyv0 will be used).
If one of the command processes exits, the rest will be killed. This
will be used by a future change to start the installer on multiple
consoles.
Reviewed by: brooks, imp, gjb
Differential Revision: https://reviews.freebsd.org/D36804
Mostly remove from the SEE ALSO section, adding a mention of the port
where not removed. Elsewhere, remove as appropriate and change from .Xr
to .Nm where a mention of telnetd continues to make sense (or removing
it would require significant reworking of the surrounding text).
Reviewed by: imp, delphij, emaste
Differential Revision: https://reviews.freebsd.org/D36785
- Add const and __unused qualifiers where appropriate.
- Localize some global variables.
- Consistently spell vmexit state as "vme" in vmexit handlers, to avoid
shadowing the global vm_exit state array.
- Similarly, avoid shadowing "optarg".
MFC after: 2 weeks
This is easier to read and addresses some compiler warnings.
One might expect these tables to be read-only but it seems that the
snapshot/restore code may modify them.
MFC after: 2 weeks
- Avoid arithmetic on void pointers.
- Avoid a signed/unsigned comparison in loops which write or fill audio
data buffers.
Convert while loops to for loops while here.
MFC after: 2 weeks
* change current NTP services offered by the FreeBSD Installer;
* no longer offer ntpdate to be enabled and started on boot;
* start offering the option to make ntpd set the date and time on boot itself.
The motivation for this change comes from the ntpdate(8) manpage:
Note: The functionality of this program is now available in the ntpd(8)
program. See the -q command line option in the ntpd(8) page. After a
suitable period of mourning, the ntpdate utility is to be retired from
this distribution.
Approved by: cy (src), dteske (src)
Differential Revision: https://reviews.freebsd.org/D36206
The RFC for this finally got published and, therefore,
now has a number. This patch puts this RFC number
in the man page.
This is a content change.
MFC after: 1 week
The RFC for this finally got published and, therefore,
now has a number. This patch puts this RFC number
in the man page.
This is a content change.
MFC after: 1 week
It is some times hard to understand the difference between
kernel version and userland version. So clarify the -r option
of uname(1) in terms of a printed kernel version.
While here, add some cross references:
- cross reference freebsd-version(1) in uname(1)
- cross reference freebsd-version(1) and uname(1) in freebsd-update(8)
PR: 265594
Reported by: rwatson
Reviewed by: gbe, imp
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D36516
Add VM_EXITCODE_IPI to permit returning unhandled IPIs to userland.
INIT and Startup IPIs are now returned to userland. Due to backward
compatibility reasons, a new capability is added for enabling
VM_EXITCODE_IPI.
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D35623
Sponsored by: Beckhoff Automation GmbH & Co. KG
vcpus could be restarted by the guest by sending an INIT SIPI SIPI
sequence to a vcpu. That's not supported by bhyve yet but it will be
supported in a future commit. So, create the vcpu threads only once on
startup to make restarting a vcpu easier.
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D35621
Sponsored by: Beckhoff Automation GmbH & Co. KG
- Ignore I/O requests with insufficiently sized input or output
buffers (those not containing compete request headers).
- Ignore control requests with improperly sized buffers.
- While here, explicitly zero the output header of an I/O request to
avoid leaking malloc garbage from the host if the header is not
fully populated.
PR: 264521
Reported by: Robert Morris <rtm@lcs.mit.edu>
Reviewed by: mav, emaste
MFC after: 1 week
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D36271
Use a consistent prefix ("virtio-scsi: ") similar to the e1000 device
model.
Reviewed by: mav, emaste
MFC after: 1 week
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D36270
When preparing to transmit pending packets, ensure that the head (TDH)
and tail (TDT) indices are in bounds. Note that validating values
when they are written is not sufficient along as the transmit length
(TDLEN) could be changed turning a value that was valid when written
into an out of bounds value.
While here, add further restrictions to the head register (TDH). The
manual states that writing to this value while transmit is enabled can
cause unexpected behavior and that it should only be written after a
reset. As such, ignore attempts to write while transmit is active,
and also ignore writes of non-zero values. Later e1000 chipsets have
this register as read-only.
Also ignore any attempts to transmit packets if the transmit ring's
size is zero.
PR: 264567
Reported by: Robert Morris <rtm@lcs.mit.edu>
Reviewed by: emaste
MFC after: 1 week
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D36269
Add an internal debug level global:
- Level 1 (-d) currently does nothing.
- Level 2 (-d -d) enables libfetch debugging (quite verbose) so it's
possible to see what pkg is attempting to download without having
to sniff traffic.
Reviewed by: debdrup, bapt
Differential Revision: https://reviews.freebsd.org/D35756
Permit naming pass through devices using the syntax accepted by
pciconf (pci[<domain>:]<bus>:<slot>:<func>) as well as by device name
(e.g. "ppt0").
While here, fix an error in the manpage that had the bus and slot
arguments for the original /-delimited scheme swapped.
Reviewed by: imp, markj
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D36147
This adds missing includes, uses the standard dirent.h rather than the
BSD-specific sys/dirent.h subset (which works on macOS but not Linux)
and works around Linux's lack of st_birthtim.
This allows usr.sbin/makefs to be added to LOCAL_XTOOL_DIRS again on
macOS and Linux so that disk images can be cross-built.
Reviewed by: markj
Fixes: 240afd8c1f ("makefs: Add ZFS support")
Differential Revision: https://reviews.freebsd.org/D36135
Currently the code copies a struct timespec's raw bits as a pair of
uint64_t. On 64-bit systems this has the same representation, but on
32-bit issues there are two issues:
1. tv_sec is a time_t which is 32-bit on i386 specifically
2. tv_nsec is a long not a 64-bit integer
On i386, this means the assertion should fire as the size doesn't match.
On other 32-bit systems there are 4 bytes of padding after tv_nsec,
which in practice are probably 0, as this data is ultimately coming from
the kernel, so it's deterministic (though the padding bytes are not
required to be preserved by the compiler, so are strictly unspecified).
However, on 32-bit big-endian systems, the padding bytes are in the
wrong half to be harmless, resulting in the nanoseconds being multiplied
by 2^32.
Fix this all by marshalling via a real uint64_t pair like is done by the
real ZFS_TIME_ENCODE.
Reviewed by: markj
Fixes: 240afd8c1f ("makefs: Add ZFS support")
Differential Revision: https://reviews.freebsd.org/D36131
The fsnode tree traversal routines used in ZFS mode assume that all
children of a (directory) fsnode can be accessed using a directory fd
for the parent and the child name. This is true when populating the
image using an mtree manifest or from a single staging directory, but
doesn't work when multiple staging directories are specified.
Change the traversal routines to use absolute path lookups when an mtree
manifest is not in use. This isn't ideal, but it's the simplest way to
fix the problem.
Reported by: imp
Sponsored by: The FreeBSD Foundation
Certain operations such as checksum insertion and VLAN insertion
require the device model to rewrite the packet header. The first step
in rewriting the packet header is to copy the existing packet header
from the source packet. This copy is done by copying data from an
iovec array that corresponds to the S/G entries described by transmit
descriptors. However, if the total packet length is smaller than the
headers that need to be copied as the initial template, this copy can
overflow the iovec array and use garbage values as the source pointer
to memcpy. The PR used a single descriptor with a length of 0 in its
PoC.
To fix, track the total packet length and drop requests to transmit
packets whose payload is smaller than the required header length.
While here, fix another issue where the final descriptor could have an
invalid length (too short) that could underflow 'len' when stripping
the checksum. Skip those requests instead, too.
PR: 264372
Reported by: Robert Morris <rtm@lcs.mit.edu>
Reviewed by: grehan, markj
MFC after: 1 week
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D36182
This avoids type confusion where a malicious guest could rewrite the
MaxPStreams field in an endpoint context after the endpoint was
initialized causing the device model to interpret a guest provided
address (stored in ep_ringaddr of the "software" endpoint state) as a
bhyve host process address (ep_sctx_trbs). It also prevents a malicious
guest from triggering overflows of ep_sctx_trbs[] by increasing the
number of streams after the endpoint has been initialized.
Rather than re-reading the MaxPStreams value out of the endpoint context
in guest memory on subsequent operations, cache the value in the software
endpoint state. Possibly the device model should raise errors if the
value of MaxPStreams changes while an endpoint is running. This approach
simply ignores any such changes by the guest.
PR: 264294, 264347
Reported by: Robert Morris <rtm@lcs.mit.edu>
Reviewed by: markj
MFC after: 1 week
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D36181
Controllers must support the Identify Controller list if they support
Namespace Management. But the UNH NVMe tests use this command regardless
of whether the device under test supports Namespace Management.
This implementation returns an empty Controller list (i.e., Number of
Identifiers is zero).
Fixes UNH Test 1.1.2
Reviewed by: jhb
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D36193
The NVMe specification only allows Controllers compliant with the
revision 1.3 and earlier specification to report a value of 0x0 in the
No-Deallocate Modifies Media After Sanitize (NODMMAS) field.
For our revision 1.4 Controller, report that media is not modified after
Sanitize as the implementation does not implement Sanitize.
Fixes UNH Test 1.1.2
Reviewed by: jhb
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D36192
Space used by the MOS is summed after all on-disk data structures are
finalized, so the MOS DSL directory represents a special case when
counting bytes used for each directory. The DSL layer failed to update
the MOS DSL directory's parent, the root directory, accordingly, so the
root directory reported less space used than was used in reality. Be
careful to update the root directory after the MOS is finalized.
PR: 265849
Sponsored by: The FreeBSD Foundation
There was a missing sentence in the description of the second mount
entry in the example. Fix that and some other bits in the EXAMPLES
section.
Also, make the STANDARDS section a bit more readable.
MFC after: 1 week
Sponsored by: Klara Inc.
Summary:
Code was using the mask value without the shift.
Test Plan: Within FreeBSD/Linux guest, Identify NVMe controller to check the result.
Reviewed by: chuck, imp
MFC after: 2 weeks
Signed-off-by: Wanpeng Qian <wanpengqian@gmail.com>
Differential Revision: https://reviews.freebsd.org/D32659
Summary:
Currently Active Firmware Info is not initialized.
Fix is to initialize the Active Firmware Info to Slot 1.
Test Plan: Within FreeBSD/Linux guests, show the Firmware Logpage to confirm.
Reviewed By: chuck
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D32658
Summary:
Set Number of Power States Supported to indicate 1 power state. Keep the
Power State Descriptor data structures as zero to indicate "Not
reported".
Test Plan:
Within FreeBSD/Linux guests, list the number of power states and check
the Max Power value.
Reviewed By: markj, chuck
MFC after: 2 weeks
Signed-off-by: Wanpeng Qian <wanpengqian@gmail.com>
Differential Revision: https://reviews.freebsd.org/D32657
The debug print in nvme_opc_get_log_page() would print an uninitialized
local variable.
In nvme_opc_write_read(), a failed LBA bounds check would cause
pci_nvme_stats_write_read_update() to be called with an uninitialized
variable as a parameter. Although the parameter is unused when the
check fails (and so status != 0), LLVM 14 emits some bogus machine code
in this path, which happens to result in a segfault when it gets
executed.
PR: 265749
Reviewed by: chuck, emaste
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D36119
Currently these are not reported because bhyve is compiled with WARNS=2.
Let's start taking small steps towards enabling more warnings.
No functional change intended.
Reviewed by: chuck, imp, emaste
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D36118
The Dataset Management code could free an uninitialized pointer if the
device doesn't support the Dataset Management command.
PR: 264548
Reported by: Robert Morris <rtm@lcs.mit.edu>
Fuzzing of bhyve uncovered an assertion failure in the NVMe emulation.
Investigation uncovered several corner cases the code did not handle.
This change handles several Controller initialization errors, including
- bad AQ sizes
- bad AQ vm_map_gpa
- doorbell writes prior to RDY
- doorbell writes to uninitialized queue
- CSTS.RDY if CFS set
PR: 256317,256319,256320,256322
Reported by: Cheolwoo Myung <cwmyung@snu.ac.kr>
Reviewed by: jhb
Differential Revision: https://reviews.freebsd.org/D35453
Fuzzing of bhyve using hyfuzz discovered a way to cause a segmentation
fault in the NVMe emulation. If a guest specifies a physical address in
either the PRP1 or PRP2 field of a command that cannot be mapped from
guest to host, the function paddr_guest2host() returns a NULL pointer.
The NVMe emulation did not check for this error case, which allowed for
the segmentation fault to occur.
Fix is to check for a return value of NULL and indicate an error back to
the guest (Data Transfer error). While in the area, slightly refactor
the write/read blockif function to use a common error exit path.
PR: 256321
Reported by: Cheolwoo Myung <cwmyung@snu.ac.kr>
Reviewed by: imp, jhb
Differential Revision: https://reviews.freebsd.org/D35452
Summary:
NVMe operations indicate the memory region(s) associated with a command
via physical region pages (PRPs). Since each PRP has a fixed size,
contiguous memory regions larger than the PRP size require multiple PRP
entries.
Instead of issuing a blockif call for each PRP, the NVMe emulation
concatenates multiple contiguous PRP entries into a single blockif
request. The test for contiguous regions has a bug such that it
mistakenly treats an initial PRP address of zero as a contiguous range
and concatenates it with the previous. But because there is no previous
IOV, the concatenation code corrupts the IO request structure and leads
to a segmentation fault when the blockif request completes.
Fix is to test for the existence of a previous range before trying to
concatenate the current range with the previous one.
While in the area, rename pci_nvme_append_iov_req()'s lba parameter to
offset to match its usage.
PR: 264177
Reported by: Robert Morris <rtm@lcs.mit.edu>
Reviewed by: jhb
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D35328
This is consistent with what other uid-morphing utilities
do, i.e. jexec(1), su(1) etc.
MFC after: 2 weeks
Reviewed by: gbe
Differential Revision: https://reviews.freebsd.org/D36148
Check the canmount property before building the mountpoint string.
Reported by: Coverity
Fixes: 240afd8c1f ("makefs: Add ZFS support")
Sponsored by: The FreeBSD Foundation
nvstring_get() returns a copy of the string, not a pointer into the
nvlist's internal buffer.
Reported by: Coverity
Fixes: 240afd8c1f ("makefs: Add ZFS support")
Sponsored by: The FreeBSD Foundation
This is in preparation for including an etcupdate tree when performing a
-DNO_ROOT release image build. Although -DNO_ROOT can be passed via -M,
to be useful we need to mangle the resulting METALOG to mirror the
various cleanups to the tree that are done after the build (removing
generated files, empty files and empty directories), so etcupdate needs
its own flag.
Reviewed by: jhb, pauamma
Obtained from: CheriBSD
Differential Revision: https://reviews.freebsd.org/D35857
