Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge FreeBSD 2024-10-25 #2305

Merged
merged 433 commits into from
Jan 29, 2025
Merged

Merge FreeBSD 2024-10-25 #2305

merged 433 commits into from
Jan 29, 2025

Conversation

bsdjhb
Copy link
Collaborator

@bsdjhb bsdjhb commented Jan 28, 2025

PR for CI

bsdjhb and others added 30 commits October 24, 2024 10:18
The previous fix for a stack buffer leak in the ahci device model
actually broke the handling of TRIM as one of the checks it added
caused TRIM commands to never be completed.  This resulted in command
timeouts if a guest OS did a 'newfs -E' of an AHCI disk, for example.
Also, for the invalid case the previous check was handling, the device
model should be failing with an error rather than claiming success.

To resolve this, validate the length of a TRIM request and fail with
an error if it exceeds the maximum number of supported blocks
advertised via IDENTIFY.  In addition, if the PRDT does not provide
enough data, fail the command with an error rather than performing a
partial completion.

This is somewhat complicated by the implementation of TRIM in the ahci
device model.  A single TRIM request can specify multiple LBA ranges.
The device model handles this by dispatching blockif_delete() requests
one at a time.  When a blockif_delete() request completes, the device
model locates the TRIM buffer and searches for the next LBA range to
handle.  Previously, the device model would re-read the trim buffer
from guest memory each time.  However, this was subject to some
unpleasant races if the guest changed the PRDT entries or CFIS while a
command was in flight.  Instead, read the buffer of trim ranges once
and cache it across multipe internal blockif requests.

Reviewed by:	mav
Fixes:		71fa171 bhyve: Initialize stack buffer in pci_ahci
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D47224
Now that we export the relevant stats via the net.route.stats sysctl,
switch to using that to avoid having to dig around in mem(4) for live
kernel statistics.  Based on callers of kresolve_list(), this is the
last live path using mem(4) that could be functional today.

Tested both with `netstat -rs` and `netstat -rs -M`.

Note that this will not be able to extract stats from a running kernel
that predates 3360a15 / 1500026, but this can be worked around by
specifying `-M /dev/mem` explicitly in the interim to fallback to
libkvm against /dev/mem.

Reviewed by:	glebius, markj, zlei
Differential Revision:	https://reviews.freebsd.org/D47231
Implement for mutex(9) and rwlock(9).

Reviewed by:		jtl
Differential Revision:	https://reviews.freebsd.org/D45745
If a callout was initialized with the flag, then the callout(9) system
will not drop the callwheel lock in softclock_call_cc() to obtain the
callout lock.  Instead it will use try-lock semantic to obtain the
callout's lock.  In case of a failure the callout will be rescheduled to
the 50% of the precision value.  The main benefit of such behavior is not
the avoidance of the lock contention in the callout thread, but the fact
that callout with such flag can be actually stopped in a safe manner,
because the race window in the beginning of softclock_call_cc() is closed.

Call of callout_stop() on such a callout would guarantee that nothing will
be executed after callout_stop() returns, neither callout lock will be
dereferenced.  A callout marked as CALLOUT_TRYLOCK |
CALLOUT_RETURNUNLOCKED can call callout_stop() from the callout function
itself (0, a failure to stop, will be returned), then unlock the lock and
then free the memory containing the callout structure.

Caveat: when calling callout_stop() from outside the callout function, the
return value from callout_stop() is still inconsistent.  A race window at
the end of softclock_call_cc() still exists, so callout_stop() may report
failure to stop, which would not be true.

Reviewed by:            jtl, kib
Differential Revision:	https://reviews.freebsd.org/D45746
This allows to remove the drop of the lock tcp_timer_enter(), which closes
a sophisticated but possible race that involves three threads.  In case we
got a callout executing and two threads trying to close the connection,
e.g. and interrupt and a syscall, then lock yielding in tcp_timer_enter()
may transfer lock from one closing thread to the other closing thread,
instead of the callout.

Reviewed by:		jtl
Differential Revision:	https://reviews.freebsd.org/D45747
With CALLOUT_TRYLOCK we don't need this special flag.

Reviewed by:		jtl
Differential Revision:	https://reviews.freebsd.org/D45748
Two functions in tmpfs_vnops.c use an interface provided by
swap_pager.c. Move most of the implementation of those functions to
swap_pager.c so that they can be implemented more effectively, with
access to implementation details of the swap pager.

