Skip to content

Commit fa7d572

Browse files
rincebraintonyhutter
authored andcommitted
Handle and detect openzfs#13709's unlock regression (openzfs#14161)
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]> Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Tony Hutter <[email protected]>
1 parent d9de079 commit fa7d572

File tree

5 files changed

+68
-6
lines changed

5 files changed

+68
-6
lines changed

module/zfs/dsl_crypt.c

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2672,6 +2672,7 @@ spa_do_crypt_objset_mac_abd(boolean_t generate, spa_t *spa, uint64_t dsobj,
26722672
objset_phys_t *osp = buf;
26732673
uint8_t portable_mac[ZIO_OBJSET_MAC_LEN];
26742674
uint8_t local_mac[ZIO_OBJSET_MAC_LEN];
2675+
const uint8_t zeroed_mac[ZIO_OBJSET_MAC_LEN] = {0};
26752676

26762677
/* look up the key from the spa's keystore */
26772678
ret = spa_keystore_lookup_key(spa, dsobj, FTAG, &dck);
@@ -2694,10 +2695,24 @@ spa_do_crypt_objset_mac_abd(boolean_t generate, spa_t *spa, uint64_t dsobj,
26942695
return (0);
26952696
}
26962697

2697-
if (bcmp(portable_mac, osp->os_portable_mac, ZIO_OBJSET_MAC_LEN) != 0 ||
2698-
bcmp(local_mac, osp->os_local_mac, ZIO_OBJSET_MAC_LEN) != 0) {
2699-
abd_return_buf(abd, buf, datalen);
2700-
return (SET_ERROR(ECKSUM));
2698+
if (memcmp(portable_mac, osp->os_portable_mac,
2699+
ZIO_OBJSET_MAC_LEN) != 0 ||
2700+
memcmp(local_mac, osp->os_local_mac, ZIO_OBJSET_MAC_LEN) != 0) {
2701+
/*
2702+
* If the MAC is zeroed out, we failed to decrypt it.
2703+
* This should only arise, at least on Linux,
2704+
* if we hit edge case handling for useraccounting, since we
2705+
* shouldn't get here without bailing out on error earlier
2706+
* otherwise.
2707+
*
2708+
* So if we're in that case, we can just fall through and
2709+
* special-casing noticing that it's zero will handle it
2710+
* elsewhere, since we can just regenerate it.
2711+
*/
2712+
if (memcmp(local_mac, zeroed_mac, ZIO_OBJSET_MAC_LEN) != 0) {
2713+
abd_return_buf(abd, buf, datalen);
2714+
return (SET_ERROR(ECKSUM));
2715+
}
27012716
}
27022717

27032718
abd_return_buf(abd, buf, datalen);

tests/runfiles/common.run

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -892,7 +892,7 @@ tests = [
892892
'userquota_007_pos', 'userquota_008_pos', 'userquota_009_pos',
893893
'userquota_010_pos', 'userquota_011_pos', 'userquota_012_neg',
894894
'userspace_001_pos', 'userspace_002_pos', 'userspace_encrypted',
895-
'userspace_send_encrypted']
895+
'userspace_send_encrypted', 'userspace_encrypted_13709']
896896
tags = ['functional', 'userquota']
897897

898898
[tests/functional/vdev_zaps]
Binary file not shown.

tests/zfs-tests/tests/functional/userquota/Makefile.am

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@ dist_pkgdata_SCRIPTS = \
2222
userspace_002_pos.ksh \
2323
userspace_003_pos.ksh \
2424
userspace_encrypted.ksh \
25-
userspace_send_encrypted.ksh
25+
userspace_send_encrypted.ksh \
26+
userspace_encrypted_13709.ksh
2627

2728
dist_pkgdata_DATA = \
29+
13709_reproducer.bz2 \
2830
userquota.cfg \
2931
userquota_common.kshlib
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)