From 72a82f312f9fd0abd5b15f1d847e448a56353ec7 Mon Sep 17 00:00:00 2001 From: George Amanakis Date: Thu, 3 Feb 2022 23:28:19 +0100 Subject: [PATCH] Report dnodes with faulty bonuslen In files created/modified before 4254acb there may be a corruption of xattrs which is not reported during scrub and normal send/receive. It manifests only as an error when raw sending/receiving. This happens because currently only the raw receive path checks for discrepancies between the dnode bonus length and the spill pointer flag. In case we encounter a dnode whose bonus length is greater than the predicted one, we should report an error. Modify in this regard dnode_sync() with an assertion at the end, dump_dnode() to error out, dsl_scan_recurse() to report errors during a scrub, and zstream to report a warning when dumping. Also added a test to verify spill blocks are sent correctly in a raw send. Reviewed-by: Brian Behlendorf Signed-off-by: George Amanakis Closes #12720 Closes #13014 --- cmd/zstream/zstream_dump.c | 8 + module/zfs/dmu_send.c | 2 + module/zfs/dnode_sync.c | 2 + module/zfs/dsl_scan.c | 13 ++ tests/runfiles/common.run | 3 +- .../tests/functional/rsend/Makefile.am | 1 + .../functional/rsend/send_raw_spill_block.ksh | 161 ++++++++++++++++++ 7 files changed, 189 insertions(+), 1 deletion(-) create mode 100755 tests/zfs-tests/tests/functional/rsend/send_raw_spill_block.ksh diff --git a/cmd/zstream/zstream_dump.c b/cmd/zstream/zstream_dump.c index 45cf7b97a1..af69a1494f 100644 --- a/cmd/zstream/zstream_dump.c +++ b/cmd/zstream/zstream_dump.c @@ -461,6 +461,14 @@ zstream_do_dump(int argc, char *argv[]) BSWAP_64(drro->drr_maxblkid); } + if (drro->drr_bonuslen > drro->drr_raw_bonuslen) { + (void) fprintf(stderr, + "Warning: Object %llu has bonuslen = " + "%u > raw_bonuslen = %u\n\n", + (u_longlong_t)drro->drr_object, + drro->drr_bonuslen, drro->drr_raw_bonuslen); + } + payload_size = DRR_OBJECT_PAYLOAD_SIZE(drro); if (verbose) { diff --git a/module/zfs/dmu_send.c b/module/zfs/dmu_send.c index 2f2fd4c3d6..58a72a68a6 100644 --- a/module/zfs/dmu_send.c +++ b/module/zfs/dmu_send.c @@ -763,6 +763,8 @@ dump_dnode(dmu_send_cookie_t *dscp, const blkptr_t *bp, uint64_t object, * to send it. */ if (bonuslen != 0) { + if (drro->drr_bonuslen > DN_MAX_BONUS_LEN(dnp)) + return (SET_ERROR(EINVAL)); drro->drr_raw_bonuslen = DN_MAX_BONUS_LEN(dnp); bonuslen = drro->drr_raw_bonuslen; } diff --git a/module/zfs/dnode_sync.c b/module/zfs/dnode_sync.c index f574130e50..12ab4bea14 100644 --- a/module/zfs/dnode_sync.c +++ b/module/zfs/dnode_sync.c @@ -854,6 +854,8 @@ dnode_sync(dnode_t *dn, dmu_tx_t *tx) dnode_rele(dn, (void *)(uintptr_t)tx->tx_txg); } + ASSERT3U(dnp->dn_bonuslen, <=, DN_MAX_BONUS_LEN(dnp)); + /* * Although we have dropped our reference to the dnode, it * can't be evicted until its written, and we haven't yet diff --git a/module/zfs/dsl_scan.c b/module/zfs/dsl_scan.c index 62ee9bb9ab..7659984171 100644 --- a/module/zfs/dsl_scan.c +++ b/module/zfs/dsl_scan.c @@ -1821,6 +1821,19 @@ dsl_scan_recurse(dsl_scan_t *scn, dsl_dataset_t *ds, dmu_objset_type_t ostype, ASSERT(!BP_IS_REDACTED(bp)); + /* + * There is an unlikely case of encountering dnodes with contradicting + * dn_bonuslen and DNODE_FLAG_SPILL_BLKPTR flag before in files created + * or modified before commit 4254acb was merged. As it is not possible + * to know which of the two is correct, report an error. + */ + if (dnp != NULL && + dnp->dn_bonuslen > DN_MAX_BONUS_LEN(dnp)) { + scn->scn_phys.scn_errors++; + spa_log_error(dp->dp_spa, zb); + return (SET_ERROR(EINVAL)); + } + if (BP_GET_LEVEL(bp) > 0) { arc_flags_t flags = ARC_FLAG_WAIT; int i; diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index f4d8eb59f0..98d91eae19 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -831,7 +831,8 @@ tests = ['recv_dedup', 'recv_dedup_encrypted_zvol', 'rsend_001_pos', 'send_freeobjects', 'send_realloc_files', 'send_realloc_encrypted_files', 'send_spill_block', 'send_holds', 'send_hole_birth', 'send_mixed_raw', 'send-wR_encrypted_zvol', - 'send_partial_dataset', 'send_invalid', 'send_doall'] + 'send_partial_dataset', 'send_invalid', 'send_doall', + 'send_raw_spill_block'] tags = ['functional', 'rsend'] [tests/functional/scrub_mirror] diff --git a/tests/zfs-tests/tests/functional/rsend/Makefile.am b/tests/zfs-tests/tests/functional/rsend/Makefile.am index 94bdd26745..ba1bcf24a5 100644 --- a/tests/zfs-tests/tests/functional/rsend/Makefile.am +++ b/tests/zfs-tests/tests/functional/rsend/Makefile.am @@ -49,6 +49,7 @@ dist_pkgdata_SCRIPTS = \ send_realloc_files.ksh \ send_realloc_encrypted_files.ksh \ send_spill_block.ksh \ + send_raw_spill_block.ksh \ send_holds.ksh \ send_hole_birth.ksh \ send_invalid.ksh \ diff --git a/tests/zfs-tests/tests/functional/rsend/send_raw_spill_block.ksh b/tests/zfs-tests/tests/functional/rsend/send_raw_spill_block.ksh new file mode 100755 index 0000000000..8d7451ae62 --- /dev/null +++ b/tests/zfs-tests/tests/functional/rsend/send_raw_spill_block.ksh @@ -0,0 +1,161 @@ +#!/bin/ksh + +# +# This file and its contents are supplied under the terms of the +# Common Development and Distribution License ("CDDL"), version 1.0. +# You may only use this file in accordance with the terms of version +# 1.0 of the CDDL. +# +# A full copy of the text of the CDDL should have accompanied this +# source. A copy of the CDDL is also available via the Internet at +# http://www.illumos.org/license/CDDL. +# + +# +# Copyright (c) 2019, Lawrence Livermore National Security, LLC. +# Copyright (c) 2021, George Amanakis. All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/rsend/rsend.kshlib + +# +# Description: +# Verify spill blocks are correctly preserved in raw sends. +# +# Strategy: +# 1) Create a set of files each containing some file data in an +# encrypted filesystem. +# 2) Add enough xattrs to the file to require a spill block. +# 3) Snapshot and raw send these files to a new dataset. +# 4) Modify the files and spill blocks in a variety of ways. +# 5) Send the changes using an raw incremental send stream. +# 6) Verify that all the xattrs (and thus the spill block) were +# preserved when receiving the incremental stream. +# + +verify_runnable "both" + +log_assert "Verify spill blocks are correctly preserved in raw sends" + +function cleanup +{ + rm -f $BACKDIR/fs@* + destroy_dataset $POOL/fs "-rR" + destroy_dataset $POOL/newfs "-rR" +} + +attrvalue="abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz" + +log_onexit cleanup + +log_must eval "echo 'password' | zfs create -o encryption=on" \ + "-o keyformat=passphrase -o keylocation=prompt " \ + "$POOL/fs" +log_must zfs set xattr=sa $POOL/fs +log_must zfs set dnodesize=legacy $POOL/fs +log_must zfs set recordsize=128k $POOL/fs + +# Create 40 files each with a spill block containing xattrs. Each file +# will be modified in a different way to validate the incremental receive. +for i in {1..40}; do + file="/$POOL/fs/file$i" + + log_must mkfile 16384 $file + for j in {1..20}; do + log_must set_xattr "testattr$j" "$attrvalue" $file + done +done + +# Snapshot the pool and send it to the new dataset. +log_must zfs snapshot $POOL/fs@snap1 +log_must eval "zfs send -w $POOL/fs@snap1 >$BACKDIR/fs@snap1" +log_must eval "zfs recv $POOL/newfs < $BACKDIR/fs@snap1" + +# +# Modify file[1-6]'s contents but not the spill blocks. +# +# file1 - Increase record size; single block +# file2 - Increase record size; multiple blocks +# file3 - Truncate file to zero size; single block +# file4 - Truncate file to smaller size; single block +# file5 - Truncate file to much larger size; add holes +# file6 - Truncate file to embedded size; embedded data +# +log_must mkfile 32768 /$POOL/fs/file1 +log_must mkfile 1048576 /$POOL/fs/file2 +log_must truncate -s 0 /$POOL/fs/file3 +log_must truncate -s 8192 /$POOL/fs/file4 +log_must truncate -s 1073741824 /$POOL/fs/file5 +log_must truncate -s 50 /$POOL/fs/file6 + +# +# Modify file[11-16]'s contents and their spill blocks. +# +# file11 - Increase record size; single block +# file12 - Increase record size; multiple blocks +# file13 - Truncate file to zero size; single block +# file14 - Truncate file to smaller size; single block +# file15 - Truncate file to much larger size; add holes +# file16 - Truncate file to embedded size; embedded data +# +log_must mkfile 32768 /$POOL/fs/file11 +log_must mkfile 1048576 /$POOL/fs/file12 +log_must truncate -s 0 /$POOL/fs/file13 +log_must truncate -s 8192 /$POOL/fs/file14 +log_must truncate -s 1073741824 /$POOL/fs/file15 +log_must truncate -s 50 /$POOL/fs/file16 + +for i in {11..20}; do + log_must rm_xattr testattr1 /$POOL/fs/file$i +done + +# +# Modify file[21-26]'s contents and remove their spill blocks. +# +# file21 - Increase record size; single block +# file22 - Increase record size; multiple blocks +# file23 - Truncate file to zero size; single block +# file24 - Truncate file to smaller size; single block +# file25 - Truncate file to much larger size; add holes +# file26 - Truncate file to embedded size; embedded data +# +log_must mkfile 32768 /$POOL/fs/file21 +log_must mkfile 1048576 /$POOL/fs/file22 +log_must truncate -s 0 /$POOL/fs/file23 +log_must truncate -s 8192 /$POOL/fs/file24 +log_must truncate -s 1073741824 /$POOL/fs/file25 +log_must truncate -s 50 /$POOL/fs/file26 + +for i in {21..30}; do + for j in {1..20}; do + log_must rm_xattr testattr$j /$POOL/fs/file$i + done +done + +# +# Modify file[31-40]'s spill blocks but not the file contents. +# +for i in {31..40}; do + file="/$POOL/fs/file$i" + log_must rm_xattr testattr$(((RANDOM % 20) + 1)) $file + log_must set_xattr testattr$(((RANDOM % 20) + 1)) "$attrvalue" $file +done + +# Calculate the expected recursive checksum for the source. +expected_cksum=$(recursive_cksum /$POOL/fs) + +# Snapshot the pool and send the incremental snapshot. +log_must zfs snapshot $POOL/fs@snap2 +log_must eval "zfs send -w -i $POOL/fs@snap1 $POOL/fs@snap2 >$BACKDIR/fs@snap2" +log_must eval "zfs recv $POOL/newfs < $BACKDIR/fs@snap2" +log_must eval "echo 'password' | zfs load-key $POOL/newfs" +log_must zfs mount $POOL/newfs + +# Validate the received copy using the received recursive checksum. +actual_cksum=$(recursive_cksum /$POOL/newfs) +if [[ "$expected_cksum" != "$actual_cksum" ]]; then + log_fail "Checksums differ ($expected_cksum != $actual_cksum)" +fi + +log_pass "Verify spill blocks are correctly preserved in raw sends"