Reviewed by:	kib
Differential Revision:	https://reviews.freebsd.org/D47212
It turns out the new libc++ 19 headers result in a -Werror warning from
gcc 13:

  In file included from /usr/obj/usr/src/freebsd/src/amd64.amd64/tmp/usr/include/c++/v1/__memory/shared_ptr.h:31:
  /usr/obj/usr/src/freebsd/src/amd64.amd64/tmp/usr/include/c++/v1/__memory/uninitialized_algorithms.h: In instantiation of 'constexpr void std::__1::__uninitialized_allocator_relocate(_Alloc&, _Tp*, _Tp*, _Tp*) [with _Alloc = allocator<basic_string<char> >; _Tp = basic_string<char>]':
  /usr/obj/usr/src/freebsd/src/amd64.amd64/tmp/usr/include/c++/v1/vector:1052:42:   required from 'void std::__1::vector<_Tp, _Alloc>::__swap_out_circular_buffer(std::__1::__split_buffer<_Tp, _Allocator&>&) [with _Tp = std::__1::basic_string<char>; _Allocator = std::__1::allocator<std::__1::basic_string<char> >]'
  /usr/obj/usr/src/freebsd/src/amd64.amd64/tmp/usr/include/c++/v1/vector:1469:31:   required from 'void std::__1::vector<_Tp, _Alloc>::reserve(size_type) [with _Tp = std::__1::basic_string<char>; _Allocator = std::__1::allocator<std::__1::basic_string<char> >; size_type = long unsigned int]'
  /usr/src/freebsd/src/contrib/googletest/googletest/src/gtest.cc:795:27:   required from here
  /usr/obj/usr/src/freebsd/src/amd64.amd64/tmp/usr/include/c++/v1/__memory/uninitialized_algorithms.h:645:21: error: 'void* __builtin_memcpy(void*, const void*, long unsigned int)' writing to an object of type 'std::__1::__remove_const_t<std::__1::basic_string<char> >' {aka 'class std::__1::basic_string<char>'} with no trivial copy-assignment; use copy-assignment or copy-initialization instead o[-Werror=class-memaccess]
    645 |     __builtin_memcpy(const_cast<__remove_const_t<_Tp>*>(__result), __first, sizeof(_Tp) * (__last - __first));
        |     ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  In file included from /usr/obj/usr/src/freebsd/src/amd64.amd64/tmp/usr/include/c++/v1/__system_error/error_category.h:15,
                   from /usr/obj/usr/src/freebsd/src/amd64.amd64/tmp/usr/include/c++/v1/__system_error/error_code.h:18,
                   from /usr/obj/usr/src/freebsd/src/amd64.amd64/tmp/usr/include/c++/v1/__ostream/basic_ostream.h:16:
  /usr/obj/usr/src/freebsd/src/amd64.amd64/tmp/usr/include/c++/v1/string:752:7: note: 'std::__1::__remove_const_t<std::__1::basic_string<char> >' {aka 'class std::__1::basic_string<char>'} declared here
    752 | class basic_string {
        |       ^~~~~~~~~~~~

Since this is all benign, turn off errors for -Wclass-memaccess.

PR:		280562
MFC after:	3 days
An assertion that an object was write-locked should be instead an
assertion that the object is read locked.

Reported by:	Jenkins
Fixes:	 db08b0b tmpfs_vnops: move swap work to swap_pager
Differential Revision:	https://reviews.freebsd.org/D47278
Moving code from tmpfs to swap_pager introduced another WLOCKED object
assert that should have been an RLOCKED object assert.  Fix it.
After talking with a number of people about the removal of some things
to make the loader fit, readjust things a little.

Add back GZIP and BZIP2 compression support. Many of the downstream MFC
packaging systems depend on this. This adds back 20k to the size of the
loader.

Make the boot loader text-only by default. This saves 40k in size. Net,
we're 20k smaller. The graphics loader for BIOS is less useful than the
zip functionality: You can still boot w/a text only one it and you can
build a custom one if you really want it. It's also the default we use
for dual console.

This should be merged back into stable/14 and stable/13 so it's in the
next release for each of these. That way we have only one release (13.4)
with the other defaults.

MFC After:		3 days
Sponsored by:		Netflix
Reviewed by:		olce, rgrimes, emaste
Differential Revision:	https://reviews.freebsd.org/D47203
These are internal to the loader and generally can only be set in a
makefile that's compiling some variation of loader. Add caveats since
these aren't really user-serviceable parts, though some downstreams will
tweak individual makefiles for their own purposes.

Sponsored by:		Netflix
Reviewed by:	emaste, trasz
MFC after:	3 days
Differential Revision: https://reviews.freebsd.org/D47262
When adding a new vchan, we are looking for a parent channel which
either already has vchans (i.e CHN_F_HAS_VCHAN), or does not, but is
also not being used (i.e !CHN_F_BUSY). Since CHN_F_BUSY essentially
tells us if the channel is currently being used or not, there is no need
to check if the channel's refcount is 0 as well.

When removing a vchan, we first check if we have only 1 vchan allocated
that is also being used (so we cannot remove it at the moment), and then
we check if the vchan is not busy and remove it. Again, checking
CHN_F_BUSY is enough.

Sponsored by:	The FreeBSD Foundation
MFC after:	2 days
Reviewed by:	dev_submerge.ch
Differential Revision:	https://reviews.freebsd.org/D47268
No longer used.

Sponsored by:	The FreeBSD Foundation
MFC after:	2 days
Reviewed by:	dev_submerge.ch
Differential Revision:	https://reviews.freebsd.org/D47269
MFC after:	3 days
No functional change intended.

MFC after: 	3 days
Avoid an ArcanistUsageException:

    Provide method parameters on stdin as a JSON blob.

Due to an invalid JSON input on strict POSIX-compliant shells (macOS):

    echo '{
            "constraints": {"phids": [-n,"PHID-USER-xxx",]}
          }'                          ^^^               ^

