A little more research (and looking at the right set of keys :P), showed that doing the zfs diff causes the sk_dsl_key leak:

zfs_refcount_t at fffffe0c4d259058 has 2 current holds, 3 recently released holds
current holds:
reference with tag fffffe0bfcb4f678 "", held at:
fffffe0c9b4f5540 is allocated from reference_cache:
            ADDR          BUFADDR        TIMESTAMP           THREAD
                            CACHE          LASTLOG         CONTENTS
fffffe0c957302f8 fffffe0c9b4f5540      2ded0296e87 fffffe0bfdd7bbc0
                 fffffe0be5cbe008 fffffe0bd56567c0 fffffe0be0355e68
                 kmem_cache_alloc_debug+0x2fc
                 kmem_cache_alloc+0x135
                 zfs_refcount_add_many+0x82
                 zfs_refcount_add+0x13
                 spa_keystore_dsl_key_hold_impl+0x81
                 spa_keystore_dsl_key_hold_dd+0x9c
                 spa_keystore_create_mapping+0x7e
                 dsl_dataset_create_key_mapping+0x39
                 dsl_dataset_hold_obj_flags+0x68
                 dsl_dataset_hold_flags+0x1ee
                 dmu_objset_hold_flags+0x8b
                 zfs_ioc_next_obj+0x3b
                 zfsdev_ioctl+0x1fd
                 cdev_ioctl+0x2b
                 spec_ioctl+0x45

Looking at the code,

static int
zfs_ioc_next_obj(zfs_cmd_t *zc)
{
    objset_t *os = NULL;
    int error;

    error = dmu_objset_hold_flags(zc->zc_name, B_TRUE, FTAG, &os);
    if (error != 0)
        return (error);

    error = dmu_object_next(os, &zc->zc_obj, B_FALSE,
        dsl_dataset_phys(os->os_dsl_dataset)->ds_prev_snap_txg);

    dmu_objset_rele(os, FTAG);
    return (error);
}
...
void
dmu_objset_rele_flags(objset_t *os, boolean_t decrypt, void *tag)
{
    ds_hold_flags_t flags = (decrypt) ? DS_HOLD_FLAG_DECRYPT : 0;

    dsl_pool_t *dp = dmu_objset_pool(os);
    dsl_dataset_rele_flags(os->os_dsl_dataset, flags, tag);
    dsl_pool_rele(dp, tag);
}

void
dmu_objset_rele(objset_t *os, void *tag)
{
    dmu_objset_rele_flags(os, B_FALSE, tag);
}

and the cause becomes obvious -- we're holding the objset w/ DS_HOLD_FLAG_DECRYPT, but not releasing it with the flag set (which skips releasing the keys).

Looking at ZoL for comparison, they are just doing dmu_objset_hold(zc->zc_name, FTAG, &os); in zfs_ioc_next_obj. I'm unsure of the reasons for the difference in illumos. It may just be an artifact of the initial illumos port of zfs encryption.

Testing Notes (from Jason)¶

To test, I built a DEBUG SmartOS platform image (PI) with the change (DEBUG illumos-joyent build), and ran the zfs test suite on a VM running the new PI. All the expected tests pass. I also ran the (soon to be RTIed) zfs diff tests on the same PI, which also passed (except for the zfs diff + large dnodes, however that will be fixed with #11950.

In addition, the original trigger of this bug was a DEBUG build ASSERT that gets tripped if a ‘zfs diff’ is done on an encrypted dataset whose zpool is later exported (the ASSERT is triggered because of the leaked key reference). I was able to reproduce the ASSERT panic on an unmodified SmartOS DEBUG image, and could not reproduce it on the DEBUG PI with the fix applied. I also used the (also soon to be RTIed) fixed ::zfs_refcount dcmd (#11944) before and after running ‘zfs diff’ with reference tracking enabled to verify the hold taken by ‘zfs diff’ did not show up after the command completes.