Skip to content

Commit

Permalink
Handle and detect openzfs#13709's unlock regression
Browse files Browse the repository at this point in the history
In openzfs#13709, as in openzfs#11294 before it, it turns out that 63a2645 still had
the same failure mode as when it was first landed as d1d4769, and
fails to unlock certain datasets that formerly worked.

Rather than reverting it again, let's add handling to just throw out
the accounting metadata that failed to unlock when that happens, as
well as a test with a pre-broken pool image to ensure that we never get
bitten by this again.

Fixes: openzfs#13709

Signed-off-by: Rich Ercolani <[email protected]>
  • Loading branch information
rincebrain committed Nov 14, 2022
1 parent 0b2428d commit 7bcad26
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 3 deletions.
18 changes: 16 additions & 2 deletions module/zfs/dsl_crypt.c
Original file line number Diff line number Diff line change
Expand Up @@ -2671,6 +2671,7 @@ spa_do_crypt_objset_mac_abd(boolean_t generate, spa_t *spa, uint64_t dsobj,
objset_phys_t *osp = buf;
uint8_t portable_mac[ZIO_OBJSET_MAC_LEN];
uint8_t local_mac[ZIO_OBJSET_MAC_LEN];
const uint8_t zeroed_mac[ZIO_OBJSET_MAC_LEN] = {0};

/* look up the key from the spa's keystore */
ret = spa_keystore_lookup_key(spa, dsobj, FTAG, &dck);
Expand All @@ -2696,8 +2697,21 @@ spa_do_crypt_objset_mac_abd(boolean_t generate, spa_t *spa, uint64_t dsobj,
if (memcmp(portable_mac, osp->os_portable_mac,
ZIO_OBJSET_MAC_LEN) != 0 ||
memcmp(local_mac, osp->os_local_mac, ZIO_OBJSET_MAC_LEN) != 0) {
abd_return_buf(abd, buf, datalen);
return (SET_ERROR(ECKSUM));
/*
* If the MAC is zeroed out, we failed to decrypt it.
* This should only arise, at least on Linux,
* if we hit edge case handling for useraccounting, since we
* shouldn't get here without bailing out on error earlier
* otherwise.
*
* So if we're in that case, we can just fall through and
* special-casing noticing that it's zero will handle it
* elsewhere, since we can just regenerate it.
*/
if (memcmp(local_mac, zeroed_mac, ZIO_OBJSET_MAC_LEN) != 0) {
abd_return_buf(abd, buf, datalen);
return (SET_ERROR(ECKSUM));
}
}

abd_return_buf(abd, buf, datalen);
Expand Down
2 changes: 1 addition & 1 deletion tests/runfiles/common.run
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,7 @@ tests = [
'userquota_007_pos', 'userquota_008_pos', 'userquota_009_pos',
'userquota_010_pos', 'userquota_011_pos', 'userquota_012_neg',
'userspace_001_pos', 'userspace_002_pos', 'userspace_encrypted',
'userspace_send_encrypted']
'userspace_send_encrypted', 'userspace_encrypted_13709']
tags = ['functional', 'userquota']

[tests/functional/vdev_zaps]
Expand Down
2 changes: 2 additions & 0 deletions tests/zfs-tests/tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ nobase_dist_datadir_zfs_tests_tests_DATA += \
functional/upgrade/upgrade_common.kshlib \
functional/user_namespace/user_namespace.cfg \
functional/user_namespace/user_namespace_common.kshlib \
functional/userquota/13709_reproducer.bz2 \
functional/userquota/userquota.cfg \
functional/userquota/userquota_common.kshlib \
functional/vdev_zaps/vdev_zaps.kshlib \
Expand Down Expand Up @@ -1935,6 +1936,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
functional/userquota/userspace_003_pos.ksh \
functional/userquota/userspace_encrypted.ksh \
functional/userquota/userspace_send_encrypted.ksh \
functional/userquota/userspace_encrypted_13709.ksh \
functional/vdev_zaps/cleanup.ksh \
functional/vdev_zaps/setup.ksh \
functional/vdev_zaps/vdev_zaps_001_pos.ksh \
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#!/bin/ksh -p
#
# 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.
#

. $STF_SUITE/include/libtest.shlib
. $STF_SUITE/tests/functional/userquota/userquota_common.kshlib

#
# DESCRIPTION:
# Avoid allowing #11294/#13709 to recur a third time.
#
# So we hardcode a copy of a pool with this bug, try unlocking it,
# and fail on error. Simple.

function cleanup
{
destroy_pool $POOLNAME
rm -f $FILEDEV
}

log_onexit cleanup

FILEDEV="$TEST_BASE_DIR/userspace_13709"
POOLNAME="testpool_13709"

log_assert "ZFS should be able to unlock pools with #13709's failure mode"

log_must bzcat $STF_SUITE/tests/functional/userquota/13709_reproducer.bz2 > $FILEDEV

log_must zpool import -d $FILEDEV $POOLNAME

echo -e 'password\npassword\n' | log_must zfs mount -al

# Cleanup
cleanup

log_pass "#13709 not happening here"

0 comments on commit 7bcad26

Please sign in to comment.