Change the non-portable "echo -n" to a printf.

Reviewed by:	0mp, imp, markj
Approved by:	markj (mentor)
MFC after:	1 week
Differential Revision:	https://reviews.freebsd.org/D46781
Store the json blobs in temporary files instead of variables, and feed
them to jq for parsing.

When echoing "\n", the new line will become a literal new line, and
therefore, invalid json input:

```
jq: parse error: Invalid string: control characters from U+0000 through U+001F must be escaped ...
```

To reproduce:

    % git arc patch -c D12345

Reviewed by:	0mp, imp, markj
Approved by:	markj (mentor)
MFC after:	1 week
Differential Revision:	https://reviews.freebsd.org/D46767
clausecker and others added 27 commits January 28, 2025 17:00
Prevent a potentially sufficiently smart compiler from optimising
away our attempts to clear sensitive buffers.

A related change was discussed and rejected in D16059, but I don't
believe the reasoning there applies: the code clearly documents its
intent that the `memset` calls clear sensitive buffers so they don't
hang around.  `explicit_bzero` is the appropriate function for this
purpose.  A potential performance disadvantage seems less important:
the functions in crypt are specifically designed to be slow, so a
few extra calls to guarantee that sensitive buffers are cleared does
not significantly affect runtime.

See also:	D16059
Reviewed by:	delphij, kevans
MFC after:	1 week
Differential Revision:	https://reviews.freebsd.org/D47037
libcrypt bundles the various hash functions it needs,
duplicating code that is also found in libmd.
Unbundle the hash functions and apply the same hack used
for libncursesw so static consumers link -lmd in addition
to -lcrypt.

Reviewed by:	kevans
Differential Revision:	https://reviews.freebsd.org/D47062
MFC after:	3 days
No functional change intended.

MFC after: 	3 days
- use raw image disk type and enable zfs, this yields smaller
  images for upload after using native qcow2 + zstd compression

