ZTS: cover change-key on a dataset whose key does not match its root
An incremental raw receive onto a dataset that was rewrapped locally
with 'zfs change-key -i' leaves it pointing at the local encryption root
while carrying the sending root's key material. Until the previous
commit, running 'zfs change-key' on such a dataset, or on its encryption
root, panicked in syncing context.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Michael Heller <75820586+mkhllr at users.noreply.github.com>
Closes #17425
Closes #18969
dsl_crypt: validate every key change-key will rewrap
spa_keystore_change_key_sync_impl() recurses through a dataset and its
children and VERIFY0()s spa_keystore_dsl_key_hold_dd() on each one,
while spa_keystore_change_key_check() only checks that the wrapping key
of the target's encryption root is loaded. A dataset whose DSL Crypto
Key can no longer be unwrapped with that wrapping key therefore turns a
'zfs change-key' into a panic in syncing context, which leaves txg_sync
blocked and the pool unusable until the machine is rebooted:
PANIC at dsl_crypt.c:1475:spa_keystore_change_key_sync_impl()
Walk the same dsl dirs in the check function and hold every DSL Crypto
Key the sync function will rewrap, so an unusable key is reported to
the caller as EACCES instead.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Michael Heller <75820586+mkhllr at users.noreply.github.com>
Closes #17425
Closes #18969
ZTS: bound ctime_001_pos by the time that actually elapsed
For each operation ctime.c reads the timestamp, sleeps two seconds,
runs the operation, reads the timestamp again and requires the
difference to be between 2 and 10 seconds. The upper bound is a
statement about how long the operation may take, not about the
timestamp: when the machine is loaded enough for a creat() to take ten
seconds, the timestamp is updated correctly and the test still fails.
That bound has already been raised once, from 4 to 10 seconds, in
cc210862d ("ZTS: ctime_001_pos increase tolerance") for exactly these
false positives. It is now being hit again at 12 seconds:
st_mtime: BAD time change: t1(1788433492), t2(1788433504)
Take the wall clock before the sleep and after the operation, and
require the new timestamp to fall in that window instead. The
operation cannot have run earlier than two seconds after the first
reading (the sleep) nor later than the second one, so the check no
[11 lines not shown]
Linux: avoid prefaulting under the ZFS range lock
zfs_write() prefaults only the first transaction-sized chunk before
taking the file range lock. A larger write faults each later chunk
while that lock is held. If its source maps the same ZFS file, the
fault enters zfs_getpage() and waits forever for the lock held by the
writing thread.
Keep prefaulting bounded to one transaction-sized chunk and make Linux
iterator copies honor uio_fault_disable. Both the normal DMU path and
the full-block ARC buffer path now copy without page faults while the
range lock is held. If a later source page is not resident, preserve
any completed data and report a Linux short write instead of faulting or
retrying under the lock.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: nexicturbo <turbonexic at gmail.com>
Closes #18135
Closes #18872
log_spacemap: make zfs_log_sm_blksz tunable
Make the block size used for the log space map feature's space maps
settable at runtime.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin at TrueNAS.com>
Signed-off-by: Christos Longros <chris.longros at gmail.com>
Closes #18974
Don't fail a release because a deferred snapshot is still busy
dsl_dataset_user_release_check_one() fails the whole release with
EBUSY when the last hold comes off a snapshot that is marked for
deferred destruction and is still long held. Mark one while it is
idle, with "zfs hold t pool/fs at snap" and then "zfs destroy -d
pool/fs at snap", mount it afterwards by listing .zfs/snapshot, and
"zfs release t pool/fs at snap" reports "dataset is busy" and leaves the
tag where it was. The caller is now sitting on a tag they cannot
drop until the mount goes, and the best-effort unmount the release
path already does cannot take a mount away from an open file.
The check was there because the release is what destroys the
snapshot, and destroying one that something still holds is not on.
Where an owner is what holds it, that is no longer the only way it
gets destroyed: the owner ends at dsl_dataset_disown(), which asks
for the sweep, so the release can drop the tag and leave the mark to
be collected in the ordinary way. Every other long hold keeps
failing the release as before. Nothing would come back for the mark
[20 lines not shown]
Defer destruction of a snapshot that a mount is holding open
On Linux, "zfs destroy -d" on a snapshot automounted under
.zfs/snapshot with a file still open fails with EBUSY, exactly the
way the plain destroy does, and leaves defer_destroy off. The -d
option is documented as marking whatever it cannot destroy right
away, so there is currently no way to say "get rid of it when you
can" about a snapshot someone is reading. Issue #16339.
dsl_destroy_snapshot_check_impl() turns away any long-held snapshot
before it looks at the defer flag, and a snapshot is long held for as
long as it is mounted. Taking the mount away instead is not on the
table: zfsctl_snapshot_unmount() invalidates the snapdir dentry,
which detaches the mount, but the dataset stays owned until the last
open file goes, and the 20ms it then waits is not enough for a file
someone is still reading. FreeBSD never lands here, since its
zfsctl_snapshot_unmount() goes through dounmount() with MS_FORCE.
So let the snapshot be marked, and destroy it once whatever was
[44 lines not shown]
ZTS: cover a meta-dnode range colliding with a redaction entry
The collision needs a redaction entry for an object whose number is an
exact multiple of DNODES_PER_BLOCK, at block zero, while every object
in that dnode block is free in the sending snapshot. The reporter's
script reaches it by chance, because objects are allocated in chunks
which are dnode block aligned, and takes a few attempts.
Build it deliberately instead: create enough files that some aligned
dnode block is filled entirely by them, search for one, redact the
file owning the first object in it, then free the whole block before
taking the sending snapshot. Both orderings of the tie are covered,
one with the redaction list on --redact and one with it reached from
the bookmark on an incremental.
The dataset is created with dnodesize=legacy. The search wants an
aligned run of 32 consecutive object numbers, which holds only while a
dnode occupies one slot; with larger dnodes they are spaced two, four
or eight apart and no such run exists.
[8 lines not shown]
dmu_send: a meta-dnode range does not start where an object does
send_range_start_compare() positions a meta-dnode range, whose blkids
count dnode blocks, at the object its first dnode block covers:
objequiv becomes start_blkid * DNODES_PER_BLOCK and l0equiv is forced
to zero. That places it correctly against the objects it covers, but
it also makes it compare equal to the first block of the object at
that exact boundary, because both sides then have the same objequiv
and an l0equiv of zero. The two do not start at the same place; their
blkids are not even in the same units.
find_next_range() relies on that comparison meaning what it says. The
loop which computes first_change skips DMU_META_DNODE_OBJECT, so a
meta-dnode range never lowers it, while the loop which then advances
every range that starts alongside the one being returned does not skip
it, and applies an object-relative blkid to a range counting dnode
blocks. On a debug build that trips
VERIFY3U(first_change, >, ranges[i]->start_blkid) failed (1 > 4)
[51 lines not shown]
ZTS: drop stale expected-failure masks
The zts-report 'maybe' list suppresses results for tests that are
expected to fail or skip, so a real regression in any of them is
reported as expected and never fails CI. Thirty-eight of those entries
no longer describe anything observable. Each test below was run ten
consecutive times on Linux and passed every time.
Twelve referenced GitHub issues that have since been closed:
cli_root/zfs_get/zfs_get_009_pos #5479
cli_root/zpool_destroy/zpool_destroy_001_pos #6145
cli_root/zpool_import/zpool_import_missing_003_pos #6839
cli_root/zpool_upgrade/zpool_upgrade_004_pos #6141
history/history_004_pos #7026
history/history_006_neg #5657
reservation/reservation_008_pos #7741
reservation/reservation_018_pos #5642
snapshot/snapshot_009_pos #7961
[66 lines not shown]
ZTS: make raidz expansion pause wait actually wait
The three raidz expansion tests pause a reflow by setting
RAIDZ_EXPAND_MAX_REFLOW_BYTES, then call wait_expand_paused() before
snapshotting the vdevs. That helper polled for progress with:
zpool status $TESTPOOL | grep 'copied out of' | awk '{print $1}'
but "copied out of" is the device removal wording printed by
print_removal_status(). A raidz expansion is reported by
print_raidz_expand_status() as:
223M / 320M copied at 2.23M/s, 69.57% done, 00:00:43 to go
The grep therefore never matched. Both variables stayed empty, the
loop's "$oldcopied != $newcopied" test compared "" against "" and
returned after a single sleep, so the helper waited about two seconds
regardless of what the reflow was doing. Both the expansion wording
and this grep arrived together in 5caeef02f ("RAID-Z expansion
[26 lines not shown]
ZTS: fix and re-enable zpool_import_missing_003_pos
The test compared a checksum captured before the pools were exported
against one captured after importing them, but the two were read in
different ways:
read -r checksum1 < <(cksum $MYTESTFILE) # whole line
read -r checksum2 _ < <(cksum $mymtpt/$file) # first field only
With a single variable, read(1) assigns the entire line, so checksum1
held "<sum> <size> <path>" while checksum2 held just "<sum>". The
comparison could therefore never succeed:
ERROR: [ 2652792171 85331 libtest.shlib = 2652792171 ] exited 1
This dates back to 9423c932d ("tests: replace sum(1) with cksum(1)"),
which switched from sum(1) to cksum(1). sum(1) prints two fields and
cksum(1) prints three; the trailing _ was added at the second call
site but not at the first.
[14 lines not shown]
ZTS: fix mkbusy kill/pgrep race in zfs_destroy_001_pos, _005_neg
Both tests killed their accumulated mkbusy pidlist and then immediately
asserted "log_mustnot pgrep -fl mkbusy". That check is racy: kill(2)
only queues the signal, so the target still has to be scheduled to take
it, and it then lingers as a zombie until init reaps it. mkbusy
daemonizes, so the test shell is not its parent and cannot wait(2) for
it -- and pgrep lists the process for that whole window, zombie
included.
Add kill_mkbusy() to zfs_destroy_common.kshlib: kill the pidlist, poll
kill -0 for each pid until it is gone (up to 5s per pid), and only then
run the "no mkbusy anywhere" leak check the tests already had. Scoping
the wait to the pids we killed keeps the leak check meaningful -- a
stale mkbusy from elsewhere is still reported, rather than silently
absorbed by a global wait loop -- and names the offending pid when a
kill really does fail to take.
Convert all 5 call sites in the two tests. The leak check now runs
[10 lines not shown]
dnode: fix heap-use-after-free in dnode_rele_and_unlock()
The ZFS_DEBUG-only assert in dnode_rele_and_unlock() dereferenced dnh,
the dnode's handle, after mutex_exit(&dn->dn_mtx), below the comment
stating "dnode could get destroyed at this point, so don't use it
anymore".
For an objset's special dnodes that is a real use-after-free. They have
no dnode block (dn_dbuf == NULL), so nothing pins their handle; it is
embedded in the objset_t. Only dn_nodnholds orders a release against
teardown, and this function is what signals it: on the last hold it
broadcasts and drops dn_mtx, freeing dnode_special_close() to destroy
that handle and, via dmu_objset_evict_done(), the objset it lives in.
The assert then reads it.
Read the zrlock ownership before dropping dn_mtx instead, where dn_mtx
still blocks dnode_special_close() and, for an ordinary dnode, the
reference from dbuf_add_ref(db, dnh) is not yet released.
[5 lines not shown]
Linux: drop cached pages in zfs_rezget()
After zfs rollback (or zfs recv -F) of a mounted dataset,
zfs_resume_fs() calls zfs_rezget() for every surviving znode. On Linux
it reloads the SA attributes but never touches the inode's page cache.
A file whose object number, generation and size are unchanged but whose
data blocks differ keeps serving pre-rollback data from pages that were
resident (the file had been mmap()ed) and later refreshed by
update_pages() from a write(). Both mappedread() and mmap() return the
stale pages; reading through .zfs/snapshot is correct, so the on-disk
data is fine.
The FreeBSD port handles this by calling vn_pages_remove_valid() at the
start of zfs_rezget(). Do the equivalent on Linux with
truncate_inode_pages() before reloading the znode, so pages are dropped
on the error paths as well. Dirty pages are discarded on purpose: their
content belongs to the state being rolled back, and writeback is blocked
on z_teardown_lock during suspend/resume anyway. Live mappings are
unmapped and fault the correct data back in through zfs_getpage().
[4 lines not shown]
unit/btree: test the negative cases
Introduce the two negative tests, insert_duplicate and remove_missing,
from tests/zfs-tests/cmd/btree_test.c to the unit tests.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Christos Longros <chris.longros at gmail.com>
Closes #19009
arc: apply arc_min tunables set before arc_init() has run
Tunables are applied as each parameter is registered, and zfs_arc_min is
registered before zfs_arc_max. So param_set_arc_min() can run while
arc_c_max is still zero, and its upper bound check rejects every
non-zero value:
Setting sysctl vfs.zfs.arc.min failed: 22
The value is dropped and the ARC floor falls back to allmem/32.
param_set_arc_max() is not affected, because it tests a lower bound
against arc_c_min, which a zero value satisfies.
Skip the upper bound while arc_c_max is zero and store the raw value.
arc_init() expects this: arc_set_limits() sets both limits, and the
arc_tuning_update() call after it applies zfs_arc_min.
Storing the value exposes a second bug. The other arc handlers call
arc_tuning_update() unconditionally, so one registered before arc_init()
[14 lines not shown]
Write uberblocks only to vdevs used by the txg
Instead of picking random top-level vdevs to write the uberblock to,
prefer the ones actually written during this txg. This allows idle
vdevs to stay asleep, which is important for pools with spun-down
HDDs.
If fewer than SPA_SYNC_MIN_VDEVS were written, top up from special
and dedup vdevs, which are expected to have no seek penalty. Pools
without those classes keep the old behavior of topping up from any
vdev, preserving the uberblock redundancy where there is nothing to
gain by reducing it.
While here, pass spa explicitly to vdev_config_sync() and
vdev_uberblock_sync_list() instead of deriving it from svd[0].
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Alexander Motin <alexander.motin at TrueNAS.com>
Closes #19003
linux: fix write cache detection on 6.11+ kernels
Linux 6.11 moved the write cache flag into queue_limits, removing both
QUEUE_FLAG_WC and QUEUE_FLAG_HW_WC. zfs_bdev_has_write_cache() only
tested those two flags, so it fell through to the pre-4.10 fallback and
assumed every vdev had a volatile write cache. vdev_nowritecache was
never set at open, and ZFS kept submitting flush BIOs to devices that
report no cache, only for the kernel to discard them in
blk_insert_flush().
Test queue_limits.features & BLK_FEAT_WRITE_CACHE instead. Read the
field directly rather than using bdev_write_cache(), which also honours
the operator's BLK_FLAG_WRITE_CACHE_DISABLED override: we only sample
this at vdev open, so if the operator later re-enabled the cache we
would never flush again. This preserves the old QUEUE_FLAG_HW_WC
semantics.
Reviewed-by: Tony Hutter <hutter2 at llnl.gov>
Reviewed-by: Rob Norris <rob.norris at truenas.com>
Signed-off-by: Alexander Motin <alexander.motin at TrueNAS.com>
Closes #19001
Speed up pool rewind and allow it to tolerate minor damage
Importing a damaged pool with -F/-X could take days: every rewind
attempt scanned the whole pool, and it kept scanning long after it hit
an error that already disqualified the txg. Worse, the scan is all or
nothing -- a single unreadable block of file meta-data rejects a txg
even when the pool itself is perfectly consistent, forcing the admin to
rewind much further back and lose unrelated data for no reason.
Abort the verification scan as soon as a txg is known to be unusable,
and skip the data scan entirely for attempts that are only looking for
a rewind target. Then split the errors found into those that really
prevent the pool from working and those that only cost the contents of
some files, and add a new "zpool import -M" asking to accept the
latter.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Alexander Motin <alexander.motin at TrueNAS.com>
Closes #18967
Undo checkpoint accounting on vdev_free()
vdev_load() increments spa_checkpoint_info.sci_dspace, and if we
don't undo it on unload, we'll see a leak on following load in case
of rewind import.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Alexander Motin <alexander.motin at TrueNAS.com>
Closes #18967
ztest: Add random seed support
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Igor Ostapenko <igor.ostapenko at klarasystems.com>
Closes #18796
arc: harness buf_hdr's anon state invariant checks
There are several places where hdr is checked for arc_anon state
without the hash_lock, for example at arc_release() or at
arc_buf_destroy(), and it's supposed that the hdr must not be
in the hash table in this state and not having IO_IN_PROGRESS.
However, those invariants are not asserted in release builds.
But even if they would, there is one place in the current code
where they would pass without noticing the problem.
This place is at arc_write_done() where we first insert hdr
into the hash table, then clear IO_IN_PROGRESS flag, then drop
the reference and only then move its state from anon to mru:
7048 exists = buf_hash_insert(hdr, &hash_lock); /* still anon */
...
7081 arc_hdr_clear_flags(hdr, ARC_FLAG_IO_IN_PROGRESS);
7082 VERIFY3S(remove_reference(hdr, hdr), >, 0); /* ref: 2 -> 1 */
7084 if (exists == NULL && hdr->b_l1hdr.b_state == arc_anon)
[34 lines not shown]
zstd: declare __asan_*_memory_region() for user space builds
ZSTD_ASAN_POISON() and ZSTD_ASAN_UNPOISON() call
__asan_poison_memory_region() and __asan_unpoison_memory_region()
directly, but nothing declares them in the user space build. The kernel
block just above declares both -- and stubs them out, since KASAN does
not provide them -- while the user space block was left with only the
macros.
That builds where an implicit function declaration is a warning, since
the real symbols come from libasan at link time, but fails on compilers
that make it an error, as GCC 14 and newer do by default:
zfs_zstd.c:278:34: error: implicit declaration of function
'__asan_poison_memory_region' [-Wimplicit-function-declaration]
Declare both in the user space block, mirroring the kernel block.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Alexander Moch <mail at alexmoch.com>
Closes #19017
unit: add nvpair test suite
Exercises add and lookup for all types, iteration, removal, replacement,
merge & copy, pack & unpack with both encodings, formatting and
infallibile wrappers.
Sponsored-by: TrueNAS
Reviewed-by: Chris Longros <chris.longros at gmail.com>
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #19006
unit: add null/notnull helpers
Sponsored-by: TrueNAS
Reviewed-by: Chris Longros <chris.longros at gmail.com>
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #19006
unit: make unit_eq() etc type-aware
Previously, unit_eq() and friends were naively routed to
munit_assert_uint64(), regardless of the actual types involved. This
meant that on failure, the error message would report eg a small
negative number as a huge unsigned one, and also plain doesn't work for
types that don't have an implicit conversion to uint64_t, like double.
Here we add a type-aware replacement that generates comparison and
failure reporting functions for the wanted types, then uses _Generic to
dispatch to the correct one.
Sponsored-by: TrueNAS
Reviewed-by: Chris Longros <chris.longros at gmail.com>
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #19006
Inherit the project ID for every object type
zfs_mknode() only assigned a project ID to regular files and
directories, so a symlink, device node, FIFO or socket created inside a
directory with ZFS_PROJINHERIT set was left at ZFS_DEFAULT_PROJID. The
cross-project checks in zfs_rename() and zfs_link() compare the object's
project ID against the directory's, so such an object is treated as
foreign to the very directory holding it, and cannot be renamed or
linked there at all -- "ln -sfn", which creates the new symlink under a
temporary name and renames it into place, fails with EXDEV.
ext4 and XFS store a project ID on every inode type and so do not have
this problem. Do the same, and quota-check new symlinks and rename
whiteouts against the inherited ID rather than the default one, so their
space is accounted to the project that owns them.
This changes accounting for newly created objects: symlinks, device
nodes and FIFOs now consume the project's quota where they previously
consumed none, so on a dataset already at its project quota, creating
[12 lines not shown]
Allow renames within a single directory under project inheritance
A rename that keeps the object in the directory it already lives in
cannot move it between projects, so refusing it with EXDEV is never
right. Objects created before the previous commit carry no project ID of
their own, which makes this reachable on existing pools: a symlink in a
project directory cannot be renamed even to another name beside itself,
and "ln -sfn" over an existing path fails.
zfs_link() keeps its unconditional check. It has no source directory to
compare against -- a hard link names an object that may live anywhere --
so there is no equivalent "the object is already here" case to exempt.
Linking a pre-existing symlink, device node or FIFO into the project
directory that already holds it therefore still fails with EXDEV, until
the object is given a project ID of its own.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Matt Turner <mattst88 at gmail.com>
Closes #18932
ZTS: check project ID inheritance and renames within a project directory
Covers both of the preceding changes. For the inheritance: a new symlink
and a new FIFO each add an object to the containing directory's project,
which only happens once non-regular files inherit a project ID, and a
symlink can then be renamed into a different directory carrying the same
project ID, which rename(2) permits only when the two project IDs match.
For the rename exemption: a regular file and a symlink are created
before the directory is tagged, so they carry no project ID of their
own, as every symlink on an existing pool does. Renaming each of them
within that directory afterwards fails with EXDEV unless the
cross-project check exempts renames whose source and target directories
are the same. The same renames are then repeated for objects created
after the tagging, along with replacing a symlink with "ln -sfn", and
again in an inheriting subdirectory.
A rename that does cross into a different project is still refused.
[7 lines not shown]
