Project

General

Profile

Bug #11880

changing encryption key on dataset with unencrypted children triggers VERIFY

Added by Jason King about 1 month ago. Updated 28 days ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
zfs - Zettabyte File System
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:

Description

Doing something similar to the following:

# zfs create -o encryption=on -o keylocation=prompt -o keyformat=passphrase pool/encrypted
...
# zfs create -o encryption=off pool/encrypted/unencrypted
# zfs change-key pool/encrypted

Will trigger a panic similar to:

assertion failed: dsl_dir_get_encryption_root_ddobj(dd, &curr_rddobj) == 0 (0x2 == 0x0), file: ../../common/fs/zfs/dsl_crypt.c, line: 1421

0x2 is ENOENT which makes sense -- an unencrypted dataset would not have an encrypted root. The panic stack shows the following top few frames:

> ::stack
vpanic()
0xfffffffffbe35131()
spa_keystore_change_key_sync_impl+0x232(20, 280, 20, fffffe8b7159ee00, fffffe8b735a8400)
spa_keystore_change_key_sync_impl+0x1fd(20, 20, 20, fffffe8b7159ee00, fffffe8b735a8400)
spa_keystore_change_key_sync+0x16f(fffffe00bae6c528, fffffe8b735a8400)
zcp_sync_task+0x83(fffffe8672cf4e08, fffffffff7e51f30, fffffffff7e52560, fffffe00bae6c528, 1, fffffe8af9ae1620)

What appears to be happening is that spa_keystore_change_key_sync does the following:

        /* Recurse into all child dsl dirs. */
        for (zap_cursor_init(zc, dp->dp_meta_objset,
            dsl_dir_phys(dd)->dd_child_dir_zapobj);
            zap_cursor_retrieve(zc, za) == 0;
            zap_cursor_advance(zc)) {
                spa_keystore_change_key_sync_impl(rddobj,
                    za->za_first_integer, new_rddobj, wkey, tx);
        }

and when it calls spa_keystore_change_key_sync_impl on an unencrypted child, the recursive call will trigger the VERIFY0.

The likely fix is to add ENOENT as another condition to stop the recursion.

History

#1

Updated by Jason King about 1 month ago

This same issue was also present in ZoL as issue 9524. We'll also use the ZoL fix.

#2

Updated by Jason King about 1 month ago

The ZoL fix was:

commit bae11ba8dc1dd992643dd0ab0b5cc3b3cc197fe9
Author: Tom Caputi <tcaputi@datto.com>
Date:   Wed Oct 30 14:27:28 2019 -0400

    Fix 'zfs change-key' with unencrypted child

    Currently, when you call 'zfs change-key' on an encrypted dataset
    that has an unencrypted child, the code will trigger a VERIFY.
    This VERIFY is leftover from before we allowed unencrypted
    datasets to exist underneath encrypted ones. This patch fixes the
    issue by simply replacing the VERIFY with an early return when
    recursing through datasets.

    Reviewed by: Jason King <jason.brian.king@gmail.com>
    Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
    Reviewed-by: Igor Kozhukhov <igor@dilos.org>
    Signed-off-by: Tom Caputi <tcaputi@datto.com>
    Closes #9524
#3

Updated by Electric Monk 28 days ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit ad3e6d4dd82f2e18743399134a4b99cf303478f6

commit  ad3e6d4dd82f2e18743399134a4b99cf303478f6
Author: Tom Caputi <tcaputi@datto.com>
Date:   2019-11-07T15:32:37.000Z

    11880 changing encryption key on dataset with unencrypted children triggers VERIFY
    Portions contributed by: Jason King <jason.brian.king@gmail.com>
    Reviewed by: Jason King <jason.brian.king@gmail.com>
    Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
    Reviewed by: Igor Kozhukhov <igor@dilos.org>
    Reviewed by: Andrew Stormont <astormont@racktopsystems.com>
    Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Also available in: Atom PDF