Reviewed by:	lwhsu, emaste
Differential Revision:	https://reviews.freebsd.org/D47055
MFC after:	3 days
Approved by:	emaste
Unfortunately gcc 12's is not yet capable of compiling all of libc++
19's C++23 code, which results in errors similar to:

  /usr/src/freebsd/src/contrib/llvm-project/libcxx/include/__algorithm/ranges_contains.h:41:3: error: 'static constexpr bool std::__1::ranges::__contains::__fn::operator()(_Iter, _Sent, const _Type&, _Proj)' must be a non-static member function
     41 |   operator()(_Iter __first, _Sent __last, const _Type& __value, _Proj __proj = {}) {
        |   ^~~~~~~~
  /usr/src/freebsd/src/contrib/llvm-project/libcxx/include/__algorithm/ranges_contains.h:48:3: error: 'static constexpr bool std::__1::ranges::__contains::__fn::operator()(_Range&&, const _Type&, _Proj)' must be a non-static member function
     48 |   operator()(_Range&& __range, const _Type& __value, _Proj __proj = {}) {
        |   ^~~~~~~~

Until we can get rid of gcc 12, work around this by making it compile
libc++ in C++20 mode instead.

NOTE: The resulting libc++ library will not be C++23 compatible! Please
try to avoid shipping it, and use gcc 13 instead, if you must use gcc.

PR:		280562
MFC after:	3 days
With gcc we are seeing the following -Werror warning:

    In file included from /workspace/src/sys/contrib/openzfs/include/os/freebsd/spl/sys/sunddi.h:28,
                     from /workspace/src/sys/contrib/openzfs/include/sys/zfs_context.h:69:
    In function 'zfs_uio_init',
        inlined from 'zio_do_crypt_data' at /workspace/src/sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c:1690:2:
    /workspace/src/sys/contrib/openzfs/include/os/freebsd/spl/sys/uio.h:102:45: error: 'puio_s.uio_offset' is used uninitialized [-Werror=uninitialized]
      102 |                 zfs_uio_soffset(uio) = uio_s->uio_offset;
          |                                        ~~~~~^~~~~~~~~~~~
    /workspace/src/sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c: In function 'zio_do_crypt_data':
    /workspace/src/sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c:1683:20: note: 'puio_s' declared here
     1683 |         struct uio puio_s, cuio_s;
          |                    ^~~~~~
    In function 'zfs_uio_init',
        inlined from 'zio_do_crypt_data' at /workspace/src/sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c:1691:2:
    /workspace/src/sys/contrib/openzfs/include/os/freebsd/spl/sys/uio.h:102:45: error: 'cuio_s.uio_offset' is used uninitialized [-Werror=uninitialized]
      102 |                 zfs_uio_soffset(uio) = uio_s->uio_offset;
          |                                        ~~~~~^~~~~~~~~~~~
    /workspace/src/sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c: In function 'zio_do_crypt_data':
    /workspace/src/sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c:1683:28: note: 'cuio_s' declared here
     1683 |         struct uio puio_s, cuio_s;
          |                            ^~~~~~

Indeed, `zfs_uio_init()` does:

    static __inline void
    zfs_uio_init(zfs_uio_t *uio, struct uio *uio_s)
    {
            memset(uio, 0, sizeof (zfs_uio_t));
            if (uio_s != NULL) {
                    GET_UIO_STRUCT(uio) = uio_s;
                    zfs_uio_soffset(uio) = uio_s->uio_offset;
            }
    }

while the code in `zio_crypt.c` has:

    /*
     * Primary encryption / decryption entrypoint for zio data.
     */
    int
    zio_do_crypt_data(boolean_t encrypt, zio_crypt_key_t *key,
        dmu_object_type_t ot, boolean_t byteswap, uint8_t *salt, uint8_t *iv,
        uint8_t *mac, uint_t datalen, uint8_t *plainbuf, uint8_t *cipherbuf,
        boolean_t *no_crypt)
    {
            int ret;
            boolean_t locked = B_FALSE;
            uint64_t crypt = key->zk_crypt;
            uint_t keydata_len = zio_crypt_table[crypt].ci_keylen;
            uint_t enc_len, auth_len;
            zfs_uio_t puio, cuio;
            struct uio puio_s, cuio_s;
            uint8_t enc_keydata[MASTER_KEY_MAX_LEN];
            crypto_key_t tmp_ckey, *ckey = NULL;
            freebsd_crypt_session_t *tmpl = NULL;
            uint8_t *authbuf = NULL;

            zfs_uio_init(&puio, &puio_s);
            zfs_uio_init(&cuio, &cuio_s);
            memset(GET_UIO_STRUCT(&puio), 0, sizeof (struct uio));
            memset(GET_UIO_STRUCT(&cuio), 0, sizeof (struct uio));

So between the declaration of `puio_s` and `cuio_s`, there is no
initialization of these variables before `zfs_uio_init()` gets called.

Similar to the Linux variant of zio_crypt.c, I think it would be better
to memset the structs `puio_s` and `cuis_s` _before_ calling
`zfs_uio_init()`.

Reviewed by:    tsoome
MFC after:      3 days
Differential Revision: https://reviews.freebsd.org/D47281
This gives the function the ability to return only global symbols.

Sponsored by:	Juniper Networks, Inc.
Reviewed by:	markj
Differential Revision:	https://reviews.freebsd.org/D47206
In PCTRIE_INSERT_LOOKUP_{G,L}E, there is a test - if two pointers are
equal, replace one with a new value. The pointers can never be equal;
one points to a struct pctrie_node and the other is the (void*) cast
of a pointer to a field within a struct pctrie_node. So the tests and
assignments can be removed with no effect.

Reviewed by:	bnovkov
Differential Revision:	https://reviews.freebsd.org/D47277
No functional change intended.

Sponsored by:	Klara, Inc.
- Remove superfluous newlines.
- Use bool literals.
- Replace an unneeded SYSINIT with static initialization.

No functional change intended.

Sponsored by:	Klara, Inc.
If, when submitting a request, the virtqueue is full, we sleep until an
interrupt has fired, then restart the request.  However, while sleeping
the channel lock is dropped, and in the meantime another thread may have
reset the per-channel SG list, so upon retrying we'd (re)submit whatever
happened to be left over in the previous request.

Fix the problem by rebuilding the SG list after sleeping.

Sponsored by:	Klara, Inc.
Otherwise we can end up with a lost interrupt, causing lost request
completion wakeups and hangs in the filesystem layer.

Continue processing until we enable interrupts and then observe an empty
queue, like other virtio drivers do.

Sponsored by:	Klara, Inc.
Remove an always-false check for whether the request has already
completed before sleeping.  Even if the request is complete, the
response tag is updated while holding the channel lock, which is also
held here.

No functional change intended.

Sponsored by:	Klara, Inc.
The hash function removal changed the mangled names of MD[45] and
SHA{256,512} hash function identifiers.  Clean these files to pick
up the symbol change.

Reported by:	bapt
Tested by:	brooks
Fixes:		cb5e41b
This reverts commit 536c8d9.  The
change seemed fine on the surface, but converting to an enum has raised
some concerns due to the asm <-> C interface.  Back it out and let
someone else deal with it later if they'd like to.

Further context about the concerns can be found in D47279.
pctrie_toval(node) can be applied to either a leaf or an internal
node; in the latter case it provides the address of the pn_owner
field. In a couple of places where a neighbor search is about to begin
for an iterator, the current code distinguishes the leaf and non-leaf
cases in a way that isn't really necessary. This change shrinks each
function by 16 bytes, and by a branch instruction.

Reviewed by:	bnovkov
Differential Revision:	https://reviews.freebsd.org/D47207
AWS Graviton [1234] systems have a bug in their ACPI where they mark
the PL061's GPIO pins as needing to be configured in PullUp mode (in
fact the PL061 has no pullup/pulldown resistors); this flag needs to
be removed in order for _AEI objects to be handled on these systems.

Reviewed by:	Ali Saidi
MFC after:	1 week
Sponsored by:	Amazon
Differential Revision:	https://reviews.freebsd.org/D47239
This allows acpi_gpiobus to override the method and fall back to the
generic gpiobus_read_ivar function if needed.

Reviewed by:	andrew
MFC after:	1 week
Sponsored by:	Amazon
Differential Revision:	https://reviews.freebsd.org/D47250
GPIO interrupts work just fine and will be used shortly.  We still
do not support GPIO_INTR_SHAREABLE however, so leave that within
the NOT_YET scope.

Reviwed by:	andrew
MFC after:	1 week
Sponsored by:	Amazon
Differential Revision:	https://reviews.freebsd.org/D47251
The purpose of this script is to detect absolute symlinks on
a machine, e.g.:

    /etc/localtime -> /usr/share/zoneinfo/UTC

Some of these absolute symbolic links can be created intentionally,
but it is usually better to use relative symlinks.

You can run the script after `make installworld', or any other
make targets thats installs files.

You can also check your local ports with:

   env ABSOLUTE_SYMLINK_DIRS=/usr/local ./absolute-symlink.sh
PR:		282192
Reported by:	wosch
Fixes:		44877c8
Sponsored by:	The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47226
via is not an abbreviation and doesn't need a period.
struct kld_file_stat embeds a reference to MAXPATHLEN, defined in
param.h.

PR:		280432
MFC after:	2 weeks
Use pctrie iterators for swblk traversal in more swap_pager
functions: swap_pager_haspage, swp_pager_meta_lookup, and
swap_pager_getpages.

Reported by:	markj
Reviewed by:	kib
Differential Revision:	https://reviews.freebsd.org/D47232
This is apparently needed for the cross-build from Linux to succeed.

Fixes:		cb5e41b
Fix the build on macOS

The bootstrap tools are statically linked, so our build system
will provide the full dependency chain which pulls in libcrypt.

since recently libcrypt.a is a linker script not supported by macOS
ar(1), given that compile_et does not use at all the function from
libroken which brings the dependency on libcrypt, bypass LIBADD
dependency chain by using the old LDADD/DPADD mechanism, like it is
done for other kerberos related bootstrap tools.

(cherry picked from commit 4d692868a6730ebf72a1d1a619af89af6fc8d763)
@bsdjhb bsdjhb force-pushed the merge-freebsd-20241025 branch from ce5e267 to 2f27910 Compare January 29, 2025 14:52
@bsdjhb bsdjhb merged commit 2f27910 into CTSRD-CHERI:dev Jan 29, 2025
27 of 29 checks passed
@bsdjhb bsdjhb deleted the merge-freebsd-20241025 branch January 29, 2025 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.