Skip to content

Commit ffad2c2

Browse files
committed
Handle and detect #13709's unlock regression
In #13709, as in #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: #13709 Signed-off-by: Rich Ercolani <[email protected]>
1 parent 0b2428d commit ffad2c2

File tree

5 files changed

+64
-3
lines changed

5 files changed

+64
-3
lines changed

module/zfs/dsl_crypt.c

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2671,6 +2671,7 @@ spa_do_crypt_objset_mac_abd(boolean_t generate, spa_t *spa, uint64_t dsobj,
26712671
objset_phys_t *osp = buf;
26722672
uint8_t portable_mac[ZIO_OBJSET_MAC_LEN];
26732673
uint8_t local_mac[ZIO_OBJSET_MAC_LEN];
2674+
uint8_t zeroed_mac[ZIO_OBJSET_MAC_LEN] = {0};
26742675

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

27032717
abd_return_buf(abd, buf, datalen);

tests/runfiles/common.run

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -911,7 +911,7 @@ tests = [
911911
'userquota_007_pos', 'userquota_008_pos', 'userquota_009_pos',
912912
'userquota_010_pos', 'userquota_011_pos', 'userquota_012_neg',
913913
'userspace_001_pos', 'userspace_002_pos', 'userspace_encrypted',
914-
'userspace_send_encrypted']
914+
'userspace_send_encrypted', 'userspace_encrypted_13709']
915915
tags = ['functional', 'userquota']
916916

917917
[tests/functional/vdev_zaps]

tests/zfs-tests/tests/Makefile.am

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,7 @@ nobase_dist_datadir_zfs_tests_tests_DATA += \
368368
functional/upgrade/upgrade_common.kshlib \
369369
functional/user_namespace/user_namespace.cfg \
370370
functional/user_namespace/user_namespace_common.kshlib \
371+
functional/userquota/13709_reproducer.bz2 \
371372
functional/userquota/userquota.cfg \
372373
functional/userquota/userquota_common.kshlib \
373374
functional/vdev_zaps/vdev_zaps.kshlib \
@@ -1935,6 +1936,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
19351936
functional/userquota/userspace_003_pos.ksh \
19361937
functional/userquota/userspace_encrypted.ksh \
19371938
functional/userquota/userspace_send_encrypted.ksh \
1939+
functional/userquota/userspace_encrypted_13709.ksh \
19381940
functional/vdev_zaps/cleanup.ksh \
19391941
functional/vdev_zaps/setup.ksh \
19401942
functional/vdev_zaps/vdev_zaps_001_pos.ksh \
Binary file not shown.
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
#!/bin/ksh -p
2+
#
3+
# This file and its contents are supplied under the terms of the
4+
# Common Development and Distribution License ("CDDL"), version 1.0.
5+
# You may only use this file in accordance with the terms of version
6+
# 1.0 of the CDDL.
7+
#
8+
# A full copy of the text of the CDDL should have accompanied this
9+
# source. A copy of the CDDL is also available via the Internet at
10+
# http://www.illumos.org/license/CDDL.
11+
#
12+
13+
. $STF_SUITE/include/libtest.shlib
14+
. $STF_SUITE/tests/functional/userquota/userquota_common.kshlib
15+
16+
#
17+
# DESCRIPTION:
18+
# Avoid allowing #11294/#13709 to recur a third time.
19+
#
20+
# So we hardcode a copy of a pool with this bug, try unlocking it,
21+
# and fail on error. Simple.
22+
23+
function cleanup
24+
{
25+
destroy_pool $POOLNAME
26+
rm -f $FILEDEV
27+
}
28+
29+
log_onexit cleanup
30+
31+
FILEDEV="$TEST_BASE_DIR/userspace_13709"
32+
POOLNAME="testpool_13709"
33+
34+
log_assert "ZFS should be able to unlock pools with #13709's failure mode"
35+
36+
log_must bzcat $STF_SUITE/tests/functional/userquota/13709_reproducer.bz2 > $FILEDEV
37+
38+
log_must zpool import -d $FILEDEV $POOLNAME
39+
40+
echo -e 'password\npassword\n' | log_must zfs mount -al
41+
42+
# Cleanup
43+
cleanup
44+
45+
log_pass "#13709 not happening here"

0 commit comments

Comments
 